diff --git a/src/io/PipeWriter.zig b/src/io/PipeWriter.zig index d85f4f1373..b85baad9db 100644 --- a/src/io/PipeWriter.zig +++ b/src/io/PipeWriter.zig @@ -816,6 +816,12 @@ fn BaseWindowsPipeWriter( pub fn close(this: *WindowsPipeWriter) void { this.is_done = true; if (this.source) |source| { + // Check if there's a pending async write before closing. + // If so, we must defer the onCloseSource notification to + // onWriteComplete, because the parent's onClose handler may + // free the writer's StreamBuffer resources, and the pending + // write callback would then access freed memory. + const has_pending_async_write = this.hasPendingAsyncWrite(); switch (source) { .sync_file, .file => |file| { // Use state machine to handle close after operation completes @@ -837,7 +843,12 @@ fn BaseWindowsPipeWriter( }, } this.source = null; - this.onCloseSource(); + if (!has_pending_async_write) { + this.onCloseSource(); + } + // When has_pending_async_write is true, onCloseSource() will + // be called from onWriteComplete/onFsWriteComplete after the + // pending write callback completes. } } @@ -989,7 +1000,22 @@ pub fn WindowsBufferedWriter(Parent: type, function_table: anytype) type { return .success; } + /// Returns true if there is an outstanding async write request + /// (uv_write or uv_fs_write) that hasn't completed yet. + pub fn hasPendingAsyncWrite(this: *const WindowsWriter) bool { + return this.pending_payload_size > 0; + } + fn onWriteComplete(this: *WindowsWriter, status: uv.ReturnCode) void { + // If the source was closed (e.g. close() was called while a write + // was in-flight), clean up and notify the parent via onCloseSource + // (which was deferred by close()). + if (this.source == null) { + this.pending_payload_size = 0; + this.onCloseSource(); + return; + } + const written = this.pending_payload_size; this.pending_payload_size = 0; if (status.toError(.write)) |err| { @@ -1292,12 +1318,28 @@ pub fn WindowsStreamingWriter(comptime Parent: type, function_table: anytype) ty return (this.outgoing.isNotEmpty() or this.current_payload.isNotEmpty()); } + /// Returns true if there is an outstanding async write request + /// (uv_write or uv_fs_write) that hasn't completed yet. + pub fn hasPendingAsyncWrite(this: *const WindowsWriter) bool { + return this.current_payload.isNotEmpty(); + } + fn isDone(this: *WindowsWriter) bool { // done is flags andd no more data queued? so we are done! return this.is_done and !this.hasPendingData(); } fn onWriteComplete(this: *WindowsWriter, status: uv.ReturnCode) void { + // If the source was closed (e.g. close() was called while a write + // was in-flight), clean up buffers and notify the parent via + // onCloseSource (which was deferred by close()). + if (this.source == null) { + this.current_payload.reset(); + this.outgoing.reset(); + this.onCloseSource(); + return; + } + if (status.toError(.write)) |err| { this.last_write_result = .{ .err = err }; log("onWrite() = {s}", .{err.name()}); diff --git a/test/regression/issue/27097.test.ts b/test/regression/issue/27097.test.ts new file mode 100644 index 0000000000..5773a8a717 --- /dev/null +++ b/test/regression/issue/27097.test.ts @@ -0,0 +1,89 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// https://github.com/oven-sh/bun/issues/27097 +// Segfault when pipe write completion callback fires after close() freed StreamBuffer +// The bug: on Windows, close() called onCloseSource() synchronously, which could +// free the writer's StreamBuffer resources. If a uv_write was pending, its callback +// would later access the freed memory, causing a segfault at 0xFFFFFFFFFFFFFFFF. + +test("closing spawn stdin while write is pending should not crash", async () => { + // Spawn a process that reads from stdin. + // Write data to stdin, then immediately close. + // This creates a scenario where a pipe write may be pending when close() is called. + for (let i = 0; i < 5; i++) { + const proc = Bun.spawn({ + cmd: [bunExe(), "-e", "process.stdin.resume(); process.stdin.on('close', () => process.exit(0));"], + env: bunEnv, + stdin: "pipe", + stdout: "ignore", + stderr: "ignore", + }); + + // Write a large amount of data to stdin - this makes it more likely that + // a write will be pending when we close + try { + proc.stdin.write("x".repeat(65536)); + proc.stdin.flush(); + } catch { + // Write may fail if process already exited - that's fine + } + + // Close stdin while the write may still be pending + proc.stdin.end(); + + // Wait for the process to exit + await proc.exited; + } +}, 30_000); + +test("rapid spawn and close cycles should not corrupt pipe state", async () => { + // Simulate the pattern from the bug report: many spawn operations over time. + // Each spawn creates pipes, writes some data, and tears down. + const iterations = 10; + + for (let i = 0; i < iterations; i++) { + const proc = Bun.spawn({ + cmd: [bunExe(), "-e", "console.log('ok');"], + env: bunEnv, + stdout: "pipe", + stderr: "ignore", + }); + + const stdout = await new Response(proc.stdout).text(); + const exitCode = await proc.exited; + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + } +}, 30_000); + +test("FileSink write and close race should not crash", async () => { + // Test the FileSink (StreamingWriter) path by using spawn with a ReadableStream + // as stdin, which creates a FileSink internally. + for (let i = 0; i < 5; i++) { + const data = "hello ".repeat(1000); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(data)); + controller.close(); + }, + }); + + const proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + "let t=0; process.stdin.on('data',(c)=>{t+=c.length}); process.stdin.on('end',()=>{console.log(t)})", + ], + env: bunEnv, + stdin: stream, + stdout: "pipe", + stderr: "ignore", + }); + + const stdout = await new Response(proc.stdout).text(); + const exitCode = await proc.exited; + expect(exitCode).toBe(0); + expect(Number(stdout.trim())).toBeGreaterThan(0); + } +}, 30_000);