Ensure session close event fires before stream close callbacks

lib/internal/http2/core.js

@@ -1224,24 +1224,30 @@ function closeSession(session, code, error) {
   session.setTimeout(0);
   session.removeAllListeners('timeout');
 
-  // Destroy any pending and open streams
-  if (state.pendingStreams.size > 0 || state.streams.size > 0) {
-    const cancel = new ERR_HTTP2_STREAM_CANCEL(error);
-    state.pendingStreams.forEach((stream) => stream.destroy(cancel));
-    state.streams.forEach((stream) => stream.destroy(error));
-  }
-
   // Disassociate from the socket and server.
   const socket = session[kSocket];
   const handle = session[kHandle];
 
-  // Destroy the handle if it exists at this point.
+  // Destroy the handle *before* destroying streams. When the socket is already
+  // closed the handle's ondone callback (finishSessionClose) is invoked
+  // synchronously from handle.destroy(), which schedules the session 'close'
+  // event via process.nextTick. Destroying the streams afterwards ensures their
+  // 'close' events are queued on the nextTick queue *after* the session 'close'
+  // event, so user code receives the session-level signal before any stream
+  // callbacks observe session.closed/session.destroyed as true.
   if (handle !== undefined) {
     handle.ondone = finishSessionClose.bind(null, session, error);
     handle.destroy(code, socket.destroyed);
   } else {
     finishSessionClose(session, error);
   }
+
+  // Destroy any pending and open streams
+  if (state.pendingStreams.size > 0 || state.streams.size > 0) {
+    const cancel = new ERR_HTTP2_STREAM_CANCEL(error);
+    state.pendingStreams.forEach((stream) => stream.destroy(cancel));
+    state.streams.forEach((stream) => stream.destroy(error));
+  }
 }
 
 // Upon creation, the Http2Session takes ownership of the socket. The session

test/parallel/test-http2-session-close-before-stream-close.js

@@ -0,0 +1,68 @@
+'use strict';
+
+// Regression test: when the server abruptly destroys the underlying socket,
+// the client session 'close' event must fire before any stream 'close'
+// callback can observe session.closed/session.destroyed as true.
+// See https://github.com/nodejs/node/issues/<issue>
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const http2 = require('node:http2');
+
+const server = http2.createServer();
+let serverSocket;
+
+server.on('connection', (socket) => {
+  serverSocket = socket;
+  socket.on('error', () => {});
+});
+
+server.on('sessionError', () => {});
+server.on('stream', (stream, headers) => {
+  if (headers[':path'] === '/close') {
+    stream.respond({ ':status': 200 });
+    stream.write('partial', () => {
+      setImmediate(() => serverSocket.destroy());
+    });
+    return;
+  }
+  stream.respond({ ':status': 200 });
+  stream.end('ok');
+});
+
+server.listen(0, common.mustCall(() => {
+  const session = http2.connect(`http://localhost:${server.address().port}`);
+
+  let sessionCloseFired = false;
+
+  session.on('close', common.mustCall(() => {
+    sessionCloseFired = true;
+    server.close();
+  }));
+
+  // We accept that an error may or may not fire, but it must fire before
+  // any stream close callback sees session.destroyed.
+  session.on('error', () => {});
+
+  const req = session.request({ ':path': '/close' });
+  req.resume();
+  req.on('response', () => {});
+
+  // The stream 'close' event must not observe the session as destroyed
+  // before the session 'close' event has fired.
+  req.on('close', common.mustCall(() => {
+    // If the session is already destroyed, the session 'close' event must
+    // have already been emitted (sessionCloseFired === true).
+    if (session.destroyed) {
+      assert.strictEqual(
+        sessionCloseFired,
+        true,
+        'session "close" event must fire before stream "close" callback ' +
+        'observes session.destroyed === true',
+      );
+    }
+  }));
+}));

test/parallel/test-http2-session-close-order-simulation.js

@@ -0,0 +1,61 @@
+'use strict';
+// Simulates the nextTick ordering in closeSession to validate the fix logic
+// without requiring a compiled binary.
+
+const assert = require('assert');
+
+function simulate(label, handleFirst) {
+  const events = [];
+  const ticks = [];
+
+  function nextTick(fn) { ticks.push(fn); }
+  function drainTicks() { while (ticks.length) ticks.shift()(); }
+
+  // stream.destroy() schedules stream 'close' on nextTick
+  function destroyStream() {
+    nextTick(() => events.push('stream close'));
+  }
+
+  // finishSessionClose (socket already destroyed) schedules session 'close' on nextTick
+  function finishSessionClose() {
+    nextTick(() => events.push('session close'));
+  }
+
+  // handle.destroy() calls ondone synchronously when socket is already destroyed
+  // and there are no writes in progress (the scenario from the bug report)
+  function destroyHandle(ondone) {
+    ondone(); // synchronous, as the C++ Http2Session::Close does
+  }
+
+  function closeSession() {
+    if (handleFirst) {
+      // Our fix: destroy handle first → session 'close' queued first
+      destroyHandle(finishSessionClose);
+      destroyStream();
+    } else {
+      // Old code: destroy streams first → stream 'close' queued first
+      destroyStream();
+      destroyHandle(finishSessionClose);
+    }
+  }
+
+  closeSession();
+  drainTicks();
+  return events;
+}
+
+const before = simulate('BEFORE fix', false);
+const after  = simulate('AFTER fix',  true);
+
+console.log('BEFORE fix ordering:', before.join(' -> '));
+console.log('AFTER fix  ordering:', after.join(' -> '));
+
+// Old ordering: stream close fires before session close (the bug)
+assert.deepStrictEqual(before, ['stream close', 'session close'],
+  'Expected old code to have wrong order (stream before session)');
+
+// New ordering: session close fires first (the fix)
+assert.deepStrictEqual(after, ['session close', 'stream close'],
+  'session "close" must fire before stream "close"');
+
+console.log('OK');