From 2263be53ae58c034068129142fcdc542732941c9 Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Sat, 8 Aug 2026 00:07:24 +0530 Subject: [PATCH] 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(); +}));