Skip to content

test: widen net-mongodb-pattern-leak RSS bound; await the named-pipe GC check - #33725

Open
robobun wants to merge 6 commits into
mainfrom
farm/609994f8/net-tcp-test-flakes
Open

test: widen net-mongodb-pattern-leak RSS bound; await the named-pipe GC check#33725
robobun wants to merge 6 commits into
mainfrom
farm/609994f8/net-tcp-test-flakes

node-net.test.ts: await the named-pipe TCPSocket GC check and destroy…

b324515
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 8, 2026 in 35m 7s

Code review found 1 important issue

Found 2 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 1
🟣 Pre-existing 1
Severity File:Line Issue
🟡 Nit test/js/node/net/node-net.test.ts:1024-1025 queueMicrotask resume drains before loop.c sees is_paused, so the test never exercises the new guard
🟣 Pre-existing packages/bun-usockets/src/loop.c:703-710 shut_down fast-close truncates bytes the is_paused defer is meant to preserve

Annotations

Check warning on line 1025 in test/js/node/net/node-net.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

queueMicrotask resume drains before loop.c sees is_paused, so the test never exercises the new guard

This test doesn't actually exercise the new `is_paused` branch in loop.c: `on_data`'s `exit_scope` drains microtasks before returning to the recv loop, so the `queueMicrotask`'d `c.resume()` clears `is_paused` back to 0 before loop.c ever checks it — the test passes with or without the loop.c change on every platform. Swap `queueMicrotask` for `setImmediate` so the pause survives past `us_dispatch_data`'s return and the test becomes a real fail-before guard on macOS.

Check notice on line 710 in packages/bun-usockets/src/loop.c

See this annotation in the file changed.

@claude claude / Claude Code Review

shut_down fast-close truncates bytes the is_paused defer is meant to preserve

Pre-existing, same bug class at the same line, so noting as another sibling for #32257 rather than a merge blocker. The ungated `us_socket_is_shut_down` fast-close at :701 runs *before* the new `is_paused` check, so the exact scenario this PR fixes is still not fixed when the client has already `end()`ed — `c.end(req)` → server writes >512KB + FIN → first `recv()` fills the buffer, `push()` returns false → pause → break → `is_shut_down` is true → `close_raw`, and the remaining bytes are silently