http2: session tracking and graceful server close

This change adds proper tracking of HTTP / 2 server sessions
to ensure they are gracefully closed when the server is
shut down.It implements:

- A new kSessions symbol for tracking active sessions
- Adding/removing sessions from a SafeSet in the server
- A closeAllSessions helper function to close active sessions
- Updates to Http2Server and Http2SecureServer close methods

Breaking Change: any client trying to create new requests
on existing connections will not be able to do so once
server close is initiated

Refs: https://datatracker.ietf.org/doc/html/rfc7540\#section-9.1
Refs: https://nodejs.org/api/http.html\#serverclosecallback

- improve HTTP/2 server shutdown to prevent race conditions

1. Fix server shutdown race condition
   - Stop listening for new connections before closing existing ones
   - Ensure server.close() properly completes in all scenarios

2. Improve HTTP/2 tests
   - Replace setTimeout with event-based flow control
   - Simplify test logic for better readability
   - Add clear state tracking for event ordering
   - Improve assertions to verify correct shutdown sequence

This eliminates a race condition where new sessions could connect
between the time existing sessions are closed and the server stops
listening, potentially preventing the server from fully shutting down.

- fix cross-platform test timing issues

Fix test-http2-server-http1-client.js failure on Ubuntu
by deferring server.close() to next event loop cycle.

The issue only affected Ubuntu where session close occurs
before error emission, causing the test to miss errors
when HTTP/1 clients connect to HTTP/2 servers.

Using setImmediate() ensures error events fire before
server close across all platforms while maintaining
recent session handling improvements.

PR-URL: #57586
Fixes: #57611
Refs: https://datatracker.ietf.org/doc/html/rfc7540#section-9.1
Refs: https://nodejs.org/api/http.html#serverclosecallback
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Tim Perry <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: pandeykushagra51

lib/internal/http2/core.js

@@ -251,6 +251,7 @@ const kServer = Symbol('server');
 const kState = Symbol('state');
 const kType = Symbol('type');
 const kWriteGeneric = Symbol('write-generic');
+const kSessions = Symbol('sessions');
 
 const {
   kBitfield,
@@ -1126,9 +1127,13 @@ function emitClose(self, error) {
 function cleanupSession(session) {
   const socket = session[kSocket];
   const handle = session[kHandle];
+  const server = session[kServer];
   session[kProxySocket] = undefined;
   session[kSocket] = undefined;
   session[kHandle] = undefined;
+  if (server) {
+    server[kSessions].delete(session);
+  }
   session[kNativeFields] = trackAssignmentsTypedArray(
     new Uint8Array(kSessionUint8FieldCount));
   if (handle)
@@ -1651,6 +1656,9 @@ class ServerHttp2Session extends Http2Session {
   constructor(options, socket, server) {
     super(NGHTTP2_SESSION_SERVER, options, socket);
     this[kServer] = server;
+    if (server) {
+      server[kSessions].add(this);
+    }
     // This is a bit inaccurate because it does not reflect changes to
     // number of listeners made after the session was created. This should
     // not be an issue in practice. Additionally, the 'priority' event on
@@ -3175,11 +3183,25 @@ function onErrorSecureServerSession(err, socket) {
     socket.destroy(err);
 }
 
+/**
+ * This function closes all active sessions gracefully.
+ * @param {*} server the underlying server whose sessions to be closed
+ */
+function closeAllSessions(server) {
+  const sessions = server[kSessions];
+  if (sessions.size > 0) {
+    for (const session of sessions) {
+      session.close();
+    }
+  }
+}
+
 class Http2SecureServer extends TLSServer {
   constructor(options, requestListener) {
     options = initializeTLSOptions(options);
     super(options, connectionListener);
     this[kOptions] = options;
+    this[kSessions] = new SafeSet();
     this.timeout = 0;
     this.on('newListener', setupCompat);
     if (options.allowHTTP1 === true) {
@@ -3209,10 +3231,11 @@ class Http2SecureServer extends TLSServer {
   }
 
   close() {
+    ReflectApply(TLSServer.prototype.close, this, arguments);
     if (this[kOptions].allowHTTP1 === true) {
       httpServerPreClose(this);
     }
-    ReflectApply(TLSServer.prototype.close, this, arguments);
+    closeAllSessions(this);
   }
 
   closeIdleConnections() {
@@ -3227,6 +3250,7 @@ class Http2Server extends NETServer {
     options = initializeOptions(options);
     super(options, connectionListener);
     this[kOptions] = options;
+    this[kSessions] = new SafeSet();
     this.timeout = 0;
     this.on('newListener', setupCompat);
     if (typeof requestListener === 'function')
@@ -3248,6 +3272,11 @@ class Http2Server extends NETServer {
     this[kOptions].settings = { ...this[kOptions].settings, ...settings };
   }
 
+  close() {
+    ReflectApply(NETServer.prototype.close, this, arguments);
+    closeAllSessions(this);
+  }
+
   async [SymbolAsyncDispose]() {
     return promisify(super.close).call(this);
   }

test/parallel/test-http2-capture-rejection.js

@@ -108,8 +108,6 @@ events.captureRejections = true;
   server.on('stream', common.mustCall(async (stream) => {
     const { port } = server.address();
 
-    server.close();
-
     stream.pushStream({
       ':scheme': 'http',
       ':path': '/foobar',
@@ -127,6 +125,8 @@ events.captureRejections = true;
     stream.respond({
       ':status': 200
     });
+
+    server.close();
   }));
 
   server.listen(0, common.mustCall(() => {

test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js

@@ -24,7 +24,6 @@ server.listen(0, common.mustCall(function() {
       response.statusMessage = 'test';
       response.statusMessage = 'test'; // only warn once
       assert.strictEqual(response.statusMessage, ''); // no change
-      server.close();
     }));
     response.end();
   }));
@@ -44,6 +43,9 @@ server.listen(0, common.mustCall(function() {
     request.on('end', common.mustCall(function() {
       client.close();
     }));
+    request.on('close', common.mustCall(function() {
+      server.close();
+    }));
     request.end();
     request.resume();
   }));

test/parallel/test-http2-compat-serverresponse-statusmessage-property.js

@@ -23,7 +23,6 @@ server.listen(0, common.mustCall(function() {
     response.on('finish', common.mustCall(function() {
       assert.strictEqual(response.statusMessage, '');
       assert.strictEqual(response.statusMessage, ''); // only warn once
-      server.close();
     }));
     response.end();
   }));
@@ -43,6 +42,9 @@ server.listen(0, common.mustCall(function() {
     request.on('end', common.mustCall(function() {
       client.close();
     }));
+    request.on('close', common.mustCall(function() {
+      server.close();
+    }));
     request.end();
     request.resume();
   }));

test/parallel/test-http2-graceful-close.js

@@ -0,0 +1,67 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+// This test verifies that the server waits for active connections/sessions
+// to complete and all data to be sent before fully closing
+
+// Create a server that will send a large response in chunks
+const server = http2.createServer();
+
+// Track server events
+server.on('stream', common.mustCall((stream, headers) => {
+
+  // Initiate the server close before client data is sent, this will
+  // test if the server properly waits for the stream to finish
+  server.close();
+  setImmediate(() => {
+    stream.respond({
+      'content-type': 'text/plain',
+      ':status': 200
+    });
+
+    // Create a large response (1MB) to ensure it takes some time to send
+    const chunk = Buffer.alloc(64 * 1024, 'x');
+
+    // Send 16 chunks (1MB total) to simulate a large response
+    for (let i = 0; i < 16; i++) {
+      stream.write(chunk);
+    }
+
+    // Stream end should happen after data is written
+    stream.end();
+  });
+
+  stream.on('close', common.mustCall(() => {
+    assert.strictEqual(stream.readableEnded, true);
+    assert.strictEqual(stream.writableFinished, true);
+  }));
+}));
+
+// Start the server
+server.listen(0, common.mustCall(() => {
+  // Create client and request
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+  const req = client.request({ ':path': '/' });
+
+  // Track received data
+  let receivedData = 0;
+  req.on('data', (chunk) => {
+    receivedData += chunk.length;
+  });
+
+  // When request closes
+  req.on('close', common.mustCall(() => {
+    // Should receive all data
+    assert.strictEqual(req.readableEnded, true);
+    assert.strictEqual(receivedData, 64 * 1024 * 16);
+    assert.strictEqual(req.writableFinished, true);
+  }));
+
+  // Start the request
+  req.end();
+}));

test/parallel/test-http2-server-close-idle-connection.js

@@ -0,0 +1,58 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+// This test verifies that the server closes idle connections
+
+const server = http2.createServer();
+
+server.on('session', common.mustCall((session) => {
+  session.on('stream', common.mustCall((stream) => {
+    // Respond to the request with a small payload
+    stream.respond({
+      'content-type': 'text/plain',
+      ':status': 200
+    });
+    stream.end('hello');
+    stream.on('close', common.mustCall(() => {
+      assert.strictEqual(stream.writableFinished, true);
+    }));
+  }));
+  session.on('close', common.mustCall());
+}));
+
+// Start the server
+server.listen(0, common.mustCall(() => {
+  // Create client and initial request
+  const client = http2.connect(`http://localhost:${server.address().port}`);
+
+  // This will ensure that server closed the idle connection
+  client.on('close', common.mustCall());
+
+  // Make an initial request
+  const req = client.request({ ':path': '/' });
+
+  // Process the response data
+  req.setEncoding('utf8');
+  let data = '';
+  req.on('data', (chunk) => {
+    data += chunk;
+  });
+
+  // When initial request ends
+  req.on('end', common.mustCall(() => {
+    assert.strictEqual(data, 'hello');
+    // Close the server as client is idle now
+    setImmediate(() => {
+      server.close();
+    });
+  }));
+
+  // Don't explicitly close the client, we want to keep it
+  // idle and check if the server closes the idle connection.
+  // As this is what we want to test here.
+}));

test/parallel/test-http2-server-http1-client.js

@@ -23,5 +23,5 @@ server.on('session', common.mustCall((session) => {
 
 server.listen(0, common.mustCall(() => {
   const req = http.get(`http://localhost:${server.address().port}`);
-  req.on('error', (error) => server.close());
+  req.on('error', (error) => setImmediate(() => server.close()));
 }));