http2: emit ERR_HTTP2_INVALID_SESSION async instead of throwing synchronously

lib/internal/http2/core.js

@@ -1809,11 +1809,24 @@
   request(headersParam, options) {
     debugSessionObj(this, 'initiating request');
 
-    if (this.destroyed)
-      throw new ERR_HTTP2_INVALID_SESSION();
-
-    if (this.closed)
-      throw new ERR_HTTP2_GOAWAY_SESSION();
+    // Session-state errors are deferred asynchronously rather than thrown
+    // synchronously. This prevents a race where stream 'close' callbacks
+    // observe session.destroyed/closed before the session 'close' event has
+    // fired: with a sync throw, code that retries inside a stream callback
+    // would crash before its session lifecycle handlers could clear the cached
+    // session. By returning a stream that emits 'error' on the next tick we
+    // match the behaviour of session.request() while connecting, where errors
+    // are also deferred, and give event-driven code a chance to handle both
+    // session and stream cleanup in any order.
+    if (this.destroyed || this.closed) {
+      // eslint-disable-next-line no-use-before-define
+      const stream = new ClientHttp2Stream(this, undefined, undefined, {});
+      const err = this.destroyed
+        ? new ERR_HTTP2_INVALID_SESSION()
+        : new ERR_HTTP2_GOAWAY_SESSION();
+      process.nextTick(() => stream.destroy(err));
+      return stream;
+    }
 
     this[kUpdateTimer]();
 

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

@@ -0,0 +1,70 @@
+'use strict';
+
+// Regression test: when the server abruptly destroys the underlying socket,
+// session.request() must not throw synchronously inside a stream 'close'
+// callback. Instead it should return a stream that emits 'error' async with
+// ERR_HTTP2_INVALID_SESSION, giving session lifecycle handlers a chance to
+// clear their cached session reference before any retry occurs.
+
+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}`);
+
+  session.on('close', common.mustCall(() => server.close()));
+  session.on('error', () => {});
+
+  const req = session.request({ ':path': '/close' });
+  req.resume();
+  req.on('response', () => {});
+  req.on('error', () => {}); // socket may emit ECONNRESET on abrupt close
+
+  req.on('close', common.mustCall(() => {
+    if (!session.destroyed) return;
+
+    // session.request() must NOT throw synchronously even though the session
+    // is already destroyed. It should return a stream that errors async.
+    let threw = false;
+    let req2;
+    try {
+      req2 = session.request({ ':path': '/again' });
+    } catch {
+      threw = true;
+    }
+
+    assert.strictEqual(threw, false,
+      'session.request() must not throw synchronously inside a stream close callback');
+
+    // The returned stream should asynchronously emit ERR_HTTP2_INVALID_SESSION
+    req2.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ERR_HTTP2_INVALID_SESSION');
+    }));
+    req2.resume();
+  }));
+}));

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');