Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: ignore extra pauses during wasm compilation #2106

Merged
merged 1 commit into from
Oct 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions src/adapter/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,12 @@ export class Thread implements IVariableStoreLocationProvider {
const context = new ExecutionContext(description);
this._executionContexts.set(description.id, context);

if (this._scriptWithSourceMapHandler) {
this._installWasmPauseHandler(description.id);
}
}

private _installWasmPauseHandler(contextId: number) {
// WASM files don't have sourcemaps and so aren't paused in the usual
// instrumentation BP. But we do need to pause, either to figure out the WAT
// lines or by mapping symbolicated files.
Expand All @@ -937,14 +943,14 @@ export class Thread implements IVariableStoreLocationProvider {
// }),
//
// For now, overwrite WebAssembly methods in the runtime to get the same effect. This needs to be run in event new execution context:
this._cdp.Runtime.evaluate({
return this._cdp.Runtime.evaluate({
expression: breakOnWasmInit.expr(),
silent: true,
contextId: description.id,
contextId,
});
}

async _executionContextDestroyed(contextId: number) {
private async _executionContextDestroyed(contextId: number) {
const context = this._executionContexts.get(contextId);
if (!context) return;
this._executionContexts.delete(contextId);
Expand Down Expand Up @@ -1016,13 +1022,18 @@ export class Thread implements IVariableStoreLocationProvider {
}

const expectedPauseReason = this._expectedPauseReason;
if (isWasmBreak && this._lastParsedWasmScriptIds) {
if (isWasmBreak) {
// Resolve all pending WASM symbols when we've just initialized something
const ids = this._lastParsedWasmScriptIds;
this._lastParsedWasmScriptIds = undefined;
await Promise.all(
ids.map(id => this._handleSourceMapPause(id, location)),
);
// In theory `ids` should always have something, but it seems like we
// can see (and ignore) multiple scriptParsed events with the same ID
// for the same program if it's WebAssembly.compile'd multiple times.
if (ids) {
await Promise.all(
ids.map(id => this._handleSourceMapPause(id, location)),
);
}
return this.resume();
} else if (scriptId && (await this._handleSourceMapPause(scriptId, location))) {
// Pause if we just resolved a breakpoint that's on this
Expand Down Expand Up @@ -1979,12 +1990,15 @@ export class Thread implements IVariableStoreLocationProvider {
// setting the URL breakpoint for wasm fails if debugger isn't fully initialized
await this.debuggerReady.promise;

const wasmHandlers = Promise.all([...this._executionContexts.keys()]
.map(id => this._installWasmPauseHandler(id)));
const result = await Promise.all([
this._cdp.Debugger.setInstrumentationBreakpoint({
instrumentation: 'beforeScriptWithSourceMapExecution',
}),
]);
this._pauseOnSourceMapBreakpointIds = result.map(r => r?.breakpointId).filter(truthy);
await wasmHandlers;
} else if (!needsPause && this._pauseOnSourceMapBreakpointIds?.length) {
const breakpointIds = this._pauseOnSourceMapBreakpointIds;
this._pauseOnSourceMapBreakpointIds = undefined;
Expand Down
Loading