From edba577e7a092595d18fc7ebe1a9395e0a6f3b25 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 16 Jul 2026 21:41:44 +0000 Subject: [PATCH] fix(utils): bounded default ptree stderr retention Default ChildProcess unconditionally pushed every raw stderr chunk into `#stderrChunks`, so long-lived noisy subprocesses (LSP/DAP/RPC) grew OMP memory linearly despite the 32 KiB visible tail cap. - Allocate `#stderrChunks` only when full capture is requested at spawn. - Decouple retention from stream exposure via `spawnInternal`, so `exec({ stderr: "full" })` retains without an unused live tee. - Reject retroactive `wait({ stderr: "full" })` on a default child with a clear error instead of returning truncated data. Fixes #5759 --- packages/utils/CHANGELOG.md | 4 ++ packages/utils/src/ptree.ts | 34 +++++++--- packages/utils/test/ptree-stderr.test.ts | 84 ++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 9 deletions(-) create mode 100644 packages/utils/test/ptree-stderr.test.ts diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 1bb87b599..f708d00a8 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Bounded default `ptree.ChildProcess` stderr retention to the existing 32 KiB tail instead of retaining every raw chunk; long-lived subprocesses (LSP/DAP/RPC) no longer grow OMP memory with their stderr volume. Full capture must now be selected at spawn time via `spawn(cmd, { stderr: "full" })` / `exec(cmd, { stderr: "full" })`, and a retroactive `wait({ stderr: "full" })` on a default child throws instead of returning truncated data ([#5759](https://github.com/can1357/oh-my-pi/issues/5759)). + ## [17.0.1] - 2026-07-16 ### Fixed diff --git a/packages/utils/src/ptree.ts b/packages/utils/src/ptree.ts index 528f5dc30..472847a70 100644 --- a/packages/utils/src/ptree.ts +++ b/packages/utils/src/ptree.ts @@ -72,6 +72,7 @@ export class TimeoutError extends AbortError { export interface WaitOptions { allowNonZero?: boolean; allowAbort?: boolean; + /** `full` requires upfront capture; `exec` enables it, while direct `spawn` callers pass `stderr: "full"`. */ stderr?: "full" | "buffer"; } @@ -97,7 +98,7 @@ export interface ExecResult { export class ChildProcess { #nothrow = false; #stderrTail = ""; - #stderrChunks: Uint8Array[] = []; + #stderrChunks?: Uint8Array[]; #exitReason?: Exception; #exitReasonPending?: Exception; #stderrDone: Promise; @@ -107,8 +108,10 @@ export class ChildProcess { constructor( readonly proc: PipedSubprocess, readonly exposeStderr: boolean, + retainFullStderr = exposeStderr, ) { - // Eagerly drain stderr into a truncated tail string + raw chunks. + if (retainFullStderr) this.#stderrChunks = []; + // Eagerly drain stderr into a truncated tail, retaining raw chunks only for explicit full capture. const dec = new TextDecoder(); const trim = () => { if (this.#stderrTail.length > NonZeroExitError.MAX_TRACE) @@ -123,7 +126,7 @@ export class ChildProcess { this.#stderrDone = (async () => { try { for await (const chunk of stderrStream) { - this.#stderrChunks.push(chunk); + this.#stderrChunks?.push(chunk); this.#stderrTail += dec.decode(chunk, { stream: true }); trim(); } @@ -259,11 +262,15 @@ export class ChildProcess { async wait(opts?: WaitOptions): Promise { const { allowNonZero = false, allowAbort = false, stderr: stderrMode = "buffer" } = opts ?? {}; + const stderrChunks = this.#stderrChunks; + if (stderrMode === "full" && !stderrChunks) { + throw new Error('Full stderr capture must be requested when spawning the process (pass stderr: "full")'); + } const stdoutP = new Response(this.stdout).text(); const stderrP = - stderrMode === "full" - ? this.#stderrDone.then(() => new TextDecoder().decode(Buffer.concat(this.#stderrChunks))) + stderrMode === "full" && stderrChunks + ? this.#stderrDone.then(() => new TextDecoder().decode(Buffer.concat(stderrChunks))) : this.#stderrDone.then(() => this.#stderrTail); const [stdout, stderr] = await Promise.all([stdoutP, stderrP]); @@ -328,11 +335,15 @@ type ChildSpawnOptions = Omit< > & { signal?: AbortSignal; detached?: boolean; + /** Expose and retain complete stderr for a later `wait({ stderr: "full" })`. */ stderr?: "full" | null; }; -/** Spawn a child process with piped stdout/stderr. */ -export function spawn(cmd: string[], opts?: ChildSpawnOptions): ChildProcess { +function spawnInternal( + cmd: string[], + opts: ChildSpawnOptions | undefined, + retainFullStderr: boolean, +): ChildProcess { const { timeout = -1, signal, stderr, ...rest } = opts ?? {}; const child = Bun.spawn(cmd, { stdin: "ignore", @@ -341,12 +352,17 @@ export function spawn(cmd: string[], opts?: ChildSpa windowsHide: true, ...rest, }); - const cp = new ChildProcess(child, stderr === "full"); + const cp = new ChildProcess(child, stderr === "full", retainFullStderr); if (signal) cp.attachSignal(signal); if (timeout > 0) cp.attachTimeout(timeout); return cp; } +/** Spawn a child process with piped stdout/stderr. */ +export function spawn(cmd: string[], opts?: ChildSpawnOptions): ChildProcess { + return spawnInternal(cmd, opts, opts?.stderr === "full"); +} + /** Options for exec. */ export interface ExecOptions extends Omit, WaitOptions { input?: string | Buffer | Uint8Array; @@ -357,7 +373,7 @@ export async function exec(cmd: string[], opts?: ExecOptions): Promise { + it("requires full stderr capture to be selected before spawning", async () => { + using child = spawn(stderrFixture(LARGE_STDERR_SIZE)); + await child.exited; + + let captureError: unknown; + try { + await child.wait({ stderr: "full" }); + } catch (caught) { + captureError = caught; + } + expect(captureError).toBeInstanceOf(Error); + if (!(captureError instanceof Error)) throw new Error("Expected full capture error"); + expect(captureError.message).toContain(FULL_CAPTURE_ERROR); + const result = await child.wait(); + + expect(result.stderr.length).toBe(STDERR_LIMIT); + expect(result.stderr).not.toContain(STDERR_HEAD); + expect(result.stderr).toEndWith(STDERR_TAIL); + expect(child.peekStderr()).toBe(result.stderr); + }); + + it("preserves complete stderr for explicit exec capture", async () => { + const result = await exec(stderrFixture(LARGE_STDERR_SIZE, 0, "stdout-ok"), { stderr: "full" }); + + expect(result.stdout).toBe("stdout-ok"); + expect(result.stderr.length).toBe(LARGE_STDERR_SIZE); + expect(result.stderr).toStartWith(STDERR_HEAD); + expect(result.stderr).toEndWith(STDERR_TAIL); + }); + + it("preserves the live stream and retained stderr for explicit spawn capture", async () => { + const size = STDERR_LIMIT * 4; + using child = spawn(stderrFixture(size, 0, "spawn-ok"), { stderr: "full" }); + const stderrStream = child.stderr; + if (!stderrStream) throw new Error("Expected exposed stderr stream"); + + const streamedStderr = new Response(stderrStream).text(); + const [result, streamed] = await Promise.all([child.wait({ stderr: "full" }), streamedStderr]); + + expect(result.stdout).toBe("spawn-ok"); + expect(result.stderr.length).toBe(size); + expect(result.stderr).toStartWith(STDERR_HEAD); + expect(result.stderr).toEndWith(STDERR_TAIL); + expect(streamed).toBe(result.stderr); + }); + + it("keeps peek and nonzero errors on the bounded stderr tail", async () => { + using child = spawn(stderrFixture(STDERR_LIMIT * 4, 7)); + let error: unknown; + try { + await child.exitedCleanly; + } catch (caught) { + error = caught; + } + + expect(error).toBeInstanceOf(NonZeroExitError); + if (!(error instanceof NonZeroExitError)) throw new Error("Expected NonZeroExitError"); + expect(error.stderr.length).toBe(STDERR_LIMIT); + expect(error.stderr).not.toContain(STDERR_HEAD); + expect(error.stderr).toEndWith(STDERR_TAIL); + expect(error.message).toContain(STDERR_TAIL); + expect(child.peekStderr()).toBe(error.stderr); + }); +});