fix: leaking server side objects (#13991)

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')

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 {

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<SdkObject>();
+
 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);
   }
 }
 

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 }) => {

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());