Repository navigation
test: hold the UDP port until the graph's socket is bound in module-graph-io.test.ts - #45871
dylan-conway wants to merge 1 commit into
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Updated 9:40 PM PT - Oct 17th, 2026
@dylan-conway, your commit 8a9f0a2 is building: |
|
Folded into #45873. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const { port, close } = await host.udpPortToClose(); | ||
| const graph = new Bun.ModuleGraph({ globals: { closedPort: port }, onError: error => resolve("graph: " + error.code) }); | ||
| const { stop } = await graph.import(import.meta.dir + "/tenant.mjs"); | ||
| await close(); |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: The test can still hang or go wrong when another process takes the freed port, and after merging this remains a flake for CI runs of this file. After close() at test/js/bun/module-graph/module-graph-io.test.ts:1058 the port is free while the tenant keeps sending every 5 ms; a sibling case in the same describe.concurrent binds port 0 at line 937 with data: boom and never closes it, so if it is handed that port the datagrams are delivered, not refused, and boom fires in the sibling for a packet it never expected. Fix: keep the port taken for the whole test (hold the host socket and make the graph socket's error come from a port that cannot be reassigned, e.g. an unroutable or firewalled target, or assert with a bounded deadline and a stated reason).
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
The sibling test at test/js/bun/module-graph/module-graph-io.test.ts:898 runs concurrently as its own process (describe.concurrent) and its cases at lines 937 and 954 bind UDP port 0 on 127.0.0.1; the line-937 Bun.udpSocket is never closed, so it lives until that process exits (up to the 60 s timeout at line 1038). After await close() at line 1058 the port P is free, and the kernel can hand it to that sibling bind. Then every 5 ms send from tenant.mjs line 1048 is delivered to the sibling's socket: no ICMP unreachable, no on_recv_error (src/runtime/socket/udp_socket.rs:97), no graph: ECONNREFUSED, and the sibling's data: boom runs for a packet from another test. On base (line 1055 old closedUdpPort) the port was freed earlier, so the exposure is not widened, but this residual path still makes the awaited promise at line 1059 wait for an unrelated process to exit. Remedy: keep the port owned (or the error source deterministic) for the whole test rather than freeing it mid-test.
Verification: Pre-existing and narrowed by this PR. After await close() at /home/claude/bun/test/js/bun/module-graph/module-graph-io.test.ts:1058 the host's dgram socket no longer owns P; if another socket binds P before the tenant's next send draws ICMP port-unreachable, every 5 ms "x" is delivered instead of refused, and await promise at line 1059 never settles. Nothing in the diff rules this out.
### What does this PR do? Fixes 20 causes of flaky tests, found in the annotations of the last 400 CI builds. Only files under `test/` change. No limit or deadline is raised, and no assertion is dropped except where the table says on which platform. Each row was first its own PR. That PR has the CI sightings, the cause and the numbers in full. | test file | what went wrong | what changes | write-up | |---|---|---|---| | `cli/install/bun-install-streaming-extract.test.ts` | the line taken for the error could be one of the verbose extract log | those lines are removed before the `error:` line is looked for | #45842 | | `cli/install/bun-install-streaming-extract.test.ts` | a body that is all there at the client's first read is not streamed, so `Streamed ` is not printed | the fixture is larger than one read of the client holds | #45844 | | `js/bun/shell/yield.test.ts` | Windows keeps the working directory of a killed process busy for a while (`EBUSY` on `rm`); the pid file exists before the pid is in it | the pid file is named by its full path and nothing has the directory as its cwd; the waiter waits for the file's content | #45846 | | `internal/harness-max-rss.test.ts` | a busy macOS machine no longer keeps the runner's 256 MiB resident when they are read | the two assertions on the runner's own RSS are for Linux, the one platform where a child inherits it | #45847 | | `js/web/fetch/body-clone.test.ts` | the FIFO case also wrote to a stdin that its child never reads (`EPIPE`) | that case gets no stdin | #45848 | | `js/first_party/ws/ws-server-compat-close.test.ts` | the count of sends that were written assumed the loopback takes bytes at once | it counts up to the last send that left nothing buffered | #45849 | | `js/bun/spawn/spawn-stdin-destroy.test.ts` | `sleep(80)` stood for "the child has exited" | `await child.exited` | #45850 | | `regression/issue/29181.test.ts` | file descriptors are closed on the work pool, so some are still open right after the last response | the count is polled until it is under the same limit, with a deadline | #45852 | | `js/web/fetch/fetch-keepalive.test.ts` | Windows Server 2019 delivers loopback bytes late while other loopback connections are busy, as macOS does | the five TCP cases are left out on `10.0.17763` as they are on macOS | #45853 | | `js/bun/jsc/bun-jsc.test.ts` | one run of 10 ms gets no sample when the sampler thread gets no CPU | `profile()` is repeated until it has a sample, with a deadline | #45854 | | `js/third_party/jsonwebtoken/async_sign.test.js` | two tokens signed on either side of a second differ in `iat` | the payload has a fixed `iat` | #45855 | | `js/bun/test/parallel/test-http-should-emit-events-in-the-right-order.ts` | the fixture asked a host on the internet | it asks a local server | #45856 | | `cli/run/transpiler-cache.test.ts` | a killed helper still had the temp directory as its cwd when the next test removed it (`EBUSY`, Windows) | the helper is held with `await using` | #45857 | | `js/web/fetch/fetch-abort-stream-body.test.ts` | `expect(p).rejects` in a socket callback runs a nested tick of the event loop, after which a concurrent test's `proc.stdout.text()` never settles | the promise is awaited | #45862 | | `js/bun/gc/gc-controller-cadence.test.ts` | the wait ended at `<= limit`, the assertion wants `< limit` | the wait goes on at `>= limit`, and the reading that ends it is the one asserted | #45863 | | `js/web/websocket/websocket-client.test.ts` | a limit of 1 s per test from 2023, now with a real closing handshake with a `node` process inside it | the limit is removed, the runner's applies | #45864 | | `js/node/tls/node-tls-connect.test.ts` | in TLS 1.3 the server's handshake ends after the client's, and the test waited for the client's | it waits for both | #45866 | | `js/node/tls/tls-shutdown-before-handshake-fixture.mjs` | the "refused" port was one `connect()` chose, which Linux can hand to the socket under test as its own: it connects to itself | `bind()` chooses the port | #45869 | | `js/bun/module-graph/module-graph-io.test.ts` | on macOS `fs.watch()` returns before anything is watched, and the case wrote once | it writes until the listener is called | #45870 | | `js/bun/module-graph/module-graph-io.test.ts` | the closed UDP port can be given to the socket under test, which then hears itself and is never refused | the port stays bound until that socket exists | #45871 | ### How did you verify your code works? Each change on its own, with release and ASAN binaries that CI built from main's code: the failure reproduced with main's file (under load, or with the losing order forced by a delayed loopback, a blocked thread, a stopped process or a narrow port range), the same conditions with the fix, and a negative test that shows the test still fails on what it guards. Linux x64 locally, Windows 11 x64 for the Windows rows. Some of the numbers: | | main | fix | |---|---|---| | `yield.test.ts`, Windows, 96 at a time | 2878 of 4000 fail | 0 of 4000 | | `transpiler-cache.test.ts`, Windows | 99 of 120 | 0 of 120 | | `bun-install-streaming-extract.test.ts` (body size), Windows | 326 of 6000 | 0 of 6000 | | `29181.test.ts` | 143 of 280 | 0 of 280 | | `bun-jsc.test.ts`, contended CPU | 111 of 200 | 0 of 200 | | `node-tls-connect.test.ts`, loopback delayed by 5 ms | 3 of 3 in each round | 0 | | `tls-shutdown-before-handshake-fixture.mjs`, 32 ports, node and Bun | 12 of 100 each | 0 of 100 each | All of it together, on Linux x64: every changed file passes, but for two tests that fail with main's files on my machine too ("a fifo in place of an entry is removed instead of opened" in `transpiler-cache.test.ts`, and "under node > a session that is set before the handshake starts is resumed" in `node-tls-connect.test.ts`, which runs under my `node` v26.7.0). Not run on the platform they are for: the macOS rows (`harness-max-rss`, `ws-server-compat-close`, the `fs.watch` case), which rest on the CI failures, the source and an emulation on Linux. The Server 2019 numbers of `fetch-keepalive` are from a CI run.
What does this PR do?
Removes a way for "an error of the graph's socket that no script threw" in
test/js/bun/module-graph/module-graph-io.test.tsto hang for good.For a UDP port that nothing listens on, the test binds port 0, closes that socket, and passes its port
Pon. The graph's module then callsBun.udpSocket({ connect: { port: P } }), which binds port 0 too, and the kernel can give itPagain. That socket is connected to itself: eachsend("x")arrives at its owndata(),ECONNREFUSEDnever comes, and the promise the test waits for never settles.The port now stays bound until the graph's socket exists, and is closed then. The module already sends every 5 ms, so the next send is refused.
The test timed out after 270 s and passed on a retry in build 127606 (Debian 13 x64, ASAN lane). The log says nothing more than that, and that branch also changes the event loop, so I cannot show that this is what happened there. It is the one permanent hang I found in the test.
How did you verify your code works?
Linux x64, with a release binary CI built from main's code (d0831bc).
To make it frequent, each run is in a fresh network namespace with 4 ephemeral ports (
ip_local_port_range). That test, main's file and this PR's in alternating rounds:The mechanism alone, in a script that does what the test does: in the runs where the socket's own port equals the closed one it hears its own packets (39) and gets no error, and in the others it hears nothing and gets 38 to 40
ECONNREFUSED.With the default range (32768 to 60999) the two ports were equal in 38 of 1000000 rounds of bind, close and connect.
The test still catches what it is for: with the module imported outside the graph it reports
host: ECONNREFUSEDwheregraph: ECONNREFUSEDis expected.The whole file passes 3 of 3 times (28 pass).