node:http2: arm the dispatch guard around the streamStart callback - #37273
Conversation
handle_received_stream_id invoked the JS streamStart callback while holding the just-created *mut Stream without arming the DispatchGuard. JS reached from inside the callback could close the stream (queueing its deferred free) and re-enter parser.read() at dispatch depth 0, where rewrite_read's pending-close drain frees the Stream box; the callback return path then wrote the stream context through the dangling pointer (heap-use-after-free in Stream::set_context under ASAN). Arm the guard across the callback like every other JS dispatch site, so the drain stays deferred while the pointer is live, and skip the context install when the callback closed the stream: free_resources already dropped its sctx root, and re-rooting it would pin the dead JS stream object until the session dies.
|
Updated 12:08 PM PT - Aug 9th, 2026
❌ @robobun, your commit 8f79562 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 37273That installs a local version of the PR into your bun-37273 --bun |
WalkthroughThe HTTP/2 parser now defers stream cleanup during ChangesHTTP/2 stream lifecycle
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 `@test/js/node/http2/node-http2-streams-rehash.test.ts`:
- Around line 174-175: Remove the local timeout override associated with the
child-process test in node-http2-streams-rehash.test.ts, including the 10_000 *
ASAN_MULTIPLIER timeout configuration. Rely on the suite-level timeout while
preserving the bounded test sequence and all other test behavior.
🪄 Autofix
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: fffaad18-e9b5-40d4-99ec-7d967cfc50ef
📒 Files selected for processing (2)
src/runtime/api/bun/h2_frame_parser.rstest/js/node/http2/node-http2-streams-rehash.test.ts
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Looked at #33191: it does carry the same This PR is the focused fix for that hole on main (live today, ASAN repro in the test), plus the skip of the context install when the callback closed the stream, which #33191 does not have. If #33191 rebases and lands first the guard hunk resolves trivially; the test and the context-install skip here stay useful either way. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it's a memory-safety fix in the HTTP/2 frame parser (network-facing native code with raw *mut Stream lifetime reasoning), a human look would still be worthwhile.
What was reviewed:
- Confirmed
rewrite_read'sdispatch_depth == 0drain is the only free path for the box, so armingenter_dispatchacross the callback is sufficient to keepstreamvalid throughset_context. - Checked callers (
get_next_stream, the threerequest()header-error paths, line 9153) — after the guard drops, they dereference the returned pointer in straight-line native code before any further JS can run, so the deferred free stays queued. - Verified
free_resourcesis the sole producer ofpending_engine_stream_closesand drops thesctxroot, so the new.contains(&stream_identifier)skip correctly avoids re-rooting a dead stream.
Extended reasoning...
Overview
The PR arms enter_dispatch() around the streamStart JS callback in H2FrameParser::handle_received_stream_id, which was the one JS dispatch site in the parser that ran user code while holding a raw *mut Stream without the guard. It also adds a pending_engine_stream_closes.contains(id) check to skip re-installing the stream context when the callback synchronously closed the stream. A new subprocess test drives the parser directly, patching EventEmitter.prototype.on to close the new stream and re-enter parser.read() from inside the callback — the exact ASAN-reproduced sequence.
Security risks
The affected code is the HTTP/2 frame parser — network-facing, memory-safety-critical. The change strictly hardens: it adds a guard that other dispatch sites already use and skips a write when the target is already dead. No new attack surface is introduced. The UAF being fixed required hostile JS in-process (patching EventEmitter.prototype.on), not merely hostile network bytes, but the fix is still the right layer.
Level of scrutiny
High. This is unsafe Rust dereferencing a raw pointer whose lifetime is governed by a reentrancy invariant ("every section that holds a Stream pointer while user JS can run must arm the guard"). I traced the invariant: the only free path for the box is rewrite_read's drain at h2_frame_parser.rs:5805-5821, gated on dispatch_depth == 0; enter_dispatch bumps that counter and the DispatchGuard Drop decrements it. The _dispatch local lives to the end of handle_received_stream_id, covering both the callback and the subsequent set_context write. After return, callers at lines 8898/9065/9135/9153 deref the pointer without JS running in between (line 9153 immediately arms its own enter_stream_dispatch), so the still-queued free cannot fire under them.
Other factors
- The fix matches the documented invariant on
enter_dispatch(line 2733-2735) exactly, and the pattern used at every sibling dispatch site. - The
free_resourcescode (line 2189-2197) confirms it both queues the id and removes thesctxentry, so the PR's rationale for the re-root skip is accurate. - Test follows file conventions (spawned fixture, combined stdout/stderr/exitCode assertion, ASAN timeout multiplier) and asserts the hook actually fired (
hookedguard) so it can't pass vacuously. - Multi-agent bug hunting found no issues.
Deferring only because raw-pointer lifetime changes in a network protocol parser warrant human sign-off per the approval bar; the change itself looks correct.
getStreamContext(2) after the callback proves the native entry survived (pre-fix the drain freed it inside the callback and this throws on every build tier, not only under ASAN) and that no context was installed for the closed stream. A follow-up depth-0 read then proves the deferred close actually drains. Also note in handle_received_stream_id why the guard is armed bare rather than via enter_stream_dispatch: rst_stream reached from the callback forms its own &mut to the same stream, so no &mut may be held across the call.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Reborrowing via the helper after the callback returns replaces the site-local unsafe deref and its safety prose; the helper already owns that reasoning. The guard-arming and skip-when-closed comments are trimmed to the invariant references.
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. The guard-arming and context-skip both follow the file's documented dispatch-depth invariant, and the test asserts each of the fix's observable effects. Because this is a UAF fix in the HTTP/2 frame parser's raw-pointer lifecycle path, a maintainer familiar with this file's stream ownership model should still take a look.
What was reviewed:
- Confirmed
enter_dispatch()at 5361 keepsdispatch_depth > 0across the callback so the drain at 5803 cannot free the box; guard scope coversset_contextand the return. - Checked callers of
handle_received_stream_id(get_next_stream, request() at 8896/9063/9133/9151, engine HEADERS at 6192) — none deref the returned pointer across further JS without their own guard. - Verified
pending_engine_stream_closesis populated byfree_resourcesand only drained at depth 0, so the.contains()check reliably detects a callback-closed stream. - Test asserts hook fired, stream survived the callback, no context installed for the closed stream, and a follow-up depth-0 read drains it — covers both the guard and the skip-install branch.
Extended reasoning...
Overview
This PR fixes a heap-use-after-free in H2FrameParser::handle_received_stream_id (src/runtime/api/bun/h2_frame_parser.rs). The function creates a Stream box, inserts it into the stream map, then invokes the JS streamStart callback while holding the raw *mut Stream — but without arming the DispatchGuard. If JS reached from that callback closes the stream (queueing it in pending_engine_stream_closes) and then re-enters parser.read() at depth 0, the drain at line 5803 frees the box, and the return path writes through a dangling pointer via set_context. The fix arms enter_dispatch() across the callback (matching every other JS dispatch site in the file and the invariant documented on enter_dispatch at line 2733), and additionally skips the context install when the callback queued the stream for close (so the dead JS stream isn't re-rooted in sctx). The raw unsafe { (*stream).set_context(...) } is replaced with enter_stream_dispatch(stream).set_context(...), moving the safety reasoning into the shared helper. A new subprocess test in node-http2-streams-rehash.test.ts reproduces the exact reentrancy sequence.
Security risks
The affected code is the HTTP/2 frame parser handling server-push / server-side stream creation. A UAF in this path is a memory-safety issue in a network-facing subsystem — the exact class of bug the repo's review guidance treats as most-blocked. The fix itself introduces no new attack surface: it arms an existing depth counter and adds a Vec::contains check on an internal queue. I did not identify any new security regression, but the sensitivity of the subsystem is why I'm not auto-approving.
Level of scrutiny
High. This is unsafe Rust manipulating heap-allocated raw pointers across reentrant JS callbacks in a network protocol parser. The change is small (~15 native lines) and mechanically consistent with the file's established pattern — every other dispatch wrapper (dispatch, call, the option-getter sites in request()) already arms enter_dispatch. The reasoning for using the bare guard rather than enter_stream_dispatch across the callback (rst_stream from inside the callback takes its own &mut on the same stream, so the outer borrow must wait) is subtle but sound. Someone who owns this file's invariants should confirm the guard scope and the .contains() gate are the right shape.
Other factors
The PR includes strong evidence: ASAN heap-use-after-free without the fix, clean pass with it on both debug+ASAN and release, and a release-build failure mode ("Invalid stream id") that proves the test fails for the right reason on non-ASAN builds too. The test asserts every observable step (hook fired, stream survived callback, no context installed, deferred drain runs afterward), follows the file's subprocess + ASAN_MULTIPLIER timeout conventions, and restores the patched EE.prototype.on. All bot review threads (CodeRabbit timeout nit, comment-cop) are resolved. A related PR #33191 carries the same guard hunk but without the context-skip or test; robobun's response on the thread explains this PR is the focused, tested fix on current main.
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 177-182: Update the error handling around
parser.getStreamContext(2) to assert that the thrown exception is the expected
invalid-stream error, rather than setting drained for any exception. Preserve
the test’s success condition only when that specific error is observed, using
the repository’s established assertion and error-matching conventions.
🪄 Autofix
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: d8fe978f-d5bf-4860-ad96-ca7b77cbccb9
📒 Files selected for processing (2)
src/runtime/api/bun/h2_frame_parser.rstest/js/node/http2/node-http2-streams-rehash.test.ts
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it's a memory-safety fix in the native h2 frame parser — arming a reentrancy guard around a JS callback while a raw *mut Stream is held — a human look from someone familiar with the dispatch-depth invariants would still be worthwhile.
What was reviewed:
- The
enter_dispatch()guard matches the invariant documented on the helper (line 2733) and the pattern at every sibling dispatch site in this file. - The
pending_engine_stream_closes.contains(&id)check correctly detects a stream closed by the callback;free_resourcesis the only producer and the guard prevents the drain from running before the check. enter_stream_dispatch(stream)forset_contextis only reached when the stream was not closed, and the outer_dispatchis still armed, so the box is live.- The test hooks
EE.prototype.onto reach the window inside the native callback, restores it, asserts the deferred-close state and the exact drain error, and follows the file's spawn/timeout conventions.
Extended reasoning...
Overview
This PR fixes a heap-use-after-free in H2FrameParser::handle_received_stream_id (src/runtime/api/bun/h2_frame_parser.rs). The function creates a Stream box, inserts it into the map, then invokes the JS onStreamStart callback while holding the raw *mut Stream — but previously did so without arming the DispatchGuard. JS reached from inside the callback could rstStream the new stream (queuing it in pending_engine_stream_closes) and re-enter parser.read() at dispatch depth 0, where rewrite_read drains that queue and frees the box; on return the code wrote set_context through the freed pointer. The fix arms enter_dispatch() around the callback (so depth > 0 blocks the drain), skips the context install when the callback closed the stream (avoiding a re-root of the dead JS stream in sctx after free_resources already removed it), and routes set_context through enter_stream_dispatch instead of a site-local unsafe deref. A regression test in node-http2-streams-rehash.test.ts reproduces the exact reentrancy sequence and asserts both the deferred-close state and the eventual drain.
Security risks
This is a memory-safety fix (UAF) in native code. The vulnerability is real but requires an attacker to control JS running inside the process (patched EventEmitter.prototype.on or similar) — it's not a remote-only trigger. The fix strictly narrows behavior: it adds a guard and skips an install; it does not remove any existing check. I don't see new attack surface introduced.
Level of scrutiny
High. This is exactly the category the repo review guide flags as most-blocked: raw-pointer lifetime across a JS callback that can synchronously free the pointee. The fix is small and follows the documented file-local invariant (enter_dispatch's doc comment at line 2733 says it must be armed "across every JS dispatch wrapper AND every section that holds a &mut Stream while user JS can run"), and the drain site at line 5803 gates on dispatch_depth == 0, so the mechanism checks out. But the correctness still rests on subtle ordering: the bare guard (not enter_stream_dispatch) across the callback so rst_stream's own &mut doesn't alias; the contains check while the guard is still armed so a queued id can't have been drained yet; the nested enter_stream_dispatch only after the callback returns and only on the not-closed branch. A maintainer who owns this subsystem (cirospaciari per the CodeRabbit suggestion) should confirm those invariants match intent.
Other factors
The PR carries strong evidence: ASAN heap-use-after-free trace on the unfixed build, passing on the fixed build, and the release-build behavioral difference (getStreamContext(2) throws pre-fix). All bot review comments (CodeRabbit on the timeout and the drain-error assertion, comment-cop on comment length) are resolved — the author trimmed comments and moved the unsafe deref into the shared helper per feedback. The test is well-constructed: it asserts the hook actually fired, checks the exact "Invalid stream id" message, restores the prototype patch, and follows the file's established spawn/timeout pattern. There's a related open PR #33191 that carries the same guard hunk as part of a larger change; the author addressed the overlap and this PR additionally has the context-install skip that #33191 lacks. Given the memory-safety stakes and the subtlety of the reentrancy reasoning, I'm deferring rather than auto-approving.
|
CI status (final for build 91041): every build and test lane is green except darwin-14-x64, which failed its retry with the same two failures that are pre-existing on main and unrelated to this diff (test/napi/napi.test.ts napi_reference_unref, and test/cli/test/parallel.test.ts 64MB truncation timeout); both are reported for triage separately. The regression test here passed on all lanes including the ASAN ones, and the evidence check confirms it fails without the fix on both ASAN and release builds. Ready for review. |
…ven-sh#37273) ### Problem `H2FrameParser::handle_received_stream_id` creates a `Stream` box, inserts it into the stream map, and then invokes the JS `streamStart` callback directly via `callback.call` without arming the `DispatchGuard`, while still holding the raw `*mut Stream`. Every other JS dispatch site in the parser arms the guard, because `rewrite_read` frees streams queued in `pending_engine_stream_closes` only at dispatch depth 0. JS reached from inside that callback (the `Http2Stream` constructor calls `this.on("pause", ...)`, so a patched `EventEmitter.prototype.on` runs there; the handler also calls back into native `rstStream` for refused streams) can close the just-created stream, queueing its deferred free, and then re-enter `parser.read()` at depth 0. The drain frees the box, and the callback return path writes the stream context through the dangling pointer: ``` ==ERROR: AddressSanitizer: heap-use-after-free ... #1 <bun_runtime::api::h2_frame_parser_body::Stream>::set_context src/runtime/api/bun/h2_frame_parser.rs:2110 #2 <...H2FrameParser>::handle_received_stream_id src/runtime/api/bun/h2_frame_parser.rs:5372 #3 <...H2FrameParser>::get_next_stream src/runtime/api/bun/h2_frame_parser.rs:8335 freed by: #12 <...H2FrameParser>::rewrite_read::{closure#3} src/runtime/api/bun/h2_frame_parser.rs:5804 ``` The callers that keep dereferencing the returned pointer (`request()`, `get_next_stream`, the engine HEADERS path) were exposed to the same freed box. ### Fix Arm `enter_dispatch` across the callback, matching the invariant documented on `enter_dispatch` (every section that holds a `Stream` pointer while user JS can run must arm the guard). With the guard armed, the deferred-close drain cannot run while the callback executes, so the pointer stays valid for `set_context` and for the callers. Also skip the context install when the callback closed the stream: `free_resources` already dropped its `sctx` root, and re-inserting one afterwards would pin the dead JS stream object until the session dies. This is the guard-arming fix for the pre-existing issue flagged during review of oven-sh#37272 (that PR only removes dead code around it). ### Verification New test in `test/js/node/http2/node-http2-streams-rehash.test.ts` (the file covering this class of reentrancy bugs) reproduces the exact sequence: close the new stream and re-enter `read()` from inside the `streamStart` callback. Without the fix it fails on every build tier: heap-use-after-free under the ASAN debug build, and on release builds `getStreamContext(2)` throws "Invalid stream id" because the drain already freed the entry inside the callback. With the fix the entry survives the callback with no context installed (covering the skip-install branch), and a follow-up depth-0 `read()` asserts the deferred close then actually drains. Existing http2 suites (`node-http2.test.js`, `h2-conformance.test.ts`, the staged h2 tests, node's server-push parallel tests) pass with the change. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2-streams-rehash.test.ts" bun test v1.4.0 (8f79562) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [3284.09ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [6184.76ms] 198 | env: bunEnv, 199 | stdout: "pipe", 200 | stderr: "pipe", 201 | }); 202 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 203 | expect({ stdout: stdout.trim(), exitCode, stderr }).toMatchObject({ stdout: "OK", exitCode: 0 }); ^ error: expect(received).toMatchObject(expected) { - "exitCode": 0, - "stdout": "OK", + "exitCode": 1, + "stderr": + "================================================================= + ==101685==ERROR: AddressSanitizer: heap-use-after-free on address 0x79be9bb005c0 at pc 0x00000e7ec22e bp ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (7725ac8) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [163.99ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [78.42ms] (pass) closing the new stream and re-entering read() inside the streamStart callback does not UAF [31.29ms] (pass) http2 client write callback that opens new streams during flushQueue does not UAF [49.40ms] (pass) DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true [46.51ms] 5 pass 0 fail 5 expect() calls Ran 5 tests across 1 file. [513.00ms] __F:0:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2-streams-rehash.test.ts" bun test v1.4.0 (8f79562) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [3278.34ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [6171.66ms] (pass) closing the new stream and re-entering read() inside the streamStart callback does not UAF [1906.06ms] (pass) http2 client write callback that opens new streams during flushQueue does not UAF [2819.28ms] (pass) DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true [2618.43ms] 5 pass 0 fail 5 expect() calls Ran 5 tests across 1 file. [19.19s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 689ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/6] gen generated_host_exports.rs generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited [1/6] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/api/bun/h2_frame_parser.rs | 19 +++- .../node/http2/node-http2-streams-rehash.test.ts | 100 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/api/bun/h2_frame_parser.rs 10 4 0 test/js/node/http2/node-http2-streams-rehash.test.ts 2 3 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2 participants
Problem
H2FrameParser::handle_received_stream_idcreates aStreambox, inserts it into the stream map, and then invokes the JSstreamStartcallback directly viacallback.callwithout arming theDispatchGuard, while still holding the raw*mut Stream. Every other JS dispatch site in the parser arms the guard, becauserewrite_readfrees streams queued inpending_engine_stream_closesonly at dispatch depth 0.JS reached from inside that callback (the
Http2Streamconstructor callsthis.on("pause", ...), so a patchedEventEmitter.prototype.onruns there; the handler also calls back into nativerstStreamfor refused streams) can close the just-created stream, queueing its deferred free, and then re-enterparser.read()at depth 0. The drain frees the box, and the callback return path writes the stream context through the dangling pointer:The callers that keep dereferencing the returned pointer (
request(),get_next_stream, the engine HEADERS path) were exposed to the same freed box.Fix
Arm
enter_dispatchacross the callback, matching the invariant documented onenter_dispatch(every section that holds aStreampointer while user JS can run must arm the guard). With the guard armed, the deferred-close drain cannot run while the callback executes, so the pointer stays valid forset_contextand for the callers.Also skip the context install when the callback closed the stream:
free_resourcesalready dropped itssctxroot, and re-inserting one afterwards would pin the dead JS stream object until the session dies.This is the guard-arming fix for the pre-existing issue flagged during review of #37272 (that PR only removes dead code around it).
Verification
New test in
test/js/node/http2/node-http2-streams-rehash.test.ts(the file covering this class of reentrancy bugs) reproduces the exact sequence: close the new stream and re-enterread()from inside thestreamStartcallback. Without the fix it fails on every build tier: heap-use-after-free under the ASAN debug build, and on release buildsgetStreamContext(2)throws "Invalid stream id" because the drain already freed the entry inside the callback. With the fix the entry survives the callback with no context installed (covering the skip-install branch), and a follow-up depth-0read()asserts the deferred close then actually drains. Existing http2 suites (node-http2.test.js,h2-conformance.test.ts, the staged h2 tests, node's server-push parallel tests) pass with the change.[review] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 1
evidence per changed file