From 0bdcdfd9ad3d842cdb60f28dbe9d469257fa5dfa Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Wed, 18 Feb 2026 13:26:59 +0000 Subject: [PATCH] fix(win): defer onCloseSource when pipe write is pending to prevent use-after-free (#27097) On Windows, when close() was called on a pipe writer while an async uv_write was still in-flight, onCloseSource() was called synchronously. This notified the parent immediately, which could free the writer's StreamBuffer resources. When the pending write callback later fired via uv__process_pipe_write_req, it accessed the freed StreamBuffer memory, causing a segfault at address 0xFFFFFFFFFFFFFFFF. The fix defers onCloseSource() when there's a pending async write. Instead of notifying the parent immediately, the notification happens in onWriteComplete after the pending write callback completes safely. This ensures the StreamBuffer memory remains valid throughout the write callback's execution. Affects both WindowsBufferedWriter and WindowsStreamingWriter in BaseWindowsPipeWriter.close(). Co-Authored-By: Claude --- src/io/PipeWriter.zig | 44 +++++++++++++- test/regression/issue/27097.test.ts | 89 +++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 test/regression/issue/27097.test.ts 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);