From 7cab51cc2856ba8627d211d718c053baa3c6c7ad Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Mon, 15 Jun 2026 01:23:23 +0800 Subject: [PATCH] fix(runtime): always unmount the tree in renderToString so a paint throw can't leak listeners (#186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderToString mounts the Vue tree, then lays out and paints, then unmounts. The app.unmount() sat inside the try AFTER paint, so when layout/paint threw (e.g. a whose transformer throws during the paint phase) control jumped to the outer finally, which only freed yoga — Vue never tore down, so onScopeDispose never ran. Any composable that registered an external listener then leaked it: useWindowSize attaches a `resize` listener to the shared process.stdout (the no-op AppContext's stdout) and only removes it via onScopeDispose, so each failed renderToString leaked one listener, accumulating toward Node's MaxListenersExceededWarning. Fix: track that mount succeeded and, in the outer finally, run app.unmount() when `mounted && !teardownSucceeded` (best-effort, in try/catch, before the yoga free). The happy path is unaffected (it already unmounted; teardownSucceeded short-circuits the fallback). The error-path unmount frees child yoga nodes and runs onScopeDispose cleanups; freeRecursive then frees the root. The original paint error still propagates (the fallback teardown can't mask it). useWindowSize is intentionally unchanged — the unmount-in-finally is the general fix and also covers any other external listener a tree registers. Test (sequential — asserts on the process-global process.stdout resize listener count): three renderToString calls whose paint throws leak zero `resize` listeners after the fix (3 before). Co-authored-by: Claude Opus 4.8 (1M context) --- .../window-size.sequential.test.tsx | 80 +++++++++++++++++-- packages/runtime/src/render-to-string.ts | 24 +++++- 2 files changed, 96 insertions(+), 8 deletions(-) diff --git a/packages/runtime-tests/integration/composables/window-size.sequential.test.tsx b/packages/runtime-tests/integration/composables/window-size.sequential.test.tsx index 1b4e062..a674600 100644 --- a/packages/runtime-tests/integration/composables/window-size.sequential.test.tsx +++ b/packages/runtime-tests/integration/composables/window-size.sequential.test.tsx @@ -1,13 +1,19 @@ -// Sequential: mutates process-global state — process.env.COLUMNS/LINES and -// process.stdout/process.stderr columns+rows. The terminal-size package (used by -// resolveSize's fallback) reads these globals directly, so a concurrent sibling -// would perturb the result. Tests restore every mutated prop in a finally block. +// Sequential: mutates / asserts on process-global state — +// • process.env.COLUMNS/LINES and process.stdout/process.stderr columns+rows +// (the terminal-size package, used by resolveSize's fallback, reads these +// globals directly, so a concurrent sibling would perturb the result), and +// • process.stdout.listenerCount("resize") — the renderToString teardown-leak +// test below mounts useWindowSize against the SHARED process.stdout (the +// no-op AppContext's stdout) and asserts the "resize" listener count returns +// to baseline; a concurrent sibling that also touches process.stdout would +// make that count flaky. +// Tests restore every mutated prop in a finally block. import { PassThrough } from "node:stream"; import process from "node:process"; import { defineComponent } from "vue"; import { expect, test } from "vite-plus/test"; -import { createApp, Text, useWindowSize } from "@vue-tui/runtime"; +import { createApp, renderToString, Text, Transform, useWindowSize } from "@vue-tui/runtime"; function makeTtyStream(columns: number): NodeJS.WriteStream { const s = new PassThrough() as unknown as NodeJS.WriteStream; @@ -84,3 +90,67 @@ test.sequential("useWindowSize falls back to terminal-size rows from env.LINES w process.stderr.rows = originalStderrRows; } }); + +// renderToString's no-op AppContext uses the REAL shared process.stdout (so +// useWindowSize attaches its `resize` listener there). If layout/paint throws, +// the happy-path app.unmount() is skipped — without an unmount in the outer +// finally, onScopeDispose never runs and the `resize` listener leaks ONE per +// failed call, accumulating toward Node's MaxListenersExceededWarning. A +// throwing transform runs during the PAINT phase, so it reproduces +// the layout/paint-phase throw exactly. (Asserts on the shared +// process.stdout listener count — hence this sequential file.) +test.sequential("renderToString does not leak useWindowSize's resize listener when paint throws", () => { + const Leaky = defineComponent(() => { + // Registers a `resize` listener on ctx.stdout (process.stdout) via + // onScopeDispose; only an unmount tears it down. + useWindowSize(); + return () => ( + // The transform runs during paint, so it throws AFTER app.mount() succeeded. + { + throw new Error("paint boom"); + }} + > + boom + + ); + }); + + const before = process.stdout.listenerCount("resize"); + + let threwCount = 0; + for (let i = 0; i < 3; i++) { + try { + renderToString(Leaky); + } catch { + // renderToString rethrows the paint error after cleanup — expected. + threwCount++; + } + } + + const after = process.stdout.listenerCount("resize"); + + // The error path must actually fire (otherwise we'd be testing the happy path). + expect(threwCount).toBe(3); + // No net listeners leaked across the three failed calls. + expect(after).toBe(before); +}); + +// Control: the CLEAN (non-throwing) renderToString path already unmounts and +// runs onScopeDispose, so it leaks zero resize listeners. Proves the harness is +// sound — the assertion above is meaningful only because this one passes too. +test.sequential("renderToString clean path leaks no useWindowSize resize listener", () => { + const Clean = defineComponent(() => { + useWindowSize(); + return () => clean; + }); + + const before = process.stdout.listenerCount("resize"); + for (let i = 0; i < 3; i++) { + const output = renderToString(Clean); + expect(output).toBe("clean"); + } + const after = process.stdout.listenerCount("resize"); + + expect(after).toBe(before); +}); diff --git a/packages/runtime/src/render-to-string.ts b/packages/runtime/src/render-to-string.ts index e9df454..960b4e3 100644 --- a/packages/runtime/src/render-to-string.ts +++ b/packages/runtime/src/render-to-string.ts @@ -144,10 +144,12 @@ function renderToStringInternal( }; let teardownSucceeded = false; + let mounted = false; try { // Synchronously render the Vue tree into the root. app.mount(root); + mounted = true; const restoreLayoutGuards = calculateLayoutWithContentGuards( root, @@ -198,12 +200,28 @@ function renderToStringInternal( return normalizedStaticOutput || output; } finally { + // If layout/paint threw, the happy-path app.unmount() above was skipped. Unmount + // here so the tree's onScopeDispose cleanups ALWAYS run — otherwise a composable + // that registered an external listener (e.g. useWindowSize's `resize` listener on + // the shared process.stdout) leaks one per failed call, accumulating toward Node's + // MaxListenersExceededWarning. Guard with `mounted` (never unmount a tree that + // never mounted) and `teardownSucceeded` (the happy path already unmounted — no + // double-unmount). app.unmount() also frees the CHILD yoga nodes via the node-ops + // remove handler, leaving only the root for freeRecursive below. + if (mounted && !teardownSucceeded) { + try { + app.unmount(); + } catch { + // Best-effort teardown: a throw here must not mask the original error. + } + } + // Ensure native yoga memory is freed even if rendering or teardown threw. - // Yoga nodes are WASM-backed and not garbage collected. + // Yoga nodes are WASM-backed and not garbage collected. In the happy path + // detachYoga(root) already freed the root; here (error path) freeRecursive + // cleans up the root and any child nodes the unmount above couldn't free. if (!teardownSucceeded) { try { - // If unmount failed, some child nodes may not have been freed. - // Use freeRecursive to clean up the entire tree as best-effort. root.yoga.freeRecursive(); } catch { // Best-effort: node may already be partially freed