fix(engine): enable browser pool and deduplicate concurrent Chrome launches

packages/cli/src/server/studioServer.ts

@@ -113,10 +113,9 @@ async function reapplyStudioManualEditsToThumbnailPage(
   });
 }
 
-// ── Shared thumbnail browser (singleton per process) ────────────────────────
-// One browser instance is reused across all composition thumbnail requests.
-// Spawning a new Puppeteer process per request adds 2-5s overhead and causes
-// contention when the sidebar requests multiple thumbnails simultaneously.
+// ── Shared thumbnail browser (pool-backed) ──────────────────────────────────
+// Uses the engine's browser pool so the thumbnail browser and render workers
+// share a single Chrome process instead of running two independent ones.
 
 let _thumbnailBrowser: import("puppeteer-core").Browser | null = null;
 let _thumbnailBrowserInitializing: Promise<import("puppeteer-core").Browser | null> | null = null;
@@ -139,9 +138,7 @@ async function getThumbnailBrowser(): Promise<import("puppeteer-core").Browser |
         /* continue — acquireBrowser will try its own resolution */
       }
 
-      const acquired = await acquireBrowser(buildChromeArgs({ width: 1920, height: 1080 }), {
-        enableBrowserPool: false,
-      });
+      const acquired = await acquireBrowser(buildChromeArgs({ width: 1920, height: 1080 }));
       _thumbnailBrowser = acquired.browser;
       _thumbnailBrowser.on("disconnected", () => {
         _thumbnailBrowser = null;

packages/engine/src/config.ts

@@ -173,7 +173,7 @@ export const DEFAULT_CONFIG: EngineConfig = {
 
   disableGpu: false,
   browserGpuMode: "software",
-  enableBrowserPool: false,
+  enableBrowserPool: true,
   browserTimeout: 120_000,
   protocolTimeout: 300_000,
   forceScreenshot: false,

packages/engine/src/index.ts

@@ -49,6 +49,7 @@ export { resolveConfig, DEFAULT_CONFIG, type EngineConfig } from "./config.js";
 export {
   acquireBrowser,
   releaseBrowser,
+  drainBrowserPool,
   resolveHeadlessShellPath,
   resolveBrowserGpuMode,
   buildChromeArgs,

packages/engine/src/services/browserManager.test.ts

@@ -1,9 +1,16 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
+import type { Browser, PuppeteerNode } from "puppeteer-core";
+
 import {
   _resetAutoBrowserGpuModeCacheForTests,
+  _resetBrowserPoolForTests,
+  _setPuppeteerForTests,
+  acquireBrowser,
   buildChromeArgs,
+  drainBrowserPool,
   forceReleaseBrowser,
+  releaseBrowser,
   resolveBrowserGpuMode,
 } from "./browserManager.js";
 
@@ -157,3 +164,112 @@ describe("forceReleaseBrowser", () => {
     expect(disconnectFn).toHaveBeenCalled();
   });
 });
+
+describe("browser pool", () => {
+  function makeMockBrowser(): Browser {
+    return {
+      connected: true,
+      newPage: vi.fn(),
+      version: vi.fn().mockResolvedValue("HeadlessChrome/131.0.0.0"),
+      close: vi.fn().mockResolvedValue(undefined),
+      disconnect: vi.fn(),
+      process: () => ({ kill: vi.fn(), killed: false }),
+    } as unknown as Browser;
+  }
+
+  let launchFn: ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    _resetBrowserPoolForTests();
+    const mockBrowser = makeMockBrowser();
+    launchFn = vi.fn().mockResolvedValue(mockBrowser);
+    _setPuppeteerForTests({ launch: launchFn } as unknown as PuppeteerNode);
+  });
+
+  afterEach(async () => {
+    await drainBrowserPool();
+    _setPuppeteerForTests(undefined);
+  });
+
+  it("sequential acquires with pool enabled return the same browser", async () => {
+    const first = await acquireBrowser(["--no-sandbox"], { enableBrowserPool: true });
+    const second = await acquireBrowser(["--no-sandbox"], { enableBrowserPool: true });
+
+    expect(first.browser).toBe(second.browser);
+    expect(launchFn).toHaveBeenCalledTimes(1);
+
+    await releaseBrowser(first.browser, { enableBrowserPool: true });
+    await releaseBrowser(second.browser, { enableBrowserPool: true });
+  });
+
+  it("concurrent acquires via Promise.all trigger exactly one launch", async () => {
+    const [a, b, c] = await Promise.all([
+      acquireBrowser(["--no-sandbox"], { enableBrowserPool: true }),
+      acquireBrowser(["--no-sandbox"], { enableBrowserPool: true }),
+      acquireBrowser(["--no-sandbox"], { enableBrowserPool: true }),
+    ]);
+
+    expect(launchFn).toHaveBeenCalledTimes(1);
+    expect(a.browser).toBe(b.browser);
+    expect(b.browser).toBe(c.browser);
+
+    await releaseBrowser(a.browser, { enableBrowserPool: true });
+    await releaseBrowser(b.browser, { enableBrowserPool: true });
+    await releaseBrowser(c.browser, { enableBrowserPool: true });
+  });
+
+  it("pool recovers from a disconnected browser", async () => {
+    const first = await acquireBrowser(["--no-sandbox"], { enableBrowserPool: true });
+    await releaseBrowser(first.browser, { enableBrowserPool: true });
+
+    // Simulate Chrome crash
+    (first.browser as unknown as { connected: boolean }).connected = false;
+
+    const freshBrowser = makeMockBrowser();
+    launchFn.mockResolvedValue(freshBrowser);
+
+    const second = await acquireBrowser(["--no-sandbox"], { enableBrowserPool: true });
+    expect(second.browser).toBe(freshBrowser);
+    expect(second.browser).not.toBe(first.browser);
+    expect(launchFn).toHaveBeenCalledTimes(2);
+
+    await releaseBrowser(second.browser, { enableBrowserPool: true });
+  });
+
+  it("release at refCount 0 closes the browser", async () => {
+    const result = await acquireBrowser(["--no-sandbox"], { enableBrowserPool: true });
+    const closeFn = result.browser.close as ReturnType<typeof vi.fn>;
+
+    await releaseBrowser(result.browser, { enableBrowserPool: true });
+    expect(closeFn).toHaveBeenCalledTimes(1);
+  });
+
+  it("drainBrowserPool is safe to call when no browser is pooled", async () => {
+    await drainBrowserPool();
+  });
+
+  it("drainBrowserPool awaits in-flight launch before closing", async () => {
+    let resolveDeferred!: (browser: Browser) => void;
+    const deferred = new Promise<Browser>((resolve) => {
+      resolveDeferred = resolve;
+    });
+    launchFn.mockReturnValue(deferred);
+
+    // Start acquire — it will be pending
+    const acquirePromise = acquireBrowser(["--no-sandbox"], { enableBrowserPool: true });
+
+    // Drain while launch is in-flight
+    const drainPromise = drainBrowserPool();
+
+    // Resolve the pending launch
+    const mockBrowser = makeMockBrowser();
+    resolveDeferred(mockBrowser);
+
+    await drainPromise;
+    const closeFn = mockBrowser.close as ReturnType<typeof vi.fn>;
+    expect(closeFn).toHaveBeenCalled();
+
+    // The acquire should still resolve (the launch completed before drain closed it)
+    await acquirePromise.catch(() => {});
+  });
+});

packages/engine/src/services/browserManager.ts

@@ -73,6 +73,7 @@ export function resolveHeadlessShellPath(
 let pooledBrowser: Browser | null = null;
 let pooledBrowserRefCount = 0;
 let pooledCaptureMode: CaptureMode = "screenshot";
+let _pooledBrowserLaunchPromise: Promise<AcquiredBrowser> | null = null;
 
 // Preserve the producer-era export so re-export shims keep the same public API.
 export const ENABLE_BROWSER_POOL = DEFAULT_CONFIG.enableBrowserPool;
@@ -261,14 +262,52 @@ export async function acquireBrowser(
   const enablePool = config?.enableBrowserPool ?? DEFAULT_CONFIG.enableBrowserPool;
 
   if (enablePool && pooledBrowser) {
+    if (pooledBrowser.connected) {
+      pooledBrowserRefCount += 1;
+      return { browser: pooledBrowser, captureMode: pooledCaptureMode };
+    }
+    pooledBrowser = null;
+    pooledBrowserRefCount = 0;
+    _pooledBrowserLaunchPromise = null;
+  }
+
+  // Dedup concurrent launches: when the pool is enabled and multiple callers
+  // (e.g. parallel workers via Promise.all) race into acquireBrowser before
+  // the first launch completes, they would all see pooledBrowser === null and
+  // each spawn a separate Chrome. Cache the in-flight launch Promise so the
+  // second+ callers await the same one instead of launching again.
+  if (enablePool && _pooledBrowserLaunchPromise) {
+    const result = await _pooledBrowserLaunchPromise;
     pooledBrowserRefCount += 1;
-    return { browser: pooledBrowser, captureMode: pooledCaptureMode };
+    return result;
   }
 
-  // Config chromePath overrides env var / auto-detection.
+  const launchPromise = launchBrowser(chromeArgs, config);
+
+  if (enablePool) {
+    _pooledBrowserLaunchPromise = launchPromise;
+    try {
+      const result = await launchPromise;
+      pooledBrowser = result.browser;
+      pooledBrowserRefCount = 1;
+      pooledCaptureMode = result.captureMode;
+      return result;
+    } finally {
+      _pooledBrowserLaunchPromise = null;
+    }
+  }
+
+  return launchPromise;
+}
+
+async function launchBrowser(
+  chromeArgs: string[],
+  config?: Partial<
+    Pick<EngineConfig, "browserTimeout" | "protocolTimeout" | "chromePath" | "forceScreenshot">
+  >,
+): Promise<AcquiredBrowser> {
   const headlessShell = resolveHeadlessShellPath(config);
 
-  // BeginFrame requires chrome-headless-shell AND Linux (crashes on macOS/Windows).
   const isLinux = process.platform === "linux";
   const forceScreenshot = config?.forceScreenshot ?? DEFAULT_CONFIG.forceScreenshot;
   let captureMode: CaptureMode;
@@ -278,7 +317,6 @@ export async function acquireBrowser(
     captureMode = "beginframe";
     executablePath = headlessShell;
   } else {
-    // Screenshot mode with renderSeek: works on all platforms.
     captureMode = "screenshot";
     executablePath = headlessShell ?? undefined;
   }
@@ -295,11 +333,6 @@ export async function acquireBrowser(
     protocolTimeout,
   });
 
-  // Probe HeadlessExperimental.beginFrame — recent chrome-headless-shell
-  // builds (observed on 147) dropped the method while keeping the flags
-  // valid, so `--enable-begin-frame-control` leaves the compositor waiting
-  // for beginFrames the engine can no longer send. Auto-fall back to
-  // screenshot mode with the appropriate flags.
   if (captureMode === "beginframe") {
     const supported = await probeBeginFrameSupport(browser).catch(() => true);
     if (!supported) {
@@ -319,11 +352,6 @@ export async function acquireBrowser(
     }
   }
 
-  if (enablePool) {
-    pooledBrowser = browser;
-    pooledBrowserRefCount = 1;
-    pooledCaptureMode = captureMode;
-  }
   return { browser, captureMode };
 }
 
@@ -341,6 +369,7 @@ export async function releaseBrowser(
     if (pooledBrowserRefCount === 0) {
       await browser.close().catch(() => {});
       pooledBrowser = null;
+      _pooledBrowserLaunchPromise = null;
     }
     return;
   }
@@ -351,6 +380,7 @@ export function forceReleaseBrowser(browser: Browser): void {
   if (pooledBrowser && pooledBrowser === browser) {
     pooledBrowserRefCount = 0;
     pooledBrowser = null;
+    _pooledBrowserLaunchPromise = null;
   }
   const proc = (
     browser as unknown as {
@@ -371,6 +401,40 @@ export function forceReleaseBrowser(browser: Browser): void {
   }
 }
 
+/**
+ * Forcefully close the pooled browser if one exists, regardless of refCount.
+ * Used for explicit cleanup at process exit or between independent render jobs
+ * that should not share browser state.
+ */
+export async function drainBrowserPool(): Promise<void> {
+  // Await any in-flight launch first — otherwise the launch resolves after we
+  // drain and produces a browser that nobody references (orphan).
+  const pending = _pooledBrowserLaunchPromise;
+  _pooledBrowserLaunchPromise = null;
+  if (pending) {
+    await pending.then((r) => r.browser.close()).catch(() => {});
+  }
+  if (pooledBrowser) {
+    const browser = pooledBrowser;
+    pooledBrowser = null;
+    pooledBrowserRefCount = 0;
+    await browser.close().catch(() => {});
+  }
+}
+
+/** Test-only: reset all pool state. */
+export function _resetBrowserPoolForTests(): void {
+  pooledBrowser = null;
+  pooledBrowserRefCount = 0;
+  pooledCaptureMode = "screenshot";
+  _pooledBrowserLaunchPromise = null;
+}
+
+/** Test-only: inject a mock PuppeteerNode so tests bypass the dynamic import. */
+export function _setPuppeteerForTests(mock: PuppeteerNode | undefined): void {
+  _puppeteer = mock;
+}
+
 export interface BuildChromeArgsOptions {
   width: number;
   height: number;

packages/producer/src/services/browserManager.ts

@@ -5,6 +5,7 @@
 export {
   acquireBrowser,
   releaseBrowser,
+  drainBrowserPool,
   resolveHeadlessShellPath,
   buildChromeArgs,
   ENABLE_BROWSER_POOL,