From de9bb773937002d3ad6e07dda12edfff9f851229 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 17 Aug 2026 12:07:58 -0700 Subject: [PATCH 1/3] fix(chromium): unrouteAll hangs when a session is gone in the browser Messages addressed to a session that no longer exists in the browser are answered with a -32001 error that has no sessionId, so it was routed to the root session, did not match any callback there and got dropped. The pending command was never resolved, e.g. context.unrouteAll() hung forever. Route such errors by the message id and reject the command as closed. Also dispose worker sessions when their frame session is disposed, otherwise a worker of a detached oopif is never removed and we keep talking to it. Fixes: https://github.com/microsoft/playwright/issues/42278 --- .../src/server/chromium/crConnection.ts | 21 +++++++++++---- .../src/server/chromium/crPage.ts | 21 ++++++++++----- tests/library/chromium/oopif.spec.ts | 27 +++++++++++++++++++ 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index f3e907233b83d..88666aac2a050 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -42,6 +42,9 @@ export type ConnectionEventMap = { // should ignore. export const kBrowserCloseMessageId = -9999; +// Error code returned by the browser for a message addressed to a closed or unknown session. +const kSessionNotFoundErrorCode = -32001; + export class CRConnection extends SdkObject { private _lastId = 0; private readonly _transport: ConnectionTransport; @@ -79,9 +82,13 @@ export class CRConnection extends SdkObject { this._protocolLogger('receive', message); if (message.id === kBrowserCloseMessageId) return; - const session = this._sessions.get(message.sessionId || ''); - if (session) - session._onMessage(message); + // Unknown session error has no sessionId, so route it by the message id. + const session = message.error?.code === kSessionNotFoundErrorCode ? this._sessionWithCallback(message.id) : this._sessions.get(message.sessionId || ''); + session?._onMessage(message); + } + + private _sessionWithCallback(id: number | undefined): CRSession | undefined { + return [...this._sessions.values()].find(session => session._hasCallback(id)); } _onClose(reason?: string) { @@ -151,18 +158,22 @@ export class CRSession extends SdkObject return this.send(method, params).catch((error: ProtocolError) => debugLogger.log('error', error)); } + _hasCallback(id: number | undefined): boolean { + return id !== undefined && this._callbacks.has(id); + } + _onMessage(object: ProtocolResponse) { if (object.id && this._callbacks.has(object.id)) { const callback = this._callbacks.get(object.id)!; this._callbacks.delete(object.id); if (object.error) { + if (object.error.code === kSessionNotFoundErrorCode) + callback.error.type = 'closed'; callback.error.setMessage(object.error.message); callback.reject(callback.error); } else { callback.resolve(object.result); } - } else if (object.id && object.error?.code === -32001) { - // Message to a closed session, just ignore it. } else { assert(!object.id, object?.error?.message || undefined); Promise.resolve().then(() => { diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 52aab753da552..d246a71f9702b 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -580,6 +580,8 @@ class FrameSession { this._firstNonInitialNavigationCommittedReject(new TargetClosedError(this._page.closeReason())); for (const childSession of this._childSessions) childSession.dispose(); + for (const sessionId of [...this._workerSessions.keys()]) + this._removeWorkerSession(sessionId); if (this._parentSession) this._parentSession._childSessions.delete(this); eventsHelper.removeEventListeners(this._eventListeners); @@ -780,16 +782,21 @@ class FrameSession { session.on('Runtime.exceptionThrown', exception => this._page.addPageError(exceptionToError(exception.exceptionDetails), stackTraceToLocation(exception.exceptionDetails.stackTrace))); } + private _removeWorkerSession(sessionId: string): boolean { + const workerSession = this._workerSessions.get(sessionId); + if (!workerSession) + return false; + this._workerSessions.delete(sessionId); + this._crPage._networkManager.removeSession(workerSession); + workerSession.dispose(); + this._page.removeWorker(sessionId); + return true; + } + _onDetachedFromTarget(event: Protocol.Target.detachedFromTargetPayload) { // This might be a worker... - const workerSession = this._workerSessions.get(event.sessionId); - if (workerSession) { - this._workerSessions.delete(event.sessionId); - this._crPage._networkManager.removeSession(workerSession); - workerSession.dispose(); - this._page.removeWorker(event.sessionId); + if (this._removeWorkerSession(event.sessionId)) return; - } // ... or an oopif. const childFrameSession = this._crPage._sessions.get(event.targetId!); diff --git a/tests/library/chromium/oopif.spec.ts b/tests/library/chromium/oopif.spec.ts index a8971fe638a4c..45d91a724360e 100644 --- a/tests/library/chromium/oopif.spec.ts +++ b/tests/library/chromium/oopif.spec.ts @@ -15,6 +15,7 @@ */ import { contextTest as it, expect } from '../../config/browserTest'; +import { attachFrame } from '../../config/utils'; import type { Frame, Browser } from '@playwright/test'; it.use({ @@ -43,6 +44,32 @@ it('should handle oopif detach', async function({ page, browser, server }) { expect(detachedFrame).toBe(frame); }); +it('should remove workers of a detached oopif', async function({ page, server }) { + await page.goto(server.EMPTY_PAGE); + const [worker] = await Promise.all([ + page.waitForEvent('worker'), + attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/worker/worker.html'), + ]); + expect(page.workers().length).toBe(1); + await Promise.all([ + worker.waitForEvent('close'), + page.goto(server.PREFIX + '/title.html'), + ]); + expect(page.workers().length).toBe(0); +}); + +it('should not hang in unrouteAll when oopif worker is gone', async function({ page, context, server }) { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42278' }); + await context.route('**/*', route => route.continue()); + await page.goto(server.EMPTY_PAGE); + await Promise.all([ + page.waitForEvent('worker'), + attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/worker/worker.html'), + ]); + await page.goto(server.CROSS_PROCESS_PREFIX + '/title.html'); + await context.unrouteAll(); +}); + it('should handle remote -> local -> remote transitions', async function({ page, browser, server }) { await page.goto(server.PREFIX + '/dynamic-oopif.html'); expect(page.frames().length).toBe(2); From f570d49c04eff16155666b66219e0daa37068c2e Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 17 Aug 2026 12:23:01 -0700 Subject: [PATCH 2/3] chore(chromium): simplify session lookup by message id - hoist the message id check into the caller, so the helpers take a number - look up the session without allocating an array - drop the redundant worker session key copy in FrameSession.dispose - assert that an oopif is actually created in the new tests --- .../src/server/chromium/crConnection.ts | 14 +++++++++----- .../playwright-core/src/server/chromium/crPage.ts | 2 +- tests/library/chromium/oopif.spec.ts | 6 ++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index 88666aac2a050..dc1d3b39f7b91 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -83,12 +83,16 @@ export class CRConnection extends SdkObject { if (message.id === kBrowserCloseMessageId) return; // Unknown session error has no sessionId, so route it by the message id. - const session = message.error?.code === kSessionNotFoundErrorCode ? this._sessionWithCallback(message.id) : this._sessions.get(message.sessionId || ''); + const session = message.id && message.error?.code === kSessionNotFoundErrorCode ? + this._sessionWithCallback(message.id) : this._sessions.get(message.sessionId || ''); session?._onMessage(message); } - private _sessionWithCallback(id: number | undefined): CRSession | undefined { - return [...this._sessions.values()].find(session => session._hasCallback(id)); + private _sessionWithCallback(id: number): CRSession | undefined { + for (const session of this._sessions.values()) { + if (session._hasCallback(id)) + return session; + } } _onClose(reason?: string) { @@ -158,8 +162,8 @@ export class CRSession extends SdkObject return this.send(method, params).catch((error: ProtocolError) => debugLogger.log('error', error)); } - _hasCallback(id: number | undefined): boolean { - return id !== undefined && this._callbacks.has(id); + _hasCallback(id: number): boolean { + return this._callbacks.has(id); } _onMessage(object: ProtocolResponse) { diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index d246a71f9702b..f53e0ecec637c 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -580,7 +580,7 @@ class FrameSession { this._firstNonInitialNavigationCommittedReject(new TargetClosedError(this._page.closeReason())); for (const childSession of this._childSessions) childSession.dispose(); - for (const sessionId of [...this._workerSessions.keys()]) + for (const sessionId of this._workerSessions.keys()) this._removeWorkerSession(sessionId); if (this._parentSession) this._parentSession._childSessions.delete(this); diff --git a/tests/library/chromium/oopif.spec.ts b/tests/library/chromium/oopif.spec.ts index 45d91a724360e..2a9f525472778 100644 --- a/tests/library/chromium/oopif.spec.ts +++ b/tests/library/chromium/oopif.spec.ts @@ -44,12 +44,13 @@ it('should handle oopif detach', async function({ page, browser, server }) { expect(detachedFrame).toBe(frame); }); -it('should remove workers of a detached oopif', async function({ page, server }) { +it('should remove workers of a detached oopif', async function({ page, browser, server }) { await page.goto(server.EMPTY_PAGE); const [worker] = await Promise.all([ page.waitForEvent('worker'), attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/worker/worker.html'), ]); + await assertOOPIFCount(browser, 1); expect(page.workers().length).toBe(1); await Promise.all([ worker.waitForEvent('close'), @@ -58,7 +59,7 @@ it('should remove workers of a detached oopif', async function({ page, server }) expect(page.workers().length).toBe(0); }); -it('should not hang in unrouteAll when oopif worker is gone', async function({ page, context, server }) { +it('should not hang in unrouteAll when oopif worker is gone', async function({ page, context, browser, server }) { it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42278' }); await context.route('**/*', route => route.continue()); await page.goto(server.EMPTY_PAGE); @@ -66,6 +67,7 @@ it('should not hang in unrouteAll when oopif worker is gone', async function({ p page.waitForEvent('worker'), attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/worker/worker.html'), ]); + await assertOOPIFCount(browser, 1); await page.goto(server.CROSS_PROCESS_PREFIX + '/title.html'); await context.unrouteAll(); }); From e631f95ea684c8790b5f138f565e0317e4a40851 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 17 Aug 2026 12:30:16 -0700 Subject: [PATCH 3/3] chore(chromium): revert unknown session routing Disposing worker sessions in FrameSession.dispose is enough for the reported hang: Target.detachedFromTarget for the oopif arrives before the pending command, and disposing the worker session rejects it. Bring the connection side back when there is a scenario that needs it. --- .../src/server/chromium/crConnection.ts | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index dc1d3b39f7b91..f3e907233b83d 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -42,9 +42,6 @@ export type ConnectionEventMap = { // should ignore. export const kBrowserCloseMessageId = -9999; -// Error code returned by the browser for a message addressed to a closed or unknown session. -const kSessionNotFoundErrorCode = -32001; - export class CRConnection extends SdkObject { private _lastId = 0; private readonly _transport: ConnectionTransport; @@ -82,17 +79,9 @@ export class CRConnection extends SdkObject { this._protocolLogger('receive', message); if (message.id === kBrowserCloseMessageId) return; - // Unknown session error has no sessionId, so route it by the message id. - const session = message.id && message.error?.code === kSessionNotFoundErrorCode ? - this._sessionWithCallback(message.id) : this._sessions.get(message.sessionId || ''); - session?._onMessage(message); - } - - private _sessionWithCallback(id: number): CRSession | undefined { - for (const session of this._sessions.values()) { - if (session._hasCallback(id)) - return session; - } + const session = this._sessions.get(message.sessionId || ''); + if (session) + session._onMessage(message); } _onClose(reason?: string) { @@ -162,22 +151,18 @@ export class CRSession extends SdkObject return this.send(method, params).catch((error: ProtocolError) => debugLogger.log('error', error)); } - _hasCallback(id: number): boolean { - return this._callbacks.has(id); - } - _onMessage(object: ProtocolResponse) { if (object.id && this._callbacks.has(object.id)) { const callback = this._callbacks.get(object.id)!; this._callbacks.delete(object.id); if (object.error) { - if (object.error.code === kSessionNotFoundErrorCode) - callback.error.type = 'closed'; callback.error.setMessage(object.error.message); callback.reject(callback.error); } else { callback.resolve(object.result); } + } else if (object.id && object.error?.code === -32001) { + // Message to a closed session, just ignore it. } else { assert(!object.id, object?.error?.message || undefined); Promise.resolve().then(() => {