From 7d3672899f86cf87acc8f8d91b5b1a0a0fe0f0ac Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 22 Nov 2021 20:08:09 -0800 Subject: [PATCH] fix(tracing): race in stopChunk (#10481) 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. --- .../src/server/trace/recorder/tracing.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index 9620bdfe3f..a3fdfd3a2b 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -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; @@ -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();