FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: fix onread assert when destroying session from stream handler by sankalpsthakur · Pull Request #65116 · nodejs/node · GitHub

/ node Public

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 commented Aug 7, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

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)

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, understand the reasoning, and added the regression test listed above.
  • 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>

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL