Skip to content

fetch: hold FetchTasklet mutex through deref_from_thread so HTTP thread is never the final deref - #32707

Open
robobun wants to merge 7 commits into
mainfrom
farm/c7e3e83d/fetch-tasklet-deref-mutex-order
Open

robobun wants to merge 7 commits into
mainfrom
farm/c7e3e83d/fetch-tasklet-deref-mutex-order

Address review: deterministic fixture, update stale deref_from_thread…

660a330
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 26, 2026 in 29m 0s

Code review found 1 potential issue

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

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/web/fetch/fetch-tasklet-deref-race.test.ts:17-33 Stress test lacks explicit timeout; ~4s runtime vs 5s default risks local flakes

Annotations

Check warning on line 33 in test/js/web/fetch/fetch-tasklet-deref-race.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Stress test lacks explicit timeout; ~4s runtime vs 5s default risks local flakes

This stress test passes no third `timeout` argument to `test(...)`, so it inherits the 5000ms default — but the PR description states the fixture takes ~4s on a debug+ASAN build, leaving essentially no headroom for slower or loaded machines on local `bun test` runs (CI is covered by `runner.node.mjs`'s `--timeout` override). Every sibling subprocess stress test in `fetch-leak.test.ts` sets an explicit timeout (30000-100000ms) for exactly this reason; suggest adding e.g. `}, 30_000);` as the thir