diff --git a/packages/cli/src/browser/launch.boundedClose.test.ts b/packages/cli/src/browser/launch.boundedClose.test.ts index 7607926b27..4e59b81d71 100644 --- a/packages/cli/src/browser/launch.boundedClose.test.ts +++ b/packages/cli/src/browser/launch.boundedClose.test.ts @@ -1,9 +1,20 @@ import { spawn } from "node:child_process"; import { once } from "node:events"; import type { Browser, LaunchOptions, PuppeteerNode } from "puppeteer-core"; -import { expect, it } from "vitest"; +import { afterEach, expect, it, vi } from "vitest"; import { launchManagedBrowser } from "./launch.js"; +const treeKill = vi.hoisted(() => vi.fn<(pid: number) => Promise>()); +vi.mock("../utils/processTree.js", async (importOriginal) => ({ + ...(await importOriginal()), + terminateWindowsProcessTree: treeKill, +})); + +afterEach(() => { + treeKill.mockReset(); + vi.restoreAllMocks(); +}); + it("kills a browser that acknowledged close but never exits, so every caller's close() returns", async () => { const child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { stdio: "pipe" }); const browser = { process: () => child, close: () => once(child, "exit") } as unknown as Browser; @@ -18,3 +29,24 @@ it("kills a browser that acknowledged close but never exits, so every caller's c child.kill("SIGKILL"); } }); + +it("on Windows ends the browser's whole tree while its root still lives, since children outlive a killed root", async () => { + vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { stdio: "pipe" }); + let rootAliveAtTreeKill: boolean | undefined; + treeKill.mockImplementation(async () => { + rootAliveAtTreeKill = child.exitCode === null && child.signalCode === null; + }); + const browser = { process: () => child, close: () => once(child, "exit") } as unknown as Browser; + const puppeteer = { + launch: async (_options: LaunchOptions) => browser, + } as unknown as PuppeteerNode; + try { + const launched = await launchManagedBrowser(puppeteer, { headless: true }, { diagnose: false }); + await launched.close(); + expect(treeKill).toHaveBeenCalledWith(child.pid); + expect(rootAliveAtTreeKill).toBe(true); + } finally { + child.kill("SIGKILL"); + } +}); diff --git a/packages/cli/src/browser/launch.ts b/packages/cli/src/browser/launch.ts index db2d3799aa..84b7e362aa 100644 --- a/packages/cli/src/browser/launch.ts +++ b/packages/cli/src/browser/launch.ts @@ -17,7 +17,7 @@ import { } from "./gpuPolicy.js"; import { normalizeErrorMessage, notePageCrash } from "../utils/errorMessage.js"; import { requestCliExit } from "../utils/commandResult.js"; -import { processIsAlive } from "../utils/processTree.js"; +import { processIsAlive, terminateWindowsProcessTree } from "../utils/processTree.js"; import { chromeDescendants, reapChromeTree } from "./chromeTree.js"; import { watchChromeTree, type ChromeWatchdog } from "./windowsChromeWatchdog.js"; @@ -290,6 +290,13 @@ function closeGracefully(browser: Browser): Promise { return (gracefulClose.get(browser) ?? (() => browser.close()))(); } +async function killBrowser(child: ChildProcess): Promise { + if (process.platform === "win32" && child.pid !== undefined) { + await terminateWindowsProcessTree(child.pid).catch(() => undefined); + } + child.kill("SIGKILL"); +} + async function closedWithoutKill(browser: Browser): Promise { const child = browser.process(); if (!child) throw new Error("A managed browser lost its owned process."); @@ -309,7 +316,7 @@ async function closedWithoutKill(browser: Browser): Promise { grace, ]); const clean = child.exitCode !== null || child.signalCode !== null; - if (!clean) child.kill("SIGKILL"); + if (!clean) await killBrowser(child); clearTimeout(timer); const exitLimit = new Promise((_, reject) => { timer = setTimeout(