Compare commits

...

1 Commits

Author SHA1 Message Date
Claude Bot
be6d3ca2d8 fix(shell): prevent use-after-free in runFromJS error path
When `setupIOBeforeRun()` fails in `runFromJS`, `#deinitFromExec()` was
called which immediately frees the interpreter via `allocator.destroy(this)`.
Since the interpreter is a GC-managed object (`JSShellInterpreter`), the
GC finalizer later calls `deinitFromFinalizer()` on already-freed memory,
causing a segfault.

Replace `#deinitFromExec()` with `#derefRootShellAndIOIfNeeded(true)` which
cleans up runtime resources (IO, shell env) and sets `cleanup_state` to
`.runtime_cleaned`, letting the GC finalizer safely handle the actual struct
destruction.

Closes #26907

Co-Authored-By: Claude <noreply@anthropic.com>
2026-02-11 20:44:17 +00:00
2 changed files with 81 additions and 1 deletions

View File

@@ -1154,7 +1154,7 @@ pub const Interpreter = struct {
_ = callframe; // autofix
if (this.setupIOBeforeRun().asErr()) |e| {
defer this.#deinitFromExec();
this.#derefRootShellAndIOIfNeeded(true);
const shellerr = bun.shell.ShellErr.newSys(e);
return try throwShellErr(&shellerr, .{ .js = globalThis.bunVM().event_loop });
}

View File

@@ -0,0 +1,80 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
// Regression test for #26907: use-after-free in shell interpreter's runFromJS error path.
// When setupIOBeforeRun() fails (e.g., stdout handle is invalid on Windows), the interpreter
// must NOT call allocator.destroy(this) directly, since the GC finalizer will later try to
// access the freed memory. Instead, it should clean up runtime resources via
// derefRootShellAndIOIfNeeded and let the GC handle destruction.
//
// The bug is most reliably triggered on Windows where stdout can be unavailable,
// causing setupIOBeforeRun() -> dup(stdout) to fail. On Linux, dup rarely fails.
test("shell interpreter does not crash on GC after shell error", async () => {
// Run many shell commands and force GC to stress the interpreter lifecycle.
// On Windows with the bug, this would segfault during GC sweep when the
// finalizer accesses already-freed interpreter memory.
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
async function main() {
const promises = [];
for (let i = 0; i < 50; i++) {
promises.push(Bun.$\`echo hello\`.quiet().catch(() => {}));
}
await Promise.all(promises);
// Force GC multiple times to trigger finalizers
Bun.gc(true);
Bun.gc(true);
process.stderr.write("OK\\n");
}
main();
`,
],
env: bunEnv,
stderr: "pipe",
stdout: "pipe",
});
const [_stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("OK");
expect(exitCode).toBe(0);
});
test("shell interpreter handles closed stdout without crashing", async () => {
// Close stdout before running shell commands to trigger setupIOBeforeRun failure.
// On Windows this reliably triggers the error path; on Linux dup may still succeed.
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const fs = require("fs");
fs.closeSync(1);
async function main() {
try {
await Bun.$\`echo hello\`;
} catch (e) {
// Error is expected when stdout is unavailable
}
// Force GC to trigger finalizer - this would segfault before the fix on Windows
Bun.gc(true);
Bun.gc(true);
process.stderr.write("OK\\n");
}
main();
`,
],
env: bunEnv,
stderr: "pipe",
stdout: "pipe",
});
const [_stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("OK");
// Process must not segfault (exit code 139 on Linux, non-zero on Windows)
expect(exitCode).toBe(0);
});