Skip to content

test(process-stdin): join stdin chunks before comparing, ignore ENOTCONN on macOS - #42260

Open
robobun wants to merge 1 commit into
mainfrom
robobun/454c3785/process-stdin-chunk-boundaries
Open

robobun wants to merge 1 commit into
mainfrom
robobun/454c3785/process-stdin-chunk-boundaries

Conversation

@robobun

@robobun robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • test/js/node/process/process-stdin.test.ts is red on main. Windows: "stdin with 'readable' event handler should receive data when paused" gets got chunk {"type":"Buffer","data":[97,98,99,10]} and a second chunk. The snapshot has one. macOS: 22 pass, 0 fail, 42 errors, each ENOTCONN: socket is not connected, write.
  • Windows: WindowsStreamingWriter::process_send (src/io/PipeWriter.rs:2260) keeps one uv_write in flight, so the second write() leaves one event loop turn later. Since make some tests faster #33622 made the tests concurrent, 12 more Bun.spawn calls come before that turn, and the child reads first.
  • macOS: the run() helper rethrows every write error except EPIPE. XNU returns ENOTCONN when the peer closes while send() is inside the kernel. FileReader: release the event loop while a pipe reader is stopped at its highwater mark #42038 added children that exit right after a read.

Fix

  • The child joins what read() returns and prints it on 'end'. No exit timer: only stdin keeps the child alive.
  • run() also ignores ENOTCONN on macOS.
  • Correct: pipe chunk boundaries are not a contract. Node reports the same ENOTCONN (libuv remaps only EPROTOTYPE).
  • Verified on Windows 11 arm64, canary e5f9986: the old file fails 10 of 10 runs, the new file 0 of 20. Linux release and debug+ASAN pass. Bun 1.2.18 (before Continue emitting 'readable' events after pausing stdin #17690) fails the new test.

Background

  • Bun.spawn({ stdin: "pipe" }) gives the child a named pipe on Windows and a socketpair on POSIX (512 KB buffers on macOS, src/spawn_sys/spawn_process.rs:826).
  • process.stdin.read() returns the bytes buffered at that moment.
  • test.concurrent runs the synchronous start of every test before the event loop turns again.
Notes

Windows

macOS

  • XNU sosend() checks the socket state (sosendcheck: SS_CANTSENDMORE first, so EPIPE), then drops the socket lock while it copies the user data, then calls uipc_send(). uipc_send() tests SS_ISCONNECTED first (ENOTCONN) and SS_CANTSENDMORE second (bsd/kern/uipc_usrreq.c). unp_disconnect() changes both flags at once. A close that lands during the copy gives ENOTCONN. With 512 KB buffers the copy is long enough to hit.
  • Reproduced outside Bun on macOS 15.7 arm64 with a Perl socketpair, 512 KB buffers, a child that reads twice and closes, and a non-blocking writer: ENOTCONN in 40 to 59 of 500 runs, EPIPE in the rest. The same script on Linux: EPIPE in 500 of 500.
  • Reproduced with Bun 1.3.14 on the same Mac: six children at a time that exit after the first read, 40 MB queued on each stdin: 149 EPIPE, 1 ENOTCONN. Linux: EPIPE only.
  • ignoreEpipe dates from stdin: apply highwater backpressure to the pipe FileReader source #35977, whose children stop reading and wait 1.5 s before they exit. The parent's socket is full by then, so no write is in progress at the close. First ENOTCONN in CI: build 113381, two hours after FileReader: release the event loop while a pipe reader is stopped at its highwater mark #42038 merged. None in the main builds from 2026-08-21 up to then. Seen since in 113463, 113483, 113533, 113549, 113565, 113566, 113625, 113635, 113709, 113725, 113732, 113737, 113771, 113795, 113805, 113824, 113831, 113843, 113899, 113906, 113910, 113926, 113933, 113935. All 25 builds contain FileReader: release the event loop while a pipe reader is stopped at its highwater mark #42038.
  • 42 errors is one run() call: 40 writes, flush() and end() reject with the same error. Forcing a mismatch in ignoreEpipe on Linux gives the same signature: 1 pass, 0 fail, 42 errors.
  • ENOTCONN stays an error on Linux: the check is gated on isMacOS.

Coverage

  • The old child held itself alive with a 1 s timer and exited with code 1. The new child has nothing but stdin, so the test now also fails if pause() lets the process exit before EOF. It also asserts isPaused() is still true at 'end'.
  • No other test in the file depends on chunk boundaries: the rest use a single write or a handshake.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/process/process-stdin.test.ts

… a write on macOS

The 'readable' while paused test snapshotted one coalesced chunk. On
Windows the parent's second write leaves one event loop turn after the
first, so a busy parent delivers two chunks. The child now joins what
read() returns and prints it on 'end'.

The pipe backpressure run() helper rethrew every write error except
EPIPE. macOS reports ENOTCONN when the child closes its end while the
write is inside the kernel.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

On-demand reviews are free for the next 10 days. After that, they cost $0.25 per reviewed file.

Or wait 51 seconds for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 7fdd9787-c7d9-40b8-a0d5-7e8f07d76ad3

📥 Commits

Reviewing files that changed from the base of the PR and between 97c191b and 1ea21b9.

📒 Files selected for processing (1)
  • test/js/node/process/process-stdin.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: fix for the test/js/node/process/process-stdin.test.ts break on main (seen red in build 113935). The diff is ready for a maintainer.

How I reproduced it:

  • Windows: bun test test/js/node/process/process-stdin.test.ts on a Windows 11 arm64 VM with canary e5f9986. The old file fails 10 of 10 runs with the two-chunk diff. The old test alone (-t) passes 10 of 10. The new file passes 20 of 20.
  • macOS: a Perl socketpair script with 512 KB buffers on macOS 15.7 arm64. A write() that is in the kernel when the peer closes returns ENOTCONN in 40 to 59 of 500 runs. Linux returns EPIPE in 500 of 500. The same with Bun 1.3.14 as the parent: 1 ENOTCONN in 150 children.
  • Linux: the new file passes with the release build and with the debug+ASAN build (22 pass).

CI on this PR (build 114093):

  • process-stdin.test.ts passes on the first attempt on every lane. That includes Windows 11 aarch64 (red on all 4 attempts in 113935), Windows 2019 x64, darwin aarch64 and darwin x64 (22 pass, 0 fail, no Unhandled error between tests).
  • One job failed: test/js/bun/http/serve-pending-promise-abort-leak.test.ts on debian 13 x64-asan. This PR does not touch that test. It fails the same way on main (also in builds 113896 and 113910), and test: tolerate one conservative-GC survivor in the aborted body stream check #42190 is open for it.
  • The build is complete: 180 of 181 jobs passed.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — test-only de-flake that follows the repo's own rules.

What was reviewed:

  • The rewritten "readable while paused" test: still asserts the original property (data arrives via read() while paused), adds isPaused() at 'end', and drops only the non-contractual chunk-boundary assumption; the removed setTimeout(1000)+exit(1) was the anti-pattern CLAUDE.md forbids.
  • Pipe draining now uses Promise.all([stdout, stderr, exited]) with stdout/stderr asserted before the exit code — matches harness convention.
  • ignoreEpipe widening: ENOTCONN is gated on isMacOS only, so Linux still fails loud; the WHY comment is present.
Extended reasoning...

Overview

This PR touches a single test file, test/js/node/process/process-stdin.test.ts, with two independent de-flake fixes and no runtime code changes. First, the "stdin with 'readable' event handler should receive data when paused" test is rewritten so the child accumulates read() chunks, waits for 'end', and prints a JSON summary; the parent drains stdout/stderr/exited concurrently and asserts the joined payload plus isPaused(). This replaces a setTimeout(1000)-driven process.exit(1) and a snapshot that assumed both writes arrive as one chunk. Second, the ignoreEpipe helper in the pipe-backpressure block now also swallows ENOTCONN on macOS, with a comment explaining XNU can surface it when the peer closes mid-send().

Security risks

None. This is test-only code with no changes to runtime behavior, no new dependencies, and no security-sensitive paths involved.

Level of scrutiny

Low-to-moderate. The main risk with de-flake PRs is silently weakening an assertion. I checked that against REVIEW.md's "keep asserting the property the original assertion protected": the original test's property was that a paused stdin still delivers data through 'readable'/read(). That is preserved and strengthened — the new test additionally asserts isPaused() is still true at 'end' and, by removing the exit timer, also fails if pause() lets the process exit before EOF. What was dropped (both writes arriving as exactly one buffer) is not a stream contract on any platform; the PR body's Windows analysis of WindowsStreamingWriter and test.concurrent scheduling explains why it flapped.

Other factors

The change lines up with the repo's test conventions point-for-point: Promise.all pipe drain, stdout/stderr asserted before exit code, bunExe()/bunEnv, isMacOS from harness rather than a hand-rolled check, and a load-bearing WHY comment on the platform branch. The ENOTCONN allowance is narrowly scoped (isMacOS && e?.code === "ENOTCONN"), so Linux and Windows semantics are unchanged and any other errno still throws — consistent with "map only the specific expected errno to the benign path". No CODEOWNERS entry covers this file, there are no prior review objections in the timeline, and the bug hunt exited on dry_streak with no findings.

@robobun

robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:04 PM PT - Sep 10th, 2026

❌ @robobun, your commit 1ea21b9 has 1 failures in Build #114093 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 42260

That installs a local version of the PR into your bun-42260 executable, so you can run:

bun-42260 --bun

Jarred-Sumner pushed a commit that referenced this pull request Sep 27, 2026
#44072)

### Problem
- `test/js/bun/console/console-write.test.ts:134` fails on macOS with `-
"caught EPIPE` / `+ "caught ENOTCONN`.
- The case closes the reader while the child writes 8 MB. The macOS
kernel fails the write that the close lands in with `ENOTCONN`, not
`EPIPE`.
- #43649 added the case and expects `EPIPE` only. Builds 121009 and
120123 were red: each attempt failed with this or with the unhandled
rejection that #43679 fixes.

### Fix
- On macOS the case accepts `caught EPIPE` or `caught ENOTCONN`. Other
platforms accept `caught EPIPE` only.
- The case still requires a rejected promise, no other stderr output,
and exit code 0.
- Verified on a CI Mac (macOS 26.6.1, M2), `main` canary, 150 runs of
the file. `caught ENOTCONN` fails 7 runs of the old file and 0 runs of
the new file.
- `bun bd test test/js/bun/console/console-write.test.ts` passes on
Linux.

### Background
- `Bun.spawn` gives a child with `stdout: "pipe"` one end of an AF_UNIX
socketpair.
- XNU `sosend()` tests for `EPIPE`, then unlocks the socket to copy the
bytes. `uipc_send()` then tests "connected" first and returns
`ENOTCONN`. Each later write returns `EPIPE`.
- Considered a retry in `try_write_with_write_fn`
(`src/io/PipeWriter.rs:84`) so that the writer reports `EPIPE`. #40935
decided that Bun passes the kernel errno through, as Node does. Node
gave `ENOTCONN` in 65 of 150 runs of this scenario.
- The unhandled rejection still fails 67 of those 150 runs. With #43679
and this PR the file passes 100 of 100 runs.

<details><summary>Notes</summary>

**This PR changes one test.** No file under `src/` changes.

**Kernel path** (xnu-12377, `bsd/kern`):
- `sosendcheck()` (`uipc_socket.c`) tests `SS_CANTSENDMORE` first and
returns `EPIPE`.
- `sosend()` calls `socket_unlock()` before the `uiomove` copy and
`socket_lock()` after it. It calls `pru_send` with no second state test.
- `uipc_send()` (`uipc_usrreq.c`, `SOCK_STREAM` branch) tests
`SS_ISCONNECTED` first (`ENOTCONN`) and `SS_CANTSENDMORE` second
(`EPIPE`).
- `soisdisconnected()` (`uipc_socket2.c`) clears `SS_ISCONNECTED` and
sets `SS_CANTSENDMORE` in one step.
- `write(2)` on a socket takes the same path (`soo_write()` calls
`sosend()`).

**C probe, no Bun and no Node.** An 8 MB write on an AF_UNIX socketpair
with 512 KB buffers. The peer reads one chunk and closes. 500 runs each.

| host | macOS | non-blocking `send()` | blocking `write()` |
| --- | --- | --- | --- |
| M2 mini | 26.6.1 arm64 | `ENOTCONN` 141 | `ENOTCONN` 396 |
| M2 Ultra | 15.7.9 arm64 | `ENOTCONN` 442 | `ENOTCONN` 392 |
| Intel i7 | 14.8.9 x64 | `ENOTCONN` 1 | `ENOTCONN` 287 |
| Linux x64 | | `ENOTCONN` 0 of 300 | `ENOTCONN` 0 of 200 |

All other runs gave `EPIPE`. So the errno is not specific to macOS 26 or
to one machine. With the default socket buffer size the M2 mini gave
`EPIPE` in 500 of 500 runs.

One more write directly after the `ENOTCONN` returned `EPIPE` in 1659 of
1659 runs. A socket that was never connected returns `ENOTCONN` on each
write.

**The scenario of the test**, on the M2 mini (macOS 26.6.1), Bun
`1.4.3-canary.1+37da174d5`, Node v26.3.0:

| parent | child | `EPIPE` | `ENOTCONN` |
| --- | --- | --- | --- |
| bun | bun, `await console.write(big)` | 63 | 137 |
| bun | bun, `process.stdout.write(big, cb)` | 66 | 84 |
| bun | node, `process.stdout.write(big, cb)` | 145 | 55 |
| node | node, `process.stdout.write(big, cb)` | 85 | 65 |

On macOS 14.8.9 x64, node parent and node child: `ENOTCONN` in 18 of 200
runs.

**Runs of the test file** on the M2 mini:

| binary | test file | runs | failed | failures |
| --- | --- | --- | --- | --- |
| `main` canary | `main` | 150 | 76 | 7 `caught ENOTCONN` (one
argument), 69 unhandled rejection (several arguments) |
| `main` canary | this PR | 150 | 67 | 67 unhandled rejection (several
arguments) |
| #43679 (4133870) | #43679 | 100 | 3 | 3 `caught ENOTCONN` (one
argument) |
| #43679 (4133870) | #43679 merged with this PR | 100 | 0 | |

The test on `main` stays flaky on macOS until #43679 merges.

**The decision in #40935.** An earlier revision of #40935 folded
`ENOTCONN` into `EPIPE` in `write_to_socket`. The merged commit
(118fdd2) dropped the fold: "The kernel errno is passed through
unchanged." Node accepts both codes on macOS in its own test
(`test/js/node/test/parallel/test-cluster-concurrent-disconnect.js:27-33`).
At that time the race was not reproduced (1100 runs, all `EPIPE`). The
probes above reproduce it. This PR keeps the decision.

**`process.platform` and not `isMacOS`.** #43679 changes the `harness`
import line of this file. The test reads `process.platform` so that the
two PRs merge in either order with no conflict (checked with `git
merge-tree`).

**The same errno in other tests.** In the annotations of the last 60
finished builds, `ENOTCONN` appears in the output of three test files:
this one, `test/js/bun/util/filesink.test.ts` (#43794 removes the race
there) and `test/js/node/process/process-stdin.test.ts` (#42260 ignores
the code on macOS).

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/bun/console/console-write.test.ts

<!-- robobun:evidence:end -->

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant