Skip to content

Exit 13 on unsettled top-level await in the main entry instead of spinning - #36051

Closed
robobun wants to merge 8 commits into
mainfrom
farm/2eaadb90/unsettled-tla-exit-13
Closed

Exit 13 on unsettled top-level await in the main entry instead of spinning#36051
robobun wants to merge 8 commits into
mainfrom
farm/2eaadb90/unsettled-tla-exit-13

test: assert stderr before exit status; assert entry path in dependen…

06a1efa
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 27, 2026 in 30m 55s

Code review found 4 important issues

Found 5 candidates, confirmed 5. See review comments for details.

Details

Severity Count
🔴 Important 4
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/runtime/cli/run_command.rs:1623-1635 Spurious 'unsettled top-level await' warning when wait was aborted by a fatal error, not a natural drain
🔴 Important src/runtime/cli/run_command.rs:1623-1625 pending_internal_promise dereferenced after GC-triggering code without a GC root; SAFETY comment is false
🔴 Important src/jsc/VirtualMachine.rs:2448-2458 Regression: unref'd handle + top-level await now exits 13, breaking spawn.test.ts on all CI platforms
🟡 Nit src/jsc/VirtualMachine.rs:2448-2458 Sibling wait_for_promise sites (test runner, --preload) still spin on unsettled TLA — guard not moved into shared helper

Annotations

Check failure on line 1635 in src/runtime/cli/run_command.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Spurious 'unsettled top-level await' warning when wait was aborted by a fatal error, not a natural drain

The new Pending check fires whenever `is_event_loop_alive()` returned false, but that function also returns false when `unhandled_error_counter > 0` (VirtualMachine.rs:1036) — i.e. the wait was aborted by a fatal uncaught exception, not a natural drain. In that case ref'd handles that would have settled the entry promise are still live, so the "Detected unsettled top-level await" warning is spurious and misattributes the failure (Node prints only the exception). Gate this block on `vm.unhandled_

Check failure on line 1625 in src/runtime/cli/run_command.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

pending_internal_promise dereferenced after GC-triggering code without a GC root; SAFETY comment is false

The `SAFETY: p is a live JSC heap cell tracked by the VM` comment is false — in the non-watcher path `pending_internal_promise` is stored with `is_protected = false` and only `ensure_still_alive()` (frame-local `black_box`), and no C++ `visitChildren` visits it. This deref runs after the stack local dies at line 1530, after `run_gc(false)` at 1540, the drain loop, and `on_before_exit()`'s inner tick loop; once the entry promise settles it is reachable only from this unrooted raw pointer, so GC c

Check failure on line 2458 in src/jsc/VirtualMachine.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Regression: unref'd handle + top-level await now exits 13, breaking spawn.test.ts on all CI platforms

The new `!is_event_loop_alive()` break regresses `p.unref(); await p.exited` — an unref'd subprocess doesn't count toward liveness, so the loop bails before `auto_tick()` ever pumps the OS loop, the entry await stays Pending, and the process exits 13 with the warning instead of resolving. This is failing `test/js/bun/spawn/spawn.test.ts:805-840` ("unref() + .exited with nothing else ref'd") on **all 8 CI platforms** per the robobun comment. Either narrow the check so an in-flight unref'd child s

Check warning on line 2458 in src/jsc/VirtualMachine.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Sibling wait_for_promise sites (test runner, --preload) still spin on unsettled TLA — guard not moved into shared helper

Two byte-identical sibling sites still call the unfixed `wait_for_promise` and will spin at 100% CPU on a never-settling top-level await: `load_entry_point_for_test_runner` (VirtualMachine.rs:4604 — `bun test foo.test.ts` with module-scope `await new Promise(()=>{})`) and the `--preload` wait (jsc_hooks.rs:836 — `bun --preload hang.mjs entry.js`). Per REVIEW.md's "fix the whole class… prefer moving the guard into the shared helper; if a site is intentionally excluded, say so in the PR": consider