diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 554221ac614636..0cc4ead811716a 100644 --- a/lib/internal/http2/core.js +++ b/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, @@ -1125,9 +1126,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) @@ -1644,6 +1649,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 @@ -3168,11 +3176,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) { + sessions.forEach((session) => { + 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) { @@ -3202,10 +3224,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() { @@ -3220,6 +3243,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') @@ -3241,6 +3265,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); } diff --git a/test/parallel/test-http2-capture-rejection.js b/test/parallel/test-http2-capture-rejection.js index 6d8cb224ce7919..39feec784f3d4a 100644 --- a/test/parallel/test-http2-capture-rejection.js +++ b/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(() => { diff --git a/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js b/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js index 87e172402899f2..778600775eb45f 100644 --- a/test/parallel/test-http2-compat-serverresponse-statusmessage-property-set.js +++ b/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(); })); diff --git a/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js b/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js index 8a083cf3ba1638..eaffcc11cd3175 100644 --- a/test/parallel/test-http2-compat-serverresponse-statusmessage-property.js +++ b/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(); })); diff --git a/test/parallel/test-http2-graceful-close.js b/test/parallel/test-http2-graceful-close.js new file mode 100644 index 00000000000000..753f65d5fd7ea4 --- /dev/null +++ b/test/parallel/test-http2-graceful-close.js @@ -0,0 +1,103 @@ +'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 + +// Keep track of the test flow with these flags +const states = { + serverListening: false, + responseStarted: false, + dataFullySent: false, + streamClosed: false, + serverClosed: false +}; + +// 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) => { + assert.strictEqual(states.serverListening, true); + + // Setup the response + stream.respond({ + 'content-type': 'text/plain', + ':status': 200 + }); + + // Initiate the server close before client data is sent, this will + // test if the server properly waits for the stream to finish + server.close(common.mustCall(() => { + // Stream should be closed before server close callback + // Should be called only after the stream has closed + assert.strictEqual(states.streamClosed, true); + states.serverClosed = true; + })); + + // Mark response as started + states.responseStarted = true; + + // 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(); + states.dataFullySent = true; + + // When stream closes, we can verify order of events + stream.on('close', common.mustCall(() => { + // Data should be fully sent before stream closes + assert.strictEqual(states.dataFullySent, true); + states.streamClosed = true; + })); +})); + +// Start the server +server.listen(0, common.mustCall(() => { + states.serverListening = true; + + // 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 the request completes + req.on('end', common.mustCall(() => { + // All data should be sent before request ends + assert.strictEqual(states.dataFullySent, true); + })); + + // When request closes + req.on('close', common.mustCall(() => { + // Check final state + assert.strictEqual(states.streamClosed, true); + // Should receive all data + assert.strictEqual(receivedData, 64 * 1024 * 16); + + // Wait for the server close confirmation + process.on('exit', () => { + // Server should be fully closed + assert.strictEqual(states.serverClosed, true); + }); + })); + + // Start the request + req.end(); +})); diff --git a/test/parallel/test-http2-server-close-idle-connection.js b/test/parallel/test-http2-server-close-idle-connection.js new file mode 100644 index 00000000000000..7bf30cb049ce76 --- /dev/null +++ b/test/parallel/test-http2-server-close-idle-connection.js @@ -0,0 +1,100 @@ +'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 + +// Track test state with flags +const states = { + serverListening: false, + initialRequestCompleted: false, + connectionBecameIdle: false, + serverClosedIdleConnection: false +}; + +const server = http2.createServer(); + +// Track server events +server.on('stream', common.mustCall((stream, headers) => { + assert.strictEqual(states.serverListening, true); + + // Respond to the request with a small payload + stream.respond({ + 'content-type': 'text/plain', + ':status': 200 + }); + stream.end('hello'); + + // After the request is done, the connection should become idle + stream.on('close', () => { + states.initialRequestCompleted = true; + // Mark connection as idle after the request completes + states.connectionBecameIdle = true; + }); +})); + +// Track session closure events +server.on('session', common.mustCall((session) => { + session.on('stream', common.mustCall((stream) => { + stream.on('close', common.mustCall(() => { + // This should only happen after the connection became idle + assert.strictEqual(states.connectionBecameIdle, true); + states.serverClosedIdleConnection = true; + + // Test is successful, close the server + server.close(common.mustCall(() => { + console.log('server closed'); + })); + })); + })); + session.on('close', common.mustCall(() => { + console.log('session closed'); + })); +})); + +// Start the server +server.listen(0, common.mustCall(() => { + states.serverListening = true; + + // Create client and initial request + const client = http2.connect(`http://localhost:${server.address().port}`); + + // Track client session events + client.on('close', common.mustCall(() => { + // Verify server closed the connection after it became idle + assert.strictEqual(states.serverClosedIdleConnection, true); + })); + + // Make an initial request + const req = client.request({ ':path': '/' }); + + req.on('response', common.mustCall()); + + // 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'); + + // Don't explicitly close the client - keep it idle + // Wait for the server to close the idle connection + })); + + client.on('close', common.mustCall()); + + req.end(); +})); + +// Final verification on exit +process.on('exit', () => { + assert.strictEqual(states.serverClosedIdleConnection, true); +});