diff --git a/packages/cli/src/commands/preview.ts b/packages/cli/src/commands/preview.ts index cafafc4752..84cf93d519 100644 --- a/packages/cli/src/commands/preview.ts +++ b/packages/cli/src/commands/preview.ts @@ -1537,7 +1537,11 @@ async function runEmbeddedMode( // Compute everything that may throw before acquiring the fs.watch handle. // Once createStudioServer returns, every subsequent exit path must close it. const serverBuildSignature = await loadPreviewServerBuildSignature(); - const { app, watcher } = createStudioServer({ + const { + app, + watcher, + shutdown: shutdownStudio, + } = createStudioServer({ projectDir: dir, projectName: pName, autoProxy: options?.autoProxy, @@ -1653,16 +1657,7 @@ async function runEmbeddedMode( // can't be blocked by a stuck drainBrowserPool(). setTimeout(() => requestCliExit(0), 3000).unref(); - // Kill ffmpeg first (sync, fast), then drain browsers (async, slower). - const cleanup = async () => { - const { closeThumbnailBrowser } = await import("../server/studioServer.js"); - const { drainBrowserPool, killTrackedProcesses } = await import("@hyperframes/engine"); - killTrackedProcesses(); - await closeThumbnailBrowser().catch(() => {}); - await drainBrowserPool().catch(() => {}); - }; - - cleanup() + shutdownStudio() .catch(() => {}) .finally(() => { watcher.close(); diff --git a/packages/cli/src/server/studioServer.test.ts b/packages/cli/src/server/studioServer.test.ts index c53288e78e..da9890b56f 100644 --- a/packages/cli/src/server/studioServer.test.ts +++ b/packages/cli/src/server/studioServer.test.ts @@ -9,6 +9,53 @@ import { loadRuntimeSource } from "./runtimeSource.js"; import { findFFmpeg, findFFprobe } from "../browser/ffmpeg.js"; import { createStudioServer, type StudioServer } from "./studioServer.js"; +// Forces loadStudioProducer() down its production import branch (real +// isDevMode() is true for a .ts test file, which instead throws a +// "requires bun" error before ever reaching executeRenderJob — see +// studioServer.ts's loadStudioProducer). Only startRender reads this. +vi.mock("../utils/env.js", () => ({ isDevMode: () => false })); + +const producerState = vi.hoisted(() => ({ + // Set per-test to control when the render "finishes" so a shutdown that + // races an in-flight render is observable instead of vacuous. + executeRenderJob: ( + _job: unknown, + _dir: string, + _outputPath: string, + _onProgress: unknown, + _signal: AbortSignal, + ): Promise => Promise.resolve(), +})); +vi.mock("@hyperframes/producer", () => ({ + createRenderJob: (opts: Record) => ({ ...opts, perfSummary: undefined }), + executeRenderJob: (...args: Parameters) => + producerState.executeRenderJob(...args), +})); +const engineState = vi.hoisted(() => ({ + acquireBrowser: async (..._args: unknown[]): Promise => { + throw new Error("acquireBrowser called without a test double"); + }, +})); +vi.mock("@hyperframes/engine", () => ({ + acquireBrowser: (...args: unknown[]) => engineState.acquireBrowser(...args), + buildChromeArgs: () => [], + killTrackedProcesses: () => {}, + drainBrowserPool: async () => {}, +})); +vi.mock("../browser/gpuPolicy.js", () => ({ + resolveCaptureBrowserGpuMode: async () => "software", + resolveLocalBrowserGpuMode: () => "software", + compositionRequiresWebGpu: () => false, + assertWebGpuAdapterAvailable: async () => {}, +})); +vi.mock("../browser/preflight.js", async (importOriginal) => ({ + ...(await importOriginal()), + resolveRenderBrowser: async () => ({ executablePath: "/fake/chrome", source: "system" }), +})); +vi.mock("../browser/manager.js", () => ({ + ensureBrowser: async () => ({ executablePath: undefined, source: "system" }), +})); + // Only `fs.watch` is replaced, so the SSE describe below can fire a file-change // on demand; every other server test keeps reading and writing real files. const mockWatcher = new EventEmitter() as EventEmitter & { close: () => void }; @@ -106,6 +153,156 @@ describe("createStudioServer autoProxy plumbing", () => { }); }); +// A render that never reaches the executor (browser check refused, import failed) must fail with +// its reason, not hang until the suite timeout. +async function untilStarted(started: Promise, state: { status: string; error?: string }) { + let timer: ReturnType | undefined; + const never = new Promise((_, reject) => { + timer = setTimeout( + () => + reject( + new Error( + `render never reached executeRenderJob: status=${state.status} error=${state.error}`, + ), + ), + 5_000, + ); + }); + try { + await Promise.race([started, never]); + } finally { + clearTimeout(timer); + } +} + +describe("createStudioServer shutdown", () => { + function startRenderOpts(jobId: string, outputPath: string) { + return { + project: { id: "demo", dir: tmpProject(), title: "demo" }, + outputPath, + format: "mp4" as const, + fps: { num: 30, den: 1 }, + quality: "draft", + jobId, + }; + } + + it("cancels an in-flight render's signal and waits for it before draining the browser pool", async () => { + const events: string[] = []; + let started!: () => void; + const startedPromise = new Promise((resolve) => { + started = resolve; + }); + producerState.executeRenderJob = (_job, _dir, _outputPath, _onProgress, signal) => { + started(); + return new Promise((_resolve, reject) => { + const onAbort = () => + setTimeout(() => { + events.push("render-settled"); + reject(Object.assign(new Error("cancelled"), { name: "AbortError" })); + }, 20); + // A signal aborted before this executor ran would never fire a later + // "abort" listener (edge-triggered, not level-triggered) — check the + // already-aborted case too, same as real capture code must. + if (signal.aborted) onAbort(); + else signal.addEventListener("abort", onAbort); + }); + }; + + server = createStudioServer({ projectDir: tmpProject() }); + const outputPath = join(tmpdir(), "shutdown-render.mp4"); + const state = server.adapter.startRender(startRenderOpts("job-1", outputPath)); + expect(state.status).toBe("rendering"); + + // Wait until the render has actually reached executeRenderJob (several + // microtask hops through loadStudioProducer/ensureBrowser) before racing + // it against shutdown, or shutdown could abort a signal nothing is + // listening on yet — a race in this test, not in the fix under test. + await untilStarted(startedPromise, state); + + await server.shutdown(); + events.push("drain-and-shutdown-returned"); + + expect(events).toEqual(["render-settled", "drain-and-shutdown-returned"]); + }); + + it("refuses a render started after shutdown has begun instead of launching a fresh browser", async () => { + let releaseFirstRender: () => void = () => {}; + let started!: () => void; + const startedPromise = new Promise((resolve) => { + started = resolve; + }); + producerState.executeRenderJob = () => { + started(); + return new Promise((resolve) => { + releaseFirstRender = resolve; + }); + }; + + server = createStudioServer({ projectDir: tmpProject() }); + const first = server.adapter.startRender(startRenderOpts("job-1", join(tmpdir(), "a.mp4"))); + await untilStarted(startedPromise, first); + + const shutdownPromise = server.shutdown(); + // shuttingDown is set synchronously as shutdown()'s first statement, so a + // render request arriving anywhere after that call has been made (even + // before it resolves) must already see it. + const late = server.adapter.startRender(startRenderOpts("job-2", join(tmpdir(), "b.mp4"))); + + expect(late.status).toBe("failed"); + expect(late.error).toMatch(/shutting down/i); + + releaseFirstRender(); + await shutdownPromise; + }); + + const thumbnailOpts = () => ({ + project: { id: "demo", dir: tmpProject(), title: "demo" }, + compPath: "index.html", + seekTime: 0.5, + width: 640, + height: 360, + outputWidth: 640, + outputHeight: 360, + previewUrl: "http://localhost/preview", + signal: new AbortController().signal, + }); + + it("does not launch a browser for a thumbnail request after shutdown has begun", async () => { + const acquire = vi.fn(); + engineState.acquireBrowser = acquire; + server = createStudioServer({ projectDir: tmpProject() }); + await server.shutdown(); + + await expect(server.adapter.generateThumbnail?.(thumbnailOpts())).resolves.toBeNull(); + expect(acquire).not.toHaveBeenCalled(); + }); + + it("releases a thumbnail browser that finished launching after shutdown began", async () => { + const release = vi.fn(async () => {}); + let launched!: () => void; + const launchedPromise = new Promise((resolve) => (launched = resolve)); + let finishLaunch: () => void = () => {}; + engineState.acquireBrowser = async () => { + launched(); + await new Promise((resolve) => (finishLaunch = resolve)); + return { browser: new EventEmitter(), release }; + }; + server = createStudioServer({ projectDir: tmpProject() }); + const thumbnail = server.adapter.generateThumbnail?.(thumbnailOpts()); + await launchedPromise; + + const shutdown = server.shutdown(); + // Let shutdown reach the browser close while the launch is still pending. + await new Promise((resolve) => setTimeout(resolve, 50)); + finishLaunch(); + await shutdown; + + await expect(thumbnail).resolves.toBeNull(); + expect(release).toHaveBeenCalledTimes(1); + }); +}); + describe("Studio project lint endpoint", () => { it("surfaces findings that require the complete project graph", async () => { const projectDir = tmpProject(); diff --git a/packages/cli/src/server/studioServer.ts b/packages/cli/src/server/studioServer.ts index 1dd6ecfac6..a43b571cb3 100644 --- a/packages/cli/src/server/studioServer.ts +++ b/packages/cli/src/server/studioServer.ts @@ -223,6 +223,7 @@ interface ThumbnailBrowserSession { async function getThumbnailBrowser( requestedGpuMode: BrowserGpuMode, + isShuttingDown: () => boolean, ): Promise { if ( _thumbnailBrowserLease?.browser.connected && @@ -242,6 +243,7 @@ async function getThumbnailBrowser( _thumbnailBrowserInitializing = (async () => { try { + if (isShuttingDown()) return null; const { ensureBrowser } = await import("../browser/manager.js"); const { acquireBrowser, buildChromeArgs } = await import("@hyperframes/engine"); let executablePath: string | undefined; @@ -286,7 +288,12 @@ async function getThumbnailBrowser( return _thumbnailBrowserInitializing; } -export async function closeThumbnailBrowser(): Promise { +async function closeThumbnailBrowser(): Promise { + // A launch kicked off just before this call is not yet reflected in + // _thumbnailBrowserLease; awaiting it here is what lets shutdown() close a + // browser that was mid-launch when the stop signal arrived, instead of + // leaving it to finish launching, unreferenced, after the process exits. + if (_thumbnailBrowserInitializing) await _thumbnailBrowserInitializing.catch(() => {}); if (!_thumbnailBrowserLease) return; const lease = _thumbnailBrowserLease; _thumbnailBrowserLease = null; @@ -315,6 +322,8 @@ export interface StudioServerOptions { export interface StudioServer { app: Hono; watcher: ProjectWatcher; + /** Cancels in-flight renders, then closes every browser this server owns. */ + shutdown(): Promise; /** Exposed for tests: the adapter handed to the shared studio API (carries * the resolved `autoProxy` flag the preview routes read). */ adapter: PreviewApiAdapter; @@ -389,6 +398,12 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { } }); + const inFlightRenders = new Map>(); + // Set synchronously by shutdown() before any await, so a render or + // thumbnail request already queued behind it sees the flag instead of + // launching a browser shutdown() has no way to know about and close. + let shuttingDown = false; + const adapter: PreviewApiAdapter = { // Explicit option wins (preview's resolved --proxy/--no-proxy + config); // otherwise honor the project's hyperframes.json media.autoProxy so every @@ -467,6 +482,15 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { rendersDir: () => join(projectDir, "renders"), startRender(opts): RenderJobState { + if (shuttingDown) { + return { + id: opts.jobId, + status: "failed", + progress: 0, + outputPath: opts.outputPath, + error: "Studio server is shutting down", + }; + } // The render POST is a request boundary like any other. Without this an // already-open Studio tab keeps rendering under the posture cached when // the server booted. @@ -482,7 +506,7 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { // Run render asynchronously, mutating the state object const startTime = Date.now(); - (async () => { + const run = (async () => { let renderJob: RenderJob | undefined; const removeCancelledOutput = () => { // User-initiated cancel: not a failure. Remove any output so the @@ -569,6 +593,9 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { } } })(); + inFlightRenders.set(abortController, run); + const forget = () => void inFlightRenders.delete(abortController); + run.then(forget, forget); return state; }, @@ -584,9 +611,11 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { }, async generateThumbnail(opts): Promise { - const session = await getThumbnailBrowser(browserGpuMode); + const session = await getThumbnailBrowser(browserGpuMode, () => shuttingDown); if (!session) { - console.warn("[Studio] Thumbnail: no browser available — Chrome may not be installed"); + if (!shuttingDown) { + console.warn("[Studio] Thumbnail: no browser available — Chrome may not be installed"); + } return null; } const sourcePath = join(opts.project.dir, opts.compPath); @@ -991,5 +1020,16 @@ export function createStudioServer(options: StudioServerOptions): StudioServer { return c.html(html, 200, { "Cache-Control": "no-cache" }); }); - return { app, watcher, adapter }; + const shutdown = async (): Promise => { + shuttingDown = true; + const renders = [...inFlightRenders]; + for (const [abortController] of renders) abortController.abort(); + const { killTrackedProcesses, drainBrowserPool } = await import("@hyperframes/engine"); + killTrackedProcesses(); + await Promise.allSettled(renders.map(([, done]) => done)); + await closeThumbnailBrowser().catch(() => {}); + await drainBrowserPool().catch(() => {}); + }; + + return { app, watcher, adapter, shutdown }; }