http2: session tracking and graceful server close · nodejs/node@d16b0ba · GitHub
Skip to content

Commit d16b0ba

Browse files
pandeykushagra51RafaelGSS
pandeykushagra51
authored and
RafaelGSS
committed
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 <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent cdedec7 commit d16b0ba

7 files changed

Lines changed: 164 additions & 6 deletions

‎lib/internal/http2/core.js‎

Lines changed: 30 additions & 1 deletion

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,6 @@ events.captureRejections = true;
108108
server.on('stream', common.mustCall(async (stream) => {
109109
const { port } = server.address();
110110

111-
server.close();
112-
113111
stream.pushStream({
114112
':scheme': 'http',
115113
':path': '/foobar',
@@ -127,6 +125,8 @@ events.captureRejections = true;
127125
stream.respond({
128126
':status': 200
129127
});
128+
129+
server.close();
130130
}));
131131

132132
server.listen(0, common.mustCall(() => {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ server.listen(0, common.mustCall(function() {
2424
response.statusMessage = 'test';
2525
response.statusMessage = 'test'; // only warn once
2626
assert.strictEqual(response.statusMessage, ''); // no change
27-
server.close();
2827
}));
2928
response.end();
3029
}));
@@ -44,6 +43,9 @@ server.listen(0, common.mustCall(function() {
4443
request.on('end', common.mustCall(function() {
4544
client.close();
4645
}));
46+
request.on('close', common.mustCall(function() {
47+
server.close();
48+
}));
4749
request.end();
4850
request.resume();
4951
}));

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ server.listen(0, common.mustCall(function() {
2323
response.on('finish', common.mustCall(function() {
2424
assert.strictEqual(response.statusMessage, '');
2525
assert.strictEqual(response.statusMessage, ''); // only warn once
26-
server.close();
2726
}));
2827
response.end();
2928
}));
@@ -43,6 +42,9 @@ server.listen(0, common.mustCall(function() {
4342
request.on('end', common.mustCall(function() {
4443
client.close();
4544
}));
45+
request.on('close', common.mustCall(function() {
46+
server.close();
47+
}));
4648
request.end();
4749
request.resume();
4850
}));

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
9+
// This test verifies that the server waits for active connections/sessions
10+
// to complete and all data to be sent before fully closing
11+
12+
// Create a server that will send a large response in chunks
13+
const server = http2.createServer();
14+
15+
// Track server events
16+
server.on('stream', common.mustCall((stream, headers) => {
17+
18+
// Initiate the server close before client data is sent, this will
19+
// test if the server properly waits for the stream to finish
20+
server.close();
21+
setImmediate(() => {
22+
stream.respond({
23+
'content-type': 'text/plain',
24+
':status': 200
25+
});
26+
27+
// Create a large response (1MB) to ensure it takes some time to send
28+
const chunk = Buffer.alloc(64 * 1024, 'x');
29+
30+
// Send 16 chunks (1MB total) to simulate a large response
31+
for (let i = 0; i < 16; i++) {
32+
stream.write(chunk);
33+
}
34+
35+
// Stream end should happen after data is written
36+
stream.end();
37+
});
38+
39+
stream.on('close', common.mustCall(() => {
40+
assert.strictEqual(stream.readableEnded, true);
41+
assert.strictEqual(stream.writableFinished, true);
42+
}));
43+
}));
44+
45+
// Start the server
46+
server.listen(0, common.mustCall(() => {
47+
// Create client and request
48+
const client = http2.connect(`http://localhost:${server.address().port}`);
49+
const req = client.request({ ':path': '/' });
50+
51+
// Track received data
52+
let receivedData = 0;
53+
req.on('data', (chunk) => {
54+
receivedData += chunk.length;
55+
});
56+
57+
// When request closes
58+
req.on('close', common.mustCall(() => {
59+
// Should receive all data
60+
assert.strictEqual(req.readableEnded, true);
61+
assert.strictEqual(receivedData, 64 * 1024 * 16);
62+
assert.strictEqual(req.writableFinished, true);
63+
}));
64+
65+
// Start the request
66+
req.end();
67+
}));

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
9+
// This test verifies that the server closes idle connections
10+
11+
const server = http2.createServer();
12+
13+
server.on('session', common.mustCall((session) => {
14+
session.on('stream', common.mustCall((stream) => {
15+
// Respond to the request with a small payload
16+
stream.respond({
17+
'content-type': 'text/plain',
18+
':status': 200
19+
});
20+
stream.end('hello');
21+
stream.on('close', common.mustCall(() => {
22+
assert.strictEqual(stream.writableFinished, true);
23+
}));
24+
}));
25+
session.on('close', common.mustCall());
26+
}));
27+
28+
// Start the server
29+
server.listen(0, common.mustCall(() => {
30+
// Create client and initial request
31+
const client = http2.connect(`http://localhost:${server.address().port}`);
32+
33+
// This will ensure that server closed the idle connection
34+
client.on('close', common.mustCall());
35+
36+
// Make an initial request
37+
const req = client.request({ ':path': '/' });
38+
39+
// Process the response data
40+
req.setEncoding('utf8');
41+
let data = '';
42+
req.on('data', (chunk) => {
43+
data += chunk;
44+
});
45+
46+
// When initial request ends
47+
req.on('end', common.mustCall(() => {
48+
assert.strictEqual(data, 'hello');
49+
// Close the server as client is idle now
50+
setImmediate(() => {
51+
server.close();
52+
});
53+
}));
54+
55+
// Don't explicitly close the client, we want to keep it
56+
// idle and check if the server closes the idle connection.
57+
// As this is what we want to test here.
58+
}));

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

Lines changed: 1 addition & 1 deletion

0 commit comments

Comments
 (0)