diff --git a/src/shell/Builtin.zig b/src/shell/Builtin.zig index b29ff24009..ab5eb31ed6 100644 --- a/src/shell/Builtin.zig +++ b/src/shell/Builtin.zig @@ -668,6 +668,10 @@ pub fn deinit(this: *Builtin) void { // No need to free it because it belongs to the parent cmd // _ = Syscall.close(this.cwd); + // Cancel any pending IOWriter chunks that reference this builtin + // to prevent use-after-free when the write completes. + this.cancelPendingChunks(); + this.stdout.deref(); this.stderr.deref(); this.stdin.deref(); @@ -676,6 +680,49 @@ pub fn deinit(this: *Builtin) void { // this.arena.deinit(); } +/// Cancel any pending IOWriter chunks that reference this builtin's impl. +/// This prevents use-after-free crashes when a write completes after +/// the builtin has been freed. +fn cancelPendingChunks(this: *Builtin) void { + const impl_ptr: usize = switch (this.kind) { + .cat => @intFromPtr(&this.impl.cat), + .touch => @intFromPtr(&this.impl.touch), + .mkdir => @intFromPtr(&this.impl.mkdir), + .@"export" => @intFromPtr(&this.impl.@"export"), + .echo => @intFromPtr(&this.impl.echo), + .cd => @intFromPtr(&this.impl.cd), + .which => @intFromPtr(&this.impl.which), + .rm => @intFromPtr(&this.impl.rm), + .pwd => @intFromPtr(&this.impl.pwd), + .mv => @intFromPtr(&this.impl.mv), + .ls => @intFromPtr(&this.impl.ls), + .exit => @intFromPtr(&this.impl.exit), + .true => @intFromPtr(&this.impl.true), + .false => @intFromPtr(&this.impl.false), + .yes => @intFromPtr(&this.impl.yes), + .seq => @intFromPtr(&this.impl.seq), + .dirname => @intFromPtr(&this.impl.dirname), + .basename => @intFromPtr(&this.impl.basename), + .cp => @intFromPtr(&this.impl.cp), + }; + + // Cancel chunks in stdout writer if it's an fd + switch (this.stdout) { + .fd => |fd| { + fd.writer.cancelChunksWithRawPtr(impl_ptr); + }, + else => {}, + } + + // Cancel chunks in stderr writer if it's an fd + switch (this.stderr) { + .fd => |fd| { + fd.writer.cancelChunksWithRawPtr(impl_ptr); + }, + else => {}, + } +} + /// If the stdout/stderr is supposed to be captured then get the bytelist associated with that pub fn stdBufferedBytelist(this: *Builtin, comptime io_kind: @Type(.enum_literal)) ?*bun.ByteList { if (comptime io_kind != .stdout and io_kind != .stderr) { diff --git a/src/shell/IOWriter.zig b/src/shell/IOWriter.zig index 059708c446..9e15f6ce3e 100644 --- a/src/shell/IOWriter.zig +++ b/src/shell/IOWriter.zig @@ -252,7 +252,12 @@ pub fn cancelChunks(this: *IOWriter, ptr_: anytype) void { ChildPtr => ptr_, else => ChildPtr.init(ptr_), }; - const actual_ptr = ptr.ptr.repr._ptr; + this.cancelChunksWithRawPtr(ptr.ptr.repr._ptr); +} + +/// Cancel chunks by raw pointer address. Used when we have an opaque +/// pointer and need to cancel chunks without type information. +pub fn cancelChunksWithRawPtr(this: *IOWriter, actual_ptr: usize) void { if (this.writers.len() == 0) return; const idx = this.writer_idx; const slice: []Writer = this.writers.sliceMutable(); diff --git a/src/shell/states/Cmd.zig b/src/shell/states/Cmd.zig index 125c297262..e22df9dcdd 100644 --- a/src/shell/states/Cmd.zig +++ b/src/shell/states/Cmd.zig @@ -726,11 +726,27 @@ pub fn deinit(this: *Cmd) void { this.spawn_arena.deinit(); } + // Cancel any pending IOWriter chunks that reference this Cmd + // to prevent use-after-free when the write completes. + this.cancelPendingChunks(); + this.io.deref(); this.base.endScope(); this.parent.destroy(this); } +/// Cancel any pending IOWriter chunks that reference this Cmd. +fn cancelPendingChunks(this: *Cmd) void { + switch (this.io.stderr) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } + switch (this.io.stdout) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } +} + pub fn bufferedInputClose(this: *Cmd) void { this.exec.subproc.buffered_closed.close(this, .stdin); } diff --git a/src/shell/states/CondExpr.zig b/src/shell/states/CondExpr.zig index 75c1dd2f37..974356b830 100644 --- a/src/shell/states/CondExpr.zig +++ b/src/shell/states/CondExpr.zig @@ -211,6 +211,10 @@ fn doStat(this: *CondExpr) Yield { } pub fn deinit(this: *CondExpr) void { + // Cancel any pending IOWriter chunks that reference this CondExpr + // to prevent use-after-free when the write completes. + this.cancelPendingChunks(); + this.io.deinit(); for (this.args.items) |item| { this.base.allocator().free(item); @@ -220,6 +224,18 @@ pub fn deinit(this: *CondExpr) void { this.parent.destroy(this); } +/// Cancel any pending IOWriter chunks that reference this CondExpr. +fn cancelPendingChunks(this: *CondExpr) void { + switch (this.io.stderr) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } + switch (this.io.stdout) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } +} + pub fn childDone(this: *CondExpr, child: ChildPtr, exit_code: ExitCode) Yield { if (child.ptr.is(Expansion)) { if (exit_code != 0) { diff --git a/src/shell/states/Pipeline.zig b/src/shell/states/Pipeline.zig index a3028ffe80..5d9d45b9f1 100644 --- a/src/shell/states/Pipeline.zig +++ b/src/shell/states/Pipeline.zig @@ -274,11 +274,28 @@ pub fn deinit(this: *Pipeline) void { if (this.cmds) |cmds| { this.base.allocator().free(cmds); } + + // Cancel any pending IOWriter chunks that reference this Pipeline + // to prevent use-after-free when the write completes. + this.cancelPendingChunks(); + this.io.deref(); this.base.endScope(); this.parent.destroy(this); } +/// Cancel any pending IOWriter chunks that reference this Pipeline. +fn cancelPendingChunks(this: *Pipeline) void { + switch (this.io.stderr) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } + switch (this.io.stdout) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } +} + fn initializePipes(pipes: []Pipe, set_count: *u32) Maybe(void) { for (pipes) |*pipe| { if (bun.Environment.isWindows) { diff --git a/src/shell/states/Subshell.zig b/src/shell/states/Subshell.zig index bcca4a4acd..de89893e05 100644 --- a/src/shell/states/Subshell.zig +++ b/src/shell/states/Subshell.zig @@ -172,12 +172,29 @@ pub fn onIOWriterChunk(this: *Subshell, _: usize, err: ?jsc.SystemError) Yield { pub fn deinit(this: *Subshell) void { this.base.shell.deinit(); + + // Cancel any pending IOWriter chunks that reference this Subshell + // to prevent use-after-free when the write completes. + this.cancelPendingChunks(); + this.io.deref(); this.redirection_file.deinit(); this.base.endScope(); this.parent.destroy(this); } +/// Cancel any pending IOWriter chunks that reference this Subshell. +fn cancelPendingChunks(this: *Subshell) void { + switch (this.io.stderr) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } + switch (this.io.stdout) { + .fd => |fd| fd.writer.cancelChunks(this), + else => {}, + } +} + pub fn writeFailingError(this: *Subshell, comptime fmt: []const u8, args: anytype) Yield { const handler = struct { fn enqueueCb(ctx: *Subshell) void { diff --git a/test/regression/issue/26660.test.ts b/test/regression/issue/26660.test.ts new file mode 100644 index 0000000000..c067b473b1 --- /dev/null +++ b/test/regression/issue/26660.test.ts @@ -0,0 +1,109 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// Test for issue #26660: Shell interpreter crash when spawning many concurrent shell commands +// The bug was that IOWriter could have pending chunks referencing freed memory after +// builtin/cmd cleanup, causing use-after-free crashes. +test("many concurrent shell commands should not crash", async () => { + // Create a test script that spawns many shell commands concurrently + // This is similar to what OpenCode does which triggered the original crash + using dir = tempDir("issue-26660", { + "test.ts": ` + const promises: Promise[] = []; + + // Spawn 60 shell commands concurrently (similar to the crash report) + for (let i = 0; i < 60; i++) { + promises.push( + Bun.$\`echo "Hello from shell \${i}"\`.text().then(() => {}) + ); + } + + await Promise.all(promises); + console.log("All shell commands completed successfully"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "test.ts"], + cwd: String(dir), + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).not.toContain("Segmentation fault"); + expect(stderr).not.toContain("panic"); + expect(stdout).toContain("All shell commands completed successfully"); + expect(exitCode).toBe(0); +}); + +test("concurrent shell commands with output should not crash", async () => { + // Test with commands that produce output, which exercises IOWriter more + using dir = tempDir("issue-26660-output", { + "test.ts": ` + const promises: Promise[] = []; + + // Spawn many echo commands that all write to IOWriter + for (let i = 0; i < 40; i++) { + promises.push( + Bun.$\`echo "Line \${i}: This is a test with some longer output to exercise the IOWriter buffer"\`.text() + ); + } + + const results = await Promise.all(promises); + console.log("Collected", results.length, "results"); + console.log("All outputs received correctly"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "test.ts"], + cwd: String(dir), + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).not.toContain("Segmentation fault"); + expect(stderr).not.toContain("panic"); + expect(stdout).toContain("All outputs received correctly"); + expect(exitCode).toBe(0); +}); + +test("shell commands with mixed builtins should not crash", async () => { + // Test various builtins that use IOWriter + using dir = tempDir("issue-26660-builtins", { + "test.ts": ` + const promises: Promise[] = []; + + // Spawn various builtin commands concurrently + for (let i = 0; i < 20; i++) { + promises.push(Bun.$\`echo "Echo \${i}"\`.text().then(() => {})); + promises.push(Bun.$\`pwd\`.text().then(() => {})); + promises.push(Bun.$\`true\`.text().then(() => {})); + } + + await Promise.all(promises); + console.log("All builtin commands completed"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "test.ts"], + cwd: String(dir), + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).not.toContain("Segmentation fault"); + expect(stderr).not.toContain("panic"); + expect(stdout).toContain("All builtin commands completed"); + expect(exitCode).toBe(0); +});