From 7ac4420cf46298185ec61cf6a498fd7c36deb38f Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 7 May 2026 10:55:16 -0700 Subject: [PATCH] fix(mcp): serialize shared browser launch in --isolated mode When multiple HTTP clients call tools concurrently with --isolated, the factory's create callback awaited createBrowserWithInfo before incrementing clientCount, so each concurrent caller saw clientCount === 0 and launched its own browser. Only the last assignment to sharedBrowser was tracked, leaking the rest. Replace sharedBrowser with sharedBrowserPromise that is set synchronously, so concurrent callers all await the same in-flight launch. On rejection, the slot clears so subsequent calls can retry. Fixes: https://github.com/microsoft/playwright-mcp/issues/1607 --- .../playwright-core/src/tools/mcp/program.ts | 23 +++++++++------ tests/mcp/http.spec.ts | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/packages/playwright-core/src/tools/mcp/program.ts b/packages/playwright-core/src/tools/mcp/program.ts index 37917055d2d58..75c76cdf9275d 100644 --- a/packages/playwright-core/src/tools/mcp/program.ts +++ b/packages/playwright-core/src/tools/mcp/program.ts @@ -95,7 +95,7 @@ export function decorateMCPCommand(command: Command) { const config = await resolveCLIConfigForMCP(options); const tools = filteredTools(config); const useSharedBrowser = config.sharedBrowserContext || config.browser.isolated; - let sharedBrowser: playwright.Browser | undefined; + let sharedBrowserPromise: Promise | undefined; let clientCount = 0; const clientNameCounters = new Map(); @@ -105,14 +105,19 @@ export function decorateMCPCommand(command: Command) { version, toolSchemas: tools.map(tool => tool.schema), create: async (clientInfo: ClientInfo) => { - if (useSharedBrowser && clientCount === 0) { - const { browser, canBind } = await createBrowserWithInfo(config, clientInfo, options); - sharedBrowser = browser; - if (canBind) - await browser.bind(clientInfo.clientName, { workspaceDir: clientInfo.cwd }); + if (useSharedBrowser && !sharedBrowserPromise) { + sharedBrowserPromise = (async () => { + const { browser, canBind } = await createBrowserWithInfo(config, clientInfo, options); + if (canBind) + await browser.bind(clientInfo.clientName, { workspaceDir: clientInfo.cwd }); + return browser; + })().catch(error => { + sharedBrowserPromise = undefined; + throw error; + }); } clientCount++; - const { browser, canBind } = sharedBrowser ? { browser: sharedBrowser, canBind: false } : await createBrowserWithInfo(config, clientInfo, options); + const { browser, canBind } = sharedBrowserPromise ? { browser: await sharedBrowserPromise, canBind: false } : await createBrowserWithInfo(config, clientInfo, options); if (canBind) { const count = (clientNameCounters.get(clientInfo.clientName) ?? 0) + 1; clientNameCounters.set(clientInfo.clientName, count); @@ -124,11 +129,11 @@ export function decorateMCPCommand(command: Command) { }, disposed: async backend => { clientCount--; - if (sharedBrowser && clientCount > 0) + if (sharedBrowserPromise && clientCount > 0) return; testDebug('close browser'); - sharedBrowser = undefined; + sharedBrowserPromise = undefined; const browserContext = (backend as BrowserBackend).browserContext; await browserContext.close().catch(() => { }); await browserContext.browser()!.close().catch(() => { }); diff --git a/tests/mcp/http.spec.ts b/tests/mcp/http.spec.ts index 6ea03c9e8482e..18d231330d54e 100644 --- a/tests/mcp/http.spec.ts +++ b/tests/mcp/http.spec.ts @@ -203,6 +203,35 @@ test('http transport browser lifecycle (isolated, multiclient)', async ({ server }); }); +test('http transport browser lifecycle (isolated, concurrent clients)', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/1607' } }, async ({ serverEndpoint, server }) => { + const { url, stderr } = await serverEndpoint({ args: ['--isolated'] }); + + const clients = await Promise.all([1, 2, 3].map(async () => { + const transport = new StreamableHTTPClientTransport(new URL('/mcp', url)); + const client = new Client({ name: 'test', version: '1.0.0' }); + await client.connect(transport); + return { transport, client }; + })); + + await Promise.all(clients.map(({ client }) => client.callTool({ + name: 'browser_navigate', + arguments: { url: server.HELLO_WORLD }, + }))); + + for (const { transport, client } of clients) { + await transport.terminateSession(); + await client.close(); + } + + await expect.poll(() => formatLog(stderr())).toEqual({ + 'create http session': 3, + 'delete http session': 3, + 'create context': 3, + 'create browser (isolated)': 1, + 'close browser': 1, + }); +}); + test('http transport browser lifecycle (persistent)', async ({ serverEndpoint, server }) => { const { url, stderr } = await serverEndpoint();