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: #64850
Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
PR-URL: #65116
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>

Author: sankalpsthakur

lib/internal/http2/core.js

@@ -358,8 +358,15 @@ 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.rstStream(NGHTTP2_REFUSED_STREAM);
+    handle.destroy();
     return;
+  }
 
   const type = session[kType];
   session[kUpdateTimer]();

src/node_http2.cc

@@ -1049,6 +1049,16 @@ 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_REFUSED_STREAM);
+      return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+    }
     if (!session->CanAddStream() ||
         Http2Stream::New(session, id, frame->headers.cat) == nullptr)
         [[unlikely]] {

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();
+}));