mirror of
https://github.com/oven-sh/bun
synced 2026-02-09 10:28:47 +00:00
5f1ca176cd
### What does this PR do? Fixes data loss when reading large amounts of data from subprocess pipes on Windows, a regression introduced by the libuv 1.51.0 upgrade in commite3783c244f. ### The Problem When piping large data through a subprocess on Windows (e.g., `process.stdin.pipe(process.stdout)`), Bun randomly loses ~73KB of data out of 1MB, receiving only ~974KB instead of the full 1048576 bytes. The subprocess correctly receives all 1MB on stdin, but the parent process loses data when reading from the subprocess stdout. ### Root Cause Analysis #### libuv 1.51.0 Change The libuv 1.51.0 upgrade (commit [libuv/libuv@727ee723](727ee7237e)) changed Windows pipe reading behavior: **Before:** libuv would call `PeekNamedPipe` to check available bytes, then read exactly that amount. **After:** libuv attempts immediate non-blocking reads (up to 65536 bytes) before falling back to async reads. If less data is available than requested, it returns what's available and signals `more=0`, causing the read loop to break. This optimization introduces **0-byte reads** when data isn't immediately available, which are delivered to Bun's read callback. #### The Race Condition When Bun's `WindowsBufferedReader` called `onRead(.drained)` for these 0-byte reads, it created a race condition. Debug logs clearly show the issue: **Error case (log.txt):** ``` Line 79-80: onStreamRead = 0 (drained) Line 81: filesink closes (stdin closes) Line 85: onStreamRead = 6024 ← Should be 74468! Line 89: onStreamRead = -4095 (EOF) ``` **Success case (success.log.txt):** ``` Line 79-80: onStreamRead = 0 (drained) Line 81: filesink closes (stdin closes) Line 85: onStreamRead = 74468 ← Full chunk! Line 89-90: onStreamRead = 0 (drained) Line 91: onStreamRead = 6024 Line 95: onStreamRead = -4095 (EOF) ``` When stdin closes while a 0-byte drained read is pending, the next read returns truncated data (6024 bytes instead of 74468 bytes). ### The Fix Two changes to `WindowsBufferedReader` in `src/io/PipeReader.zig`: #### 1. Ignore 0-byte reads (line 937-940) Don't call `onRead(.drained)` for 0-byte reads. Just return and let libuv queue the next read. This prevents the race condition that causes truncated reads. ```zig 0 => { // With libuv 1.51.0+, calling onRead(.drained) here causes a race condition // where subsequent reads return truncated data. Just ignore 0-byte reads. return; }, ``` #### 2. Defer `has_inflight_read` flag clearing (line 827-839) Clear the flag **after** the read callback completes, not before. This prevents libuv from starting a new overlapped read operation while we're still processing the current data buffer, which could cause memory corruption per the libuv commit message: > "Starting a new read after uv_read_cb returns causes memory corruption on the OVERLAPPED read_req if uv_read_stop+uv_read_start was called during the callback" ```zig const result = onReadChunkFn(this.parent, buf, hasMore); // Clear has_inflight_read after the callback completes this.flags.has_inflight_read = false; return result; ``` ### How to Test Run the modified test in `test/js/bun/spawn/spawn-stdin-readable-stream.test.ts`: ```js test("ReadableStream with very large chunked data", async () => { const chunkSize = 64 * 1024; // 64KB chunks const numChunks = 16; // 1MB total const chunk = Buffer.alloc(chunkSize, "x"); const stream = new ReadableStream({ pull(controller) { if (pushedChunks < numChunks) { controller.enqueue(chunk); pushedChunks++; } else { controller.close(); } }, }); await using proc = spawn({ cmd: [bunExe(), "-e", ` let length = 0; process.stdin.on('data', (data) => length += data.length); process.once('beforeExit', () => console.error(length)); process.stdin.pipe(process.stdout) `], stdin: stream, stdout: "pipe", env: bunEnv, }); const text = await proc.stdout.text(); expect(text.length).toBe(chunkSize * numChunks); // Should be 1048576 }); ``` **Before fix:** Randomly fails with ~974KB instead of 1MB **After fix:** Consistently passes with full 1MB Run ~100 times to verify the race condition is fixed. ### Related Issues This may also fix #23071 (Windows scripts hanging), though that issue needs separate verification. ### Why Draft? Marking as draft for Windows testing by the team. The fix is based on detailed debug log analysis showing the exact race condition, but needs verification on Windows CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Tests
Finding tests
Tests are located in the test/ directory and are organized using the following structure:
test/js/- tests for JavaScript APIs.cli/- tests for commands, configs, and stdout.bundler/- tests for the transpiler/bundler.regression/- tests that reproduce a specific issue.harness.ts- utility functions that can be imported from any test.
The tests in test/js/ directory are further categorized by the type of API.
test/js/bun/- tests forBun-specific APIs.node/- tests for Node.js APIs.web/- tests for Web APIs, likefetch().first_party/- tests for npm packages that are built-in, likeundici.third_party/- tests for npm packages that are not built-in, but are popular, likeesbuild.
Running tests
To run a test, use Bun's built-in test command: bun test.
bun test # Run all tests
bun test js/bun # Only run tests in a directory
bun test sqlite.test.ts # Only run a specific test
If you encounter lots of errors, try running bun install, then trying again.
Writing tests
Tests are written in TypeScript (preferred) or JavaScript using Jest's describe(), test(), and expect() APIs.
import { describe, test, expect } from "bun:test";
import { gcTick } from "harness";
describe("TextEncoder", () => {
test("can encode a string", async () => {
const encoder = new TextEncoder();
const actual = encoder.encode("bun");
await gcTick();
expect(actual).toBe(new Uint8Array([0x62, 0x75, 0x6E]));
});
});
If you are fixing a bug that was reported from a GitHub issue, remember to add a test in the test/regression/ directory.
// test/regression/issue/02005.test.ts
import { it, expect } from "bun:test";
it("regex literal should work with non-latin1", () => {
const text = "这是一段要替换的文字";
expect(text.replace(new RegExp("要替换"), "")).toBe("这是一段的文字");
expect(text.replace(/要替换/, "")).toBe("这是一段的文字");
});
In the future, a bot will automatically close or re-open issues when a regression is detected or resolved.
Zig tests
These tests live in various .zig files throughout Bun's codebase, leveraging Zig's builtin test keyword.
Currently, they're not run automatically nor is there a simple way to run all of them. We will make this better soon.
TypeScript
Test files should be written in TypeScript. The types in packages/bun-types should be updated to support all new APIs. Changes to the .d.ts files in packages/bun-types will be immediately reflected in test files; no build step is necessary.
Writing a test will often require using invalid syntax, e.g. when checking for errors when an invalid input is passed to a function. TypeScript provides a number of escape hatches here.
// @ts-expect-error- This should be your first choice. It tells TypeScript that the next line should fail typechecking.// @ts-ignore- Ignore the next line entirely.// @ts-nocheck- Put this at the top of the file to disable typechecking on the entire file. Useful for autogenerated test files, or when ignoring/disabling type checks an a per-line basis is too onerous.