diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index c9b8299173..c9cba9c1f3 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -108,7 +108,6 @@ export abstract class BrowserContext extends SdkObject { return; // Debugger will pause execution upon page.pause in headed mode. const contextDebugger = new Debugger(this); - this.instrumentation.addListener(contextDebugger, this); // When PWDEBUG=1, show inspector for each context. if (debugMode() === 'inspector') diff --git a/packages/playwright-core/src/server/debugger.ts b/packages/playwright-core/src/server/debugger.ts index 72f73713df..8bab8f780e 100644 --- a/packages/playwright-core/src/server/debugger.ts +++ b/packages/playwright-core/src/server/debugger.ts @@ -16,7 +16,7 @@ import { EventEmitter } from 'events'; import { debugMode, isUnderTest, monotonicTime } from '../utils'; -import type { BrowserContext } from './browserContext'; +import { BrowserContext } from './browserContext'; import type { CallMetadata, InstrumentationListener, SdkObject } from './instrumentation'; import { commandsWithTracingSnapshots, pausesBeforeInputActions } from '../protocol/channels'; @@ -40,6 +40,10 @@ export class Debugger extends EventEmitter implements InstrumentationListener { this._enabled = debugMode() === 'inspector'; if (this._enabled) this.pauseOnNextStatement(); + context.instrumentation.addListener(this, context); + this._context.once(BrowserContext.Events.Close, () => { + this._context.instrumentation.removeListener(this); + }); } static lookup(context?: BrowserContext): Debugger | undefined { diff --git a/packages/playwright-core/src/server/instrumentation.ts b/packages/playwright-core/src/server/instrumentation.ts index b9d696f750..8ab414aedf 100644 --- a/packages/playwright-core/src/server/instrumentation.ts +++ b/packages/playwright-core/src/server/instrumentation.ts @@ -36,6 +36,8 @@ export type Attribution = { import type { CallMetadata } from '../protocol/callMetadata'; export type { CallMetadata } from '../protocol/callMetadata'; +export const kTestSdkObjects = new WeakSet(); + export class SdkObject extends EventEmitter { guid: string; attribution: Attribution; @@ -47,6 +49,8 @@ export class SdkObject extends EventEmitter { this.setMaxListeners(0); this.attribution = { ...parent.attribution }; this.instrumentation = parent.instrumentation; + if (process.env._PW_INTERNAL_COUNT_SDK_OBJECTS) + kTestSdkObjects.add(this); } } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 8b7df7bc2e..346a962f36 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -122,6 +122,7 @@ export class Recorder implements InstrumentationListener { this._context.once(BrowserContext.Events.Close, () => { this._contextRecorder.dispose(); + this._context.instrumentation.removeListener(this); recorderApp.close().catch(() => {}); }); this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], primaryFileName: string }) => { diff --git a/tests/library/channels.spec.ts b/tests/library/channels.spec.ts index 00ac430ae4..e9b357bcbe 100644 --- a/tests/library/channels.spec.ts +++ b/tests/library/channels.spec.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import fs from 'fs'; import domain from 'domain'; import { playwrightTest as it, expect } from '../config/browserTest'; @@ -204,6 +205,85 @@ it('should work with the domain module', async ({ browserType, server, browserNa throw err; }); +it('make sure that the client/server side context, page, etc. objects were garbage collected', async ({ browserName, server, childProcess }, testInfo) => { + // WeakRef was added in Node.js 14 + it.skip(parseInt(process.version.slice(1), 10) < 14); + const scriptPath = testInfo.outputPath('test.js'); + const script = ` + const playwright = require(${JSON.stringify(require.resolve('playwright'))}); + const { kTestSdkObjects } = require(${JSON.stringify(require.resolve('../../packages/playwright-core/lib/server/instrumentation'))}); + const { existingDispatcher } = require(${JSON.stringify(require.resolve('../../packages/playwright-core/lib/server/dispatchers/dispatcher'))}); + + const toImpl = playwright._toImpl; + + (async () => { + const clientSideObjectsSizeBeforeLaunch = playwright._connection._objects.size; + const browser = await playwright['${browserName}'].launch(); + const objectRefs = []; + const dispatcherRefs = []; + + for (let i = 0; i < 5; i++) { + const context = await browser.newContext(); + const page = await context.newPage(); + const response = await page.goto('${server.EMPTY_PAGE}'); + objectRefs.push(new WeakRef(toImpl(context))); + objectRefs.push(new WeakRef(toImpl(page))); + objectRefs.push(new WeakRef(toImpl(response))); + dispatcherRefs.push( + new WeakRef(existingDispatcher(toImpl(context))), + new WeakRef(existingDispatcher(toImpl(page))), + new WeakRef(existingDispatcher(toImpl(response))), + ); + } + + assertServerSideObjectsExistance(true); + assertServerSideDispatchersExistance(true); + await browser.close(); + + for (let i = 0; i < 5; i++) { + await new Promise(resolve => setTimeout(resolve, 100)); + global.gc(); + } + + assertServerSideObjectsExistance(false); + assertServerSideDispatchersExistance(false); + + assertClientSideObjects(); + + function assertClientSideObjects() { + if (playwright._connection._objects.size !== clientSideObjectsSizeBeforeLaunch) + throw new Error('Client-side objects were not cleaned up'); + } + + function assertServerSideObjectsExistance(expected) { + for (const ref of objectRefs) { + if (kTestSdkObjects.has(ref.deref()) !== expected) { + throw new Error('Unexpected SdkObject existence! (expected: ' + expected + ')'); + } + } + } + + function assertServerSideDispatchersExistance(expected) { + for (const ref of dispatcherRefs) { + const impl = ref.deref(); + if (!!impl !== expected) + throw new Error('Dispatcher is still alive!'); + } + } + })(); + `; + await fs.promises.writeFile(scriptPath, script); + const testSdkObjectsProcess = childProcess({ + command: ['node', '--expose-gc', scriptPath], + env: { + ...process.env, + _PW_INTERNAL_COUNT_SDK_OBJECTS: '1', + } + }); + const { exitCode } = await testSdkObjectsProcess.exited; + expect(exitCode).toBe(0); +}); + async function expectScopeState(object, golden) { golden = trimGuids(golden); const remoteState = trimGuids(await object._channel.debugScopeState());