Skip to content

Commit

Permalink
cherry-pick(#10481): fix(tracing): race in stopChunk (#10498)
Browse files Browse the repository at this point in the history
Consider the following scenario:
- Tracing is started.
- API call is made (e.g. page.waitForResponse), almost finishes, and
  enters onAfterCall where it starts a snapshot.
- tracing.stopChunk is called, and waits for existing actions to finish.
  However, it does so by calling onAfterCall one more time.
- tracing.stopChunk removes instrumentation listener and returns
  to the client.
- Client starts zipping files.
- Original API call finishes the snapshot and saves it to the trace file.

This results in trace file being written to while the zip is still working.
  • Loading branch information
dgozman authored Nov 23, 2021
1 parent 2771fae commit 4190791
Showing 1 changed file with 12 additions and 11 deletions.
23 changes: 12 additions & 11 deletions packages/playwright-core/src/server/trace/recorder/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,18 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
throw new Error(`Tracing is already stopping`);
this._isStopping = true;

if (!this._state || !this._state.recording) {
this._isStopping = false;
if (save)
throw new Error(`Must start tracing before stopping`);
return { artifact: null, entries: [] };
}

const state = this._state!;
this._context.instrumentation.removeListener(this);
if (this._state?.options.screenshots)
this._stopScreencast();

for (const { sdkObject, metadata, beforeSnapshot, actionSnapshot, afterSnapshot } of this._pendingCalls.values()) {
await Promise.all([beforeSnapshot, actionSnapshot, afterSnapshot]);
let callMetadata = metadata;
Expand All @@ -185,17 +197,6 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha
await this.onAfterCall(sdkObject, callMetadata);
}

if (!this._state || !this._state.recording) {
this._isStopping = false;
if (save)
throw new Error(`Must start tracing before stopping`);
return { artifact: null, entries: [] };
}

const state = this._state!;
this._context.instrumentation.removeListener(this);
if (state.options.screenshots)
this._stopScreencast();
if (state.options.snapshots)
await this._snapshotter.stop();

Expand Down

0 comments on commit 4190791

Please sign in to comment.