Skip to content

http2: fix onread assert when destroying session from stream handler - #65116

Open
sankalpsthakur wants to merge 1 commit into
nodejs:mainfrom
sankalpsthakur:fix/64850-http2-onread-assert
Open

http2: fix onread assert when destroying session from stream handler#65116
sankalpsthakur wants to merge 1 commit into
nodejs:mainfrom
sankalpsthakur:fix/64850-http2-onread-assert

Conversation

@sankalpsthakur

@sankalpsthakur sankalpsthakur commented Aug 7, 2026

Copy link
Copy Markdown

When session.destroy() is called from a 'stream' handler, MakeCallback drains nextTick while nghttp2 is still inside mem_recv. Session close is deferred for that window (from #64166), so later HEADERS in the same receive buffer created C++ streams without a JS wrapper / onread. Subsequent DATA delivery hit:

Assertion failed: onread->IsFunction()
  <- StreamBase::CallJSOnreadMethod
  <- Http2StreamListener::OnStreamRead
  <- Http2Session::OnDataChunkReceived

Changes

  1. Do not create streams while the session is closing (OnBeginHeadersCallback) — RST with NGHTTP2_CANCEL instead.
  2. If on_headers runs after JS destroy, destroy the C++ handle (mirrors the existing session.closed path).
  3. Defensive: drop DATA when onread is not a function rather than aborting.
  4. Regression test exercising concurrent POSTs with session.destroy() from the stream handler.

Test plan

  • New test/parallel/test-http2-session-destroy-stream-handler.js (40 rounds × 8 concurrent POSTs)
  • CI (this PR)
  • Local full Node build was still running at submit time; relying on CI for the binary test run.

Fixes #64850

AI/LLM disclosure

  • AI coding tools (including Grok and/or Codex agent-assisted editing) were used to help draft or modify code and this PR description.
  • I reviewed the complete change and understand the reasoning. Local full rebuild was in progress at submit; CI will run the suite.
  • This submission is original work of authorship under the project CLA / contributor terms; AI output was not pasted unreviewed.

When session.destroy() runs from a 'stream' handler, MakeCallback drains
nextTick while nghttp2 is still inside mem_recv. Close is deferred for
that window (see nodejs#64166), so later HEADERS in the same buffer created
C++ streams without a JS wrapper or onread, and DATA delivery aborted
with Assertion failed: onread->IsFunction().

- Reject new streams while the session is closing
- Destroy the C++ handle if on_headers runs after JS destroy
- Drop DATA when onread is not installed (defensive)

Fixes: nodejs#64850
Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http2: assertion failure onread->IsFunction() when a session is destroyed from a 'stream' handler

2 participants