From 2263be53ae58c034068129142fcdc542732941c9 Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Sat, 8 Aug 2026 00:07:24 +0530 Subject: [PATCH 1/3] http2: fix onread assert when destroying session from stream handler 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 #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: https://github.com/nodejs/node/issues/64850 Signed-off-by: Sankalp Thakur --- lib/internal/http2/core.js | 8 ++- src/node_http2.cc | 23 +++++++ ...st-http2-session-destroy-stream-handler.js | 60 +++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http2-session-destroy-stream-handler.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 200471dca0fd..1d2e7b727e04 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -364,8 +364,14 @@ function emit(self, ...args) { // the block of headers on. function onSessionHeaders(handle, id, cat, flags, headers, sensitiveHeaders) { const session = this[kOwner]; - if (session.destroyed) + // Session may have been destroyed mid-receive (e.g. session.destroy() from a + // 'stream' handler drained via nextTick inside MakeCallback while nghttp2 is + // still walking the receive buffer). Tear down the C++ stream so subsequent + // DATA frames do not call CallJSOnreadMethod with a missing onread. + if (session.destroyed) { + handle.destroy(); return; + } const type = session[kType]; session[kUpdateTimer](); diff --git a/src/node_http2.cc b/src/node_http2.cc index 04b2acca148d..bf5d275c8a8a 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1089,6 +1089,19 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle, // The common case is that we're creating a new stream. The less likely // case is that we're receiving a set of trailers if (!stream) [[likely]] { + // Close() may be deferred while mem_recv is in progress (see + // Http2Session::Close). A 'stream' handler that calls session.destroy() + // runs via nextTick from MakeCallback during that window, so later + // HEADERS in the same receive buffer must not create a C++ stream + // whose JS wrapper (and onread) is never installed. + if (session->is_closing()) { + nghttp2_submit_rst_stream( + session->session(), + NGHTTP2_FLAG_NONE, + id, + NGHTTP2_CANCEL); + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } if (!session->CanAddStream() || Http2Stream::New(session, id, frame->headers.cat) == nullptr) [[unlikely]] { @@ -1561,6 +1574,16 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { return; } + // Streams created after a deferred session close may never get a JS + // wrapper (handle.onread is only set in Http2Stream[kInit]). Drop the + // chunk instead of asserting in CallJSOnreadMethod. + Local onread = + stream->object() + ->GetInternalField(StreamBase::kOnReadFunctionField) + .As(); + if (!onread->IsFunction()) + return; + Local ab; if (session->stream_buf_ab_.IsEmpty()) { ab = ArrayBuffer::New(env->isolate(), diff --git a/test/parallel/test-http2-session-destroy-stream-handler.js b/test/parallel/test-http2-session-destroy-stream-handler.js new file mode 100644 index 000000000000..35c5b16471ef --- /dev/null +++ b/test/parallel/test-http2-session-destroy-stream-handler.js @@ -0,0 +1,60 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const http2 = require('http2'); + +// Regression test for https://github.com/nodejs/node/issues/64850 +// +// Destroying the session from a 'stream' handler runs (via nextTick drained +// from MakeCallback) while nghttp2 is still inside mem_recv. Close is deferred +// for that window; later HEADERS/DATA in the same buffer must not abort with +// Assertion failed: onread->IsFunction(). + +const STREAMS = 8; +const BODY = Buffer.alloc(2048, 'a'); +const ROUNDS = 40; + +const server = http2.createServer({ + settings: { maxConcurrentStreams: 4 }, +}); + +server.on('session', (session) => session.on('error', () => {})); + +server.on('stream', (stream) => { + stream.on('error', () => {}); + stream.session.destroy(); +}); + +server.listen(0, '127.0.0.1', common.mustCall(() => { + const port = server.address().port; + const origin = `http://127.0.0.1:${port}`; + let remaining = ROUNDS; + + const round = () => { + if (remaining-- <= 0) { + server.close(); + return; + } + + const session = http2.connect(origin); + session.on('error', () => {}); + session.on('close', () => setImmediate(round)); + + session.on('connect', () => { + for (let i = 0; i < STREAMS; i++) { + const stream = session.request({ + ':path': `/${i}`, + ':method': 'POST', + }); + stream.on('error', () => {}); + stream.resume(); + stream.end(BODY); + } + }); + }; + + round(); +})); From 5ef12f1c26eb044cf88b8bbfe76c76bea101f0eb Mon Sep 17 00:00:00 2001 From: Sankalp Thakur <31366524+sankalpsthakur@users.noreply.github.com> Date: Tue, 1 Sep 2026 01:42:42 +0530 Subject: [PATCH 2/3] http2: use refused stream on deferred close --- lib/internal/http2/core.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 1d2e7b727e04..c29f382ecf1b 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -369,6 +369,7 @@ function onSessionHeaders(handle, id, cat, flags, headers, sensitiveHeaders) { // still walking the receive buffer). Tear down the C++ stream so subsequent // DATA frames do not call CallJSOnreadMethod with a missing onread. if (session.destroyed) { + handle.rstStream(NGHTTP2_REFUSED_STREAM); handle.destroy(); return; } From ae6f128d17a237566558fc20f14309cfa6776d83 Mon Sep 17 00:00:00 2001 From: Sankalp Thakur <31366524+sankalpsthakur@users.noreply.github.com> Date: Tue, 1 Sep 2026 01:42:59 +0530 Subject: [PATCH 3/3] http2: address review feedback --- src/node_http2.cc | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index bf5d275c8a8a..f7c010fec888 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1096,10 +1096,7 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle, // whose JS wrapper (and onread) is never installed. if (session->is_closing()) { nghttp2_submit_rst_stream( - session->session(), - NGHTTP2_FLAG_NONE, - id, - NGHTTP2_CANCEL); + session->session(), NGHTTP2_FLAG_NONE, id, NGHTTP2_REFUSED_STREAM); return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } if (!session->CanAddStream() || @@ -1574,16 +1571,6 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { return; } - // Streams created after a deferred session close may never get a JS - // wrapper (handle.onread is only set in Http2Stream[kInit]). Drop the - // chunk instead of asserting in CallJSOnreadMethod. - Local onread = - stream->object() - ->GetInternalField(StreamBase::kOnReadFunctionField) - .As(); - if (!onread->IsFunction()) - return; - Local ab; if (session->stream_buf_ab_.IsEmpty()) { ab = ArrayBuffer::New(env->isolate(),