event_loop: make DeferredTaskQueue::run tolerate re-entrant map mutation - #32703
Conversation
H2FrameParser::on_auto_flush calls flush -> uncork -> unregister_auto_flush, which removes its own entry from the DeferredTaskQueue map while run() is still iterating, and then returns true. run() cached the map length in 'last' and only re-synced it when a callback returned false, so the next iteration indexed past the new length and panicked with 'index out of bounds: the len is 1 but the index is 1'. run() now re-reads map.len() on each iteration, bounds the pass to the initial entry count so entries appended mid-run are left for the next pass, and after the callback re-checks whether the current key is still at the same position before advancing. unregister_task() now uses swap_remove (O(1), matches the original Zig) instead of ordered remove. The test registers two auto-flushers in one microtask (an h2 client request and a small Bun.serve direct-stream write) so both share one run() pass; the h2 entry removes itself mid-iteration.
|
Updated 12:58 PM PT - Jul 10th, 2026
❌ @robobun, your commit 0a4c96b has 2 failures in
🧪 To try this PR locally: bunx bun-pr 32703That installs a local version of the PR into your bun-32703 --bun |
|
Warning Review limit reached
More reviews will be available in 10 minutes and 18 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughDeferredTaskQueue::unregister_task and DeferredTaskQueue::run now use swap-based removals, bound each drain pass, skip tombstones, and re-check slot ownership after callbacks mutate the map. The HTTP/2 test harness adds timeout scaling and a new fixture-backed regression case. ChangesDeferred task queue re-entrancy
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/event_loop/DeferredTaskQueue.rs`:
- Around line 76-80: The DeferredTaskQueue::pop_front_until pass is letting tail
swaps expose newly posted tasks in the same iteration, which breaks the intended
append-later behavior. Update the loop in DeferredTaskQueue::pop_front_until so
it only processes the original frontier for this pass, either by tracking the
initial length/frontier separately from the live map or by avoiding
swap_remove_at for in-pass removals. Make sure the callback path that can post
new tasks (the logic around the callback and the subsequent removal/requeue
handling) never causes a fresh tail entry to be run before the next pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b51629f5-df37-4c04-89bc-1083a90ec850
📒 Files selected for processing (3)
src/event_loop/DeferredTaskQueue.rstest/js/node/http2/node-http2-deferred-task-queue.fixture.jstest/js/node/http2/node-http2-streams-rehash.test.ts
The counter is a livelock bound, not a strict guarantee that newly appended entries never run in the current pass (a swap-remove can pull one forward). Any entry skipped in one pass runs on the next; each callback flushes an independent buffer, so order within a pass has no correctness effect.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs and the fix looks sound, but this touches the core event-loop drain path with subtle re-entrant iteration semantics, so it's worth a human pass.
Extended reasoning...
Overview
This PR fixes an index-out-of-bounds panic in DeferredTaskQueue::run (src/event_loop/DeferredTaskQueue.rs) when an on_auto_flush callback mutates the map mid-iteration and returns true. The loop now re-reads len() each iteration, re-checks slot ownership before advancing, and bounds the pass with a remaining counter. unregister_task is switched from ordered remove() to O(1) swap_remove(). A regression fixture and test are added.
Security risks
None identified. The change is purely internal iteration logic over an in-process map of auto-flusher pointers; no input parsing, auth, or external-data handling is affected.
Level of scrutiny
High. DeferredTaskQueue::run executes after every microtask drain across the entire runtime, so a regression here would be broad. The new logic handles several re-entrant mutation cases (self-removal, removal of other entries, append-during-callback, swap-into-current-slot), and while my trace-through of those cases checks out, the reasoning is subtle enough that a maintainer who owns this code should confirm the invariants — particularly the interaction between the !keep swap-remove-by-key path and a callback that has already moved/removed its own entry, and the intentional change of unregister_task from order-preserving to swap semantics.
Other factors
- The bug hunter found no issues; CodeRabbit's one finding was discussed and withdrawn (the swap-forward behavior is pre-existing and order is not a correctness contract here).
- API usage verified:
ArrayHashMap::swap_remove(&K) -> boolandswap_remove_at(usize)both exist insrc/collections/array_hash_map.rs. - A reproducer test is included and the PR description states it panics 10/10 on 1.4.0.
- CI shows build failures on aarch64-android and freebsd-x64 build-rust jobs, plus Windows aarch64 agent-creation failures; worth confirming whether those are infra-only before merge.
|
CI status as of build 71574 (sha 0a4c96b, branch merged with current main). The new test, "DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true", passes on every lane; it has not appeared in a failure annotation on any of this PR's builds. Two error-level failures, neither related to this change:
Verification on the merged tree with the debug build: with |
There was a problem hiding this comment.
I didn't find any bugs and the iteration logic looks sound, but this rewrites the drain loop in core event-loop code (DeferredTaskQueue::run, called on every microtask drain) with subtle re-entrancy/swap-remove reasoning, so it's worth a human pass.
Extended reasoning...
Overview
This PR fixes an index-out-of-bounds panic in DeferredTaskQueue::run (src/event_loop/DeferredTaskQueue.rs) that occurs when an on_auto_flush callback mutates the map mid-iteration and returns true. The fix re-reads self.map.len() each iteration, re-checks slot ownership after each callback, bounds the pass with a remaining counter, and switches unregister_task from ordered remove() to O(1) swap_remove(). A new fixture + test reproduces the panic via H2FrameParser + HTTPServerWritable sharing one drain pass.
Security risks
None. No untrusted input parsing, auth, crypto, or permissions. The change is internal iteration mechanics over a map of native function pointers that are only populated by Bun's own auto-flusher registrations.
Level of scrutiny
High. DeferredTaskQueue::run runs after every microtask drain in every Bun process — it is a hot, critical event-loop path. The fix involves non-trivial reasoning about re-entrant map mutation during index-based iteration: swap-remove pulling tail entries forward, the remaining livelock bound vs. a strict frontier, and the slot-ownership re-check after the callback. The unregister_task change from ordered remove to swap-remove is also a semantic change (restoring Zig parity per the description, but still a behavior change). I traced the branches and they look correct for the documented contract — entries that get skipped stay in the map for the next pass, and the !keep branch's remove-by-key correctly no-ops if the callback already self-unregistered. But this is exactly the kind of code where a second pair of eyes on the edge cases is valuable.
Other factors
- The bug-hunting system found no issues.
- CodeRabbit raised a frontier-invariant concern that was convincingly rebutted (swap-forward existed before, intra-pass order is not a correctness requirement here, skipped entries run next pass) and withdrawn; the comment was reworded in 800ac36 to clarify
remainingis a livelock bound, not a strict frontier. - A targeted regression test is added and passes on all CI lanes; the 3 CI failures (napi.test.ts, bundler_defer.test.ts, node-tls-connect.test.ts on Windows) are documented as pre-existing/unrelated and also failing on other PR builds.
- No CODEOWNERS entry covers this path.
… debug builds node-http2-streams-rehash.test.ts has a pre-existing test that times out under the debug+ASAN build, which masked this file's other tests. The DeferredTaskQueue test is about the event loop, not h2 stream rehashing, so give it its own file named after the fixture. The spawned fixture takes ~130ms in release but ~8s under debug+ASAN (crash reporting on the failing path dominates), so the default 5s per-test budget killed the child before it could exit. Scale the timeout the same way node-http2.test.js does.
isASAN is detected via the isASANEnabled() runtime probe in harness.ts, not by binary name, so it is true under bun-debug. The previous wording claimed the opposite. The multiplier itself is unchanged.
There was a problem hiding this comment.
Thanks for fixing the comment — no issues found in the logic, but since this rewrites the iteration in DeferredTaskQueue::run (which sits on the microtask drain path and has subtle re-entrancy semantics), I'd like a human to sign off.
Extended reasoning...
Overview
This PR fixes an index-out-of-bounds panic in DeferredTaskQueue::run (src/event_loop/DeferredTaskQueue.rs) when an on_auto_flush callback re-entrantly mutates the map and returns true. The loop now re-reads self.map.len() every iteration, only advances i if the key is still at slot i after the callback, and bounds the pass with a remaining counter to prevent livelock. unregister_task is switched from ordered remove() to O(1) swap_remove(). A regression fixture (node-http2-deferred-task-queue.fixture.js) and test reproduce the panic 10/10 on 1.4.0.
Security risks
None. No untrusted input parsing, auth, crypto, or permissions are touched; this is internal scheduling logic operating on already-registered native callback pointers.
Level of scrutiny
High. DeferredTaskQueue::run is invoked from drain_microtasks_with_global and runs after every microtask drain in every Bun process. The re-entrancy reasoning — swap-remove pulling tail entries forward, the keys().get(i) == Some(&key) check deciding whether to advance, removing by key vs. index on the !keep path, and the remaining livelock bound — is subtle enough that a maintainer familiar with the auto-flush callback contract should confirm the invariants. I walked through the cases (callback removes self + returns true, removes a different entry, appends a new entry, removes self then re-posts) and they look correct, but this is not a mechanical change.
Other factors
- The CodeRabbit concern about tail-swap pulling fresh entries forward was discussed and withdrawn; the author's response (pre-existing behavior, per-entry flush order is not semantically meaningful,
remainingis a livelock bound not a strict frontier) is reflected in the in-code comment. - My prior nit about the inaccurate
isASANrationale in the test comment was addressed in e5cf4e9. - I verified
ArrayHashMap::swap_remove(&K) -> boolandswap_remove_at(usize)exist with the expected signatures. - CI on build 64706 was green for the new test across all lanes; the author's failure analysis attributes the remaining failures to unrelated flakes/pre-existing main breakage.
…est.ts Adding a new test file changes which files share a CI shard: the runner assigns tests by index modulo max-shards over the test list (scripts/runner.node.mjs), so one inserted filename shifts every later file to a different shard on every lane. That reshuffle put node-tls-connect.test.ts (which talks to the live bun.sh) next to new neighbors and it started failing on 13 lanes in build 66237, while build 64706 (same src, no extra file) was green. Keeping the test in the existing file restores that layout. The reason it was split out in the first place was that the neighboring options-getter test times out at the default 5s budget under debug+ASAN. It is not hung: it passes in 5.9s given a budget. Give it and the new test the same debug/ASAN-scaled timeout that node-http2.test.js uses.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/node/http2/node-http2-streams-rehash.test.ts`:
- Around line 5-10: The rationale comment near the ASAN_MULTIPLIER declaration
is too long and must be shortened to 3 lines or less. Condense the explanation
in the test file by keeping only the essential reason for the multiplier choice,
or move the extra background into the PR description while leaving a brief
comment next to ASAN_MULTIPLIER.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f44ae7f-fb9f-408f-90eb-271d97465277
📒 Files selected for processing (1)
test/js/node/http2/node-http2-streams-rehash.test.ts
The inline comment had grown past the three line limit. Keep the mechanics at the call site and put the contract (re-entrant mutation, the livelock bound, why intra-pass order does not matter) in the file level doc block.
There was a problem hiding this comment.
Both earlier nits are addressed and I found nothing further, but this rewrites the iteration of a core event-loop hot path under re-entrant mutation (with an unsafe callback dispatch and a remove → swap_remove semantics change), so it's worth a human sign-off.
Extended reasoning...
Overview
This PR fixes an index-out-of-bounds panic in DeferredTaskQueue::run (src/event_loop/DeferredTaskQueue.rs), which runs after every microtask drain. The previous loop cached self.map.len() in last and only refreshed it on the keep == false path; H2FrameParser::on_auto_flush removes its own entry mid-iteration and returns true, so with a second flusher behind it the next index read past the shrunk map. The fix re-reads len() every iteration, re-checks whether key is still at slot i before advancing, and bounds the pass with a remaining counter initialized to the entry count at start. unregister_task is also switched from ordered remove() to O(1) swap_remove(), matching the original Zig and the new iteration scheme. A new fixture (node-http2-deferred-task-queue.fixture.js) reproduces the panic 10/10 on 1.4.0, and the test is added to node-http2-streams-rehash.test.ts with an ASAN/debug timeout multiplier.
Security risks
None identified. No new inputs are parsed, no auth/crypto/permissions surface is touched, and the unsafe block (invoking the registered extern "C" callback with its own pointer) is unchanged in contract from the previous code — only the surrounding iteration was rewritten.
Level of scrutiny
High. DeferredTaskQueue::run sits directly in the microtask-drain path (drain_microtasks_with_global), so every Bun process exercises it constantly. The new loop has to be correct under several re-entrant cases (callback removes itself, removes another entry before/after i, appends a new entry, returns false after self-removal), and the remove → swap_remove change in unregister_task alters how indices shift for all callers, not just run(). I traced the cases and believe the logic is sound — the keys().get(i) == Some(&key) re-check plus the i < self.map.len() loop guard cover the self-removal and shrink cases, and the file doc now explicitly states that intra-pass order and the strict frontier are non-requirements — but this is exactly the kind of subtle, hot-path concurrency-adjacent change a maintainer should eyeball rather than take on bot approval.
Other factors
All prior review threads are resolved: CodeRabbit's swap-forward concern was rebutted (pre-existing behavior, order-insensitive flushers, remaining is a livelock bound not a frontier) and withdrawn; my two comment-length/accuracy nits were addressed in ab84bbd and 17d2a84. The author verified locally that the new test panics with the fix reverted and passes with it applied, and reports it green on every CI lane. The remaining CI red (bundler_defer.test.ts on Windows 2019, x64-musl build failures) is unrelated per the author's CI-status comment. The bug-hunting system found nothing in this run.
|
Independent confirmation of this one from the Sentry side, plus a second reproduction that may be worth folding in. This is Sentry Second repro: http2 session + an unread
|
|
Still reproducing as of today. This is the #6 most frequent Windows flaky test across the last 40 PR builds (10 hits on Fresh symbolication against the 91675d0 baseline PDB (bun.exe and bun-profile.exe are byte-identical, so the profile PDB works for both): The fixture in this PR crashes 3/3 on today's main on both Linux x64 (1498d7b) and Windows x64-baseline (91675d0), so it is still a deterministic fail-before. The only CI failure on build 66425 is |
…d-task-queue-reentrant
There was a problem hiding this comment.
I didn't find any bugs and both of my earlier nits are addressed, but this rewrites the re-entrant iteration logic in DeferredTaskQueue::run — the post-microtask-drain hot path — so a maintainer familiar with the auto-flusher call graph should sign off on the intra-pass ordering and livelock-bound reasoning.
Extended reasoning...
Overview
This PR fixes an index-out-of-bounds panic in src/event_loop/DeferredTaskQueue.rs by making run() tolerate callbacks that re-entrantly mutate the map (the concrete trigger is H2FrameParser::on_auto_flush unregistering itself mid-iteration and returning true). The loop now re-reads len() each iteration, re-checks slot ownership after each callback, and bounds the pass with a remaining counter. unregister_task also switches from ordered remove() to O(1) swap_remove(). A new fixture (node-http2-deferred-task-queue.fixture.js) plus a test in node-http2-streams-rehash.test.ts deterministically reproduce the panic on main, and the test file gains debug/ASAN timeout scaling.
Security risks
None identified. There is no untrusted-input parsing, auth, crypto, or filesystem path handling here — the change is purely internal iteration bookkeeping over an in-process map of native auto-flusher pointers.
Level of scrutiny
High. DeferredTaskQueue::run executes after every microtask drain, so it is one of the hottest and most latency-sensitive paths in the runtime. The re-entrancy contract is subtle: the CodeRabbit thread already surfaced a swap-remove-pulls-fresh-entry-forward edge case that took a four-point argument to dismiss, and the correctness of the "only advance i if key is still at slot i" / "remove by key on !keep" pair depends on the documented claim that intra-pass ordering has no correctness effect and that skipped entries safely run next pass. I traced the cases and believe the logic is sound, and I confirmed ArrayHashMap::swap_remove(&K) -> bool and swap_remove_at(usize) both exist with the expected signatures — but this is exactly the class of change where a maintainer who owns the H2/FileSink/HTTPServerWritable auto-flush surfaces should confirm the invariants rather than a bot.
Other factors
All prior review threads are resolved (CodeRabbit's frontier concern withdrawn; my two comment-length/accuracy nits fixed in e5cf4e9, ab84bbd, 17d2a84). CI on build 66425 is green except for an unrelated Windows bundler_defer.test.ts flake. The author verified fail-before/pass-after on the merged tree, and there is an independent second reproduction (h2 + Bun.spawn stdin FileSink) plus a live Sentry issue (BUN-3NC8) and #6-ranked Windows flaky-test correlation, so the fix is well-motivated and well-tested — it just warrants human eyes on the event-loop iteration change itself.
* upstream/main: (422 commits) install: drop packages held only by optional-peer resolution slots from bun.lock (oven-sh#35681) Update mimalloc to the upstream dev3 (v3.4.3) sync (oven-sh#36431) compile(pe): ftruncate the Windows --compile output after writing (oven-sh#36430) Strong: back bun_jsc::Strong with StrongRootBlock; free AbortSignal.timeout at wrapper GC (oven-sh#35849) test(harness): replace toRun matcher with async bunRun + toSpawn (oven-sh#36424) test: measure memory via harness rss() instead of process.memoryUsage.rss() (oven-sh#36429) Deflake a few tests no-orphans(windows): allow CREATE_BREAKAWAY_FROM_JOB and set DIE_ON_UNHANDLED_EXCEPTION on the Job (oven-sh#36414) GarbageCollectionController: replace per-tick heap sampler with idle timer only (oven-sh#35356) exe_format(pe): write a valid OptionalHeader.CheckSum for --compile output (oven-sh#36383) FileSink: flush buffered bytes when process.exit() runs in the same tick as write() (oven-sh#36250) test(http): speed up and de-flake serve-async-stream-client-abort.test.ts (oven-sh#35919) test(20144): stop racing child startup against the 1s SIGKILL guard (oven-sh#34166) test(no-orphans): skip fast-exit perl daemon test on macOS (oven-sh#36413) fs: return negative BigIntStats *Ns for pre-epoch timestamps (oven-sh#36187) event_loop: make DeferredTaskQueue::run tolerate re-entrant map mutation (oven-sh#32703) dotenv: stop panicking on nested `${...}` inside `${VAR:-default}` (oven-sh#36199) fetch: make the idle timer an absolute deadline for the response header block (oven-sh#36145) bundler: don't panic on unterminated naming template placeholders (oven-sh#36325) Buffer#indexOf/lastIndexOf: rare-byte SIMD filter with a Two-Way O(n+m) fallback (oven-sh#36420) ... # Conflicts: # src/jsc/bindings/BunDebugger.cpp
* upstream/main: (422 commits) install: drop packages held only by optional-peer resolution slots from bun.lock (oven-sh#35681) Update mimalloc to the upstream dev3 (v3.4.3) sync (oven-sh#36431) compile(pe): ftruncate the Windows --compile output after writing (oven-sh#36430) Strong: back bun_jsc::Strong with StrongRootBlock; free AbortSignal.timeout at wrapper GC (oven-sh#35849) test(harness): replace toRun matcher with async bunRun + toSpawn (oven-sh#36424) test: measure memory via harness rss() instead of process.memoryUsage.rss() (oven-sh#36429) Deflake a few tests no-orphans(windows): allow CREATE_BREAKAWAY_FROM_JOB and set DIE_ON_UNHANDLED_EXCEPTION on the Job (oven-sh#36414) GarbageCollectionController: replace per-tick heap sampler with idle timer only (oven-sh#35356) exe_format(pe): write a valid OptionalHeader.CheckSum for --compile output (oven-sh#36383) FileSink: flush buffered bytes when process.exit() runs in the same tick as write() (oven-sh#36250) test(http): speed up and de-flake serve-async-stream-client-abort.test.ts (oven-sh#35919) test(20144): stop racing child startup against the 1s SIGKILL guard (oven-sh#34166) test(no-orphans): skip fast-exit perl daemon test on macOS (oven-sh#36413) fs: return negative BigIntStats *Ns for pre-epoch timestamps (oven-sh#36187) event_loop: make DeferredTaskQueue::run tolerate re-entrant map mutation (oven-sh#32703) dotenv: stop panicking on nested `${...}` inside `${VAR:-default}` (oven-sh#36199) fetch: make the idle timer an absolute deadline for the response header block (oven-sh#36145) bundler: don't panic on unterminated naming template placeholders (oven-sh#36325) Buffer#indexOf/lastIndexOf: rare-byte SIMD filter with a Two-Way O(n+m) fallback (oven-sh#36420) ...
* upstream/main: (422 commits) install: drop packages held only by optional-peer resolution slots from bun.lock (oven-sh#35681) Update mimalloc to the upstream dev3 (v3.4.3) sync (oven-sh#36431) compile(pe): ftruncate the Windows --compile output after writing (oven-sh#36430) Strong: back bun_jsc::Strong with StrongRootBlock; free AbortSignal.timeout at wrapper GC (oven-sh#35849) test(harness): replace toRun matcher with async bunRun + toSpawn (oven-sh#36424) test: measure memory via harness rss() instead of process.memoryUsage.rss() (oven-sh#36429) Deflake a few tests no-orphans(windows): allow CREATE_BREAKAWAY_FROM_JOB and set DIE_ON_UNHANDLED_EXCEPTION on the Job (oven-sh#36414) GarbageCollectionController: replace per-tick heap sampler with idle timer only (oven-sh#35356) exe_format(pe): write a valid OptionalHeader.CheckSum for --compile output (oven-sh#36383) FileSink: flush buffered bytes when process.exit() runs in the same tick as write() (oven-sh#36250) test(http): speed up and de-flake serve-async-stream-client-abort.test.ts (oven-sh#35919) test(20144): stop racing child startup against the 1s SIGKILL guard (oven-sh#34166) test(no-orphans): skip fast-exit perl daemon test on macOS (oven-sh#36413) fs: return negative BigIntStats *Ns for pre-epoch timestamps (oven-sh#36187) event_loop: make DeferredTaskQueue::run tolerate re-entrant map mutation (oven-sh#32703) dotenv: stop panicking on nested `${...}` inside `${VAR:-default}` (oven-sh#36199) fetch: make the idle timer an absolute deadline for the response header block (oven-sh#36145) bundler: don't panic on unterminated naming template placeholders (oven-sh#36325) Buffer#indexOf/lastIndexOf: rare-byte SIMD filter with a Two-Way O(n+m) fallback (oven-sh#36420) ... # Conflicts: # src/js/internal/debugger.ts
* upstream/main: (422 commits) install: drop packages held only by optional-peer resolution slots from bun.lock (oven-sh#35681) Update mimalloc to the upstream dev3 (v3.4.3) sync (oven-sh#36431) compile(pe): ftruncate the Windows --compile output after writing (oven-sh#36430) Strong: back bun_jsc::Strong with StrongRootBlock; free AbortSignal.timeout at wrapper GC (oven-sh#35849) test(harness): replace toRun matcher with async bunRun + toSpawn (oven-sh#36424) test: measure memory via harness rss() instead of process.memoryUsage.rss() (oven-sh#36429) Deflake a few tests no-orphans(windows): allow CREATE_BREAKAWAY_FROM_JOB and set DIE_ON_UNHANDLED_EXCEPTION on the Job (oven-sh#36414) GarbageCollectionController: replace per-tick heap sampler with idle timer only (oven-sh#35356) exe_format(pe): write a valid OptionalHeader.CheckSum for --compile output (oven-sh#36383) FileSink: flush buffered bytes when process.exit() runs in the same tick as write() (oven-sh#36250) test(http): speed up and de-flake serve-async-stream-client-abort.test.ts (oven-sh#35919) test(20144): stop racing child startup against the 1s SIGKILL guard (oven-sh#34166) test(no-orphans): skip fast-exit perl daemon test on macOS (oven-sh#36413) fs: return negative BigIntStats *Ns for pre-epoch timestamps (oven-sh#36187) event_loop: make DeferredTaskQueue::run tolerate re-entrant map mutation (oven-sh#32703) dotenv: stop panicking on nested `${...}` inside `${VAR:-default}` (oven-sh#36199) fetch: make the idle timer an absolute deadline for the response header block (oven-sh#36145) bundler: don't panic on unterminated naming template placeholders (oven-sh#36325) Buffer#indexOf/lastIndexOf: rare-byte SIMD filter with a Two-Way O(n+m) fallback (oven-sh#36420) ...
2 participants
Panic
Cause
DeferredTaskQueue::runwalksself.mapby index, caching the length inlastand invoking each entry'son_auto_flushcallback.lastwas only re-synced when a callback returnedfalse(remove). When a callback mutated the map and returnedtrue, the loop didi += 1with a stalelast, so the nextself.map.keys()[i]indexed past the new length.H2FrameParser::on_auto_flushhits this every time it runs with another auto-flusher behind it: it callsflush -> uncork -> unregister_auto_flush, removing its own entry mid-iteration, then returnstrue.Reproduction
Register two auto-flushers in one microtask so they share one
run()pass: an h2 client request (corks theH2FrameParser, registers it at index 0) and a small write on aBun.servedirect-stream controller (buffers and registersHTTPServerWritableat index 1). Whenrun()processes the h2 entry it removes itself and returnstrue; the next index is one past the shrunk map.Added as
test/js/node/http2/node-http2-deferred-task-queue.fixture.js; panics 10/10 on 1.4.0.Fix
run()now re-readsself.map.len()on every iteration, and after a callback returnstrueit only advancesiifkeyis still at positioni(otherwise it re-processes whatever got swapped in). Aremainingcounter initialized to the entry count at the start of the pass bounds the loop so entries appended mid-run are left for the nextrun()and a callback that removes-and-reposts can't spin.unregister_taskis also switched fromremove()(ordered, O(n) with index rebuild) toswap_remove()(O(1)), matching the original Zig behaviour and the iteration scheme above.