Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: add session tracking and graceful server close #57668

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand All @@ -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')
Expand All @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-capture-rejection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -127,6 +125,8 @@ events.captureRejections = true;
stream.respond({
':status': 200
});

server.close();
}));

server.listen(0, common.mustCall(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));
Expand All @@ -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();
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));
Expand All @@ -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();
}));
Expand Down
103 changes: 103 additions & 0 deletions test/parallel/test-http2-graceful-close.js
Original file line number Diff line number Diff line change
@@ -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();
}));
100 changes: 100 additions & 0 deletions test/parallel/test-http2-server-close-idle-connection.js
Original file line number Diff line number Diff line change
@@ -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);
});
Loading