mirror of https://github.com/garrytan/gstack.git
fix(bun-polyfill): cap spawned-pipe buffer to prevent OOM
Codex review pass 3 flagged a [P2]: the unbounded eager-drain buffer defeats the consumer's existing readCapped at the call site. A runaway child that prints hundreds of MB before timing out now balloons server memory regardless of the consumer's documented 1 MB cap. Cap the per-pipe drain at 16 MB by default (GSTACK_SPAWN_MAX_BUFFER overrides), generous for every real use case (DPAPI ~32B, tasklist <1KB, codex review ~100KB) and small enough to bound worst-case memory. Past the cap, the pipe keeps flowing so the child never blocks in write(), but further bytes are discarded. Consumers reading the replay stream see exactly MAX_BUFFER bytes; the existing readCapped at browser-skill- commands then applies its own 1 MB cap on top with no surprises. Regression test sets GSTACK_SPAWN_MAX_BUFFER=1024, has the child write 10 KB, asserts captured stdout is exactly 1024 bytes and exit resolves cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
899b3be30f
commit
33143ad923
|
|
@ -99,11 +99,32 @@ globalThis.Bun = {
|
|||
// (~16-64 KB) back-pressures the child until it blocks in write() and
|
||||
// `exit` never fires. Eager draining keeps the pipes flowing regardless
|
||||
// of read order; replay below is via fresh Web ReadableStreams.
|
||||
//
|
||||
// Cap the buffer so a runaway child can't OOM the server. 16 MB is
|
||||
// generous: DPAPI outputs are tiny, tasklist is <1 KB, and the
|
||||
// browser-skill consumer has its own 1 MB readCapped. Once the cap is
|
||||
// reached we keep draining the pipe (so the child never blocks) but
|
||||
// discard further bytes. Override via GSTACK_SPAWN_MAX_BUFFER (bytes).
|
||||
const MAX_BUFFER = Math.max(
|
||||
0,
|
||||
parseInt(process.env.GSTACK_SPAWN_MAX_BUFFER || '', 10) || 16 * 1024 * 1024,
|
||||
);
|
||||
const drain = (stream) => {
|
||||
if (!stream) return { done: Promise.resolve(), chunks: [] };
|
||||
const chunks = [];
|
||||
if (!stream) return { done: Promise.resolve(), chunks: [], truncated: false };
|
||||
const state = { chunks: [], bytes: 0, truncated: false };
|
||||
const done = new Promise((resolve) => {
|
||||
stream.on('data', (chunk) => chunks.push(chunk));
|
||||
stream.on('data', (chunk) => {
|
||||
if (state.bytes >= MAX_BUFFER) { state.truncated = true; return; }
|
||||
if (state.bytes + chunk.length <= MAX_BUFFER) {
|
||||
state.chunks.push(chunk);
|
||||
state.bytes += chunk.length;
|
||||
} else {
|
||||
const remaining = MAX_BUFFER - state.bytes;
|
||||
state.chunks.push(chunk.subarray(0, remaining));
|
||||
state.bytes = MAX_BUFFER;
|
||||
state.truncated = true;
|
||||
}
|
||||
});
|
||||
// Any terminal event resolves: 'end' on normal close, 'error' on a
|
||||
// stream-level error, 'close' as the belt-and-suspenders for spawn
|
||||
// failures where Node fires 'close' but neither 'end' nor 'error'.
|
||||
|
|
@ -111,7 +132,7 @@ globalThis.Bun = {
|
|||
stream.once('error', resolve);
|
||||
stream.once('close', resolve);
|
||||
});
|
||||
return { done, chunks };
|
||||
return { done, chunks: state.chunks };
|
||||
};
|
||||
const stdoutDrain = drain(proc.stdout);
|
||||
const stderrDrain = drain(proc.stderr);
|
||||
|
|
|
|||
|
|
@ -121,6 +121,32 @@ describe('bun-polyfill', () => {
|
|||
expect(out).toMatch(/^exit:\d+$/);
|
||||
});
|
||||
|
||||
// GSTACK_SPAWN_MAX_BUFFER caps the drain so a runaway child can't OOM the
|
||||
// server. Past the cap, the pipe keeps flowing (child doesn't block) but
|
||||
// further bytes are dropped. Set a small cap, write more than that, assert
|
||||
// the captured stdout equals the cap and the child exits cleanly.
|
||||
test('Bun.spawn caps buffered output at GSTACK_SPAWN_MAX_BUFFER', async () => {
|
||||
const result = Bun.spawnSync(['node', '-e', `
|
||||
process.env.GSTACK_SPAWN_MAX_BUFFER = '${1024}';
|
||||
require('${polyfillPath}');
|
||||
(async () => {
|
||||
// Child writes 10 KB; cap is 1 KB; drained output should be exactly 1 KB
|
||||
// and exit should still resolve cleanly (child not back-pressured to death).
|
||||
const p = Bun.spawn(
|
||||
['node', '-e', 'process.stdout.write("y".repeat(10 * 1024)); process.exit(0)'],
|
||||
{ stdio: ['ignore', 'pipe', 'ignore'] }
|
||||
);
|
||||
const code = await Promise.race([
|
||||
p.exited,
|
||||
new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000))
|
||||
]).catch(() => 'TIMEOUT');
|
||||
const out = await new Response(p.stdout).text();
|
||||
console.log(out.length + ':' + code);
|
||||
})();
|
||||
`], { stdout: 'pipe', stderr: 'pipe' });
|
||||
expect(result.stdout.toString().trim()).toBe('1024:0');
|
||||
});
|
||||
|
||||
// Regression for the pipe-blocking case: if the child writes more than the
|
||||
// OS pipe buffer (~16-64 KB) and the polyfill doesn't drain eagerly, the
|
||||
// child blocks in write() and `exit` never fires. 1 MB is well past every
|
||||
|
|
|
|||
Loading…
Reference in New Issue