diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index e980c12d948..ad81acc9d14 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -35,6 +35,7 @@ const { kSize, kHTTPContext, kClosed, + kKeepAliveDefaultTimeout, kHeadersTimeout, kBodyTimeout, kEnableConnectProtocol, @@ -191,6 +192,7 @@ function connectH2 (client, socket) { session[kClient] = client session[kSocket] = socket session[kHTTP2SessionState] = { + idleTimeout: null, ping: { interval: client[kPingInterval] === 0 ? null : setInterval(onHttp2SendPing, client[kPingInterval], session).unref() } @@ -308,18 +310,68 @@ function connectH2 (client, socket) { function resumeH2 (client) { const socket = client[kSocket] + const session = client[kHTTP2Session] if (socket?.destroyed === false) { if (client[kSize] === 0 || client[kMaxConcurrentStreams] === 0) { socket.unref() - client[kHTTP2Session].unref() + session.unref() } else { socket.ref() - client[kHTTP2Session].ref() + session.ref() + } + + if (client[kSize] === 0 && session[kOpenStreams] === 0) { + setHttp2IdleTimeout(session) + } else { + clearHttp2IdleTimeout(session) } } } +function clearHttp2IdleTimeout (session) { + const state = session[kHTTP2SessionState] + + if (state?.idleTimeout != null) { + clearTimeout(state.idleTimeout) + state.idleTimeout = null + } +} + +function setHttp2IdleTimeout (session) { + const client = session[kClient] + + if (client[kHTTP2Session] !== session || session.closed || session.destroyed) { + return + } + + if (session[kOpenStreams] !== 0 || client[kSize] !== 0) { + clearHttp2IdleTimeout(session) + return + } + + const state = session[kHTTP2SessionState] + if (state.idleTimeout == null) { + state.idleTimeout = setTimeout(onHttp2SessionIdleTimeout, client[kKeepAliveDefaultTimeout], session).unref() + } +} + +function onHttp2SessionIdleTimeout (session) { + const client = session[kClient] + const socket = session[kSocket] + const state = session[kHTTP2SessionState] + + state.idleTimeout = null + + if (client[kHTTP2Session] !== session || session[kOpenStreams] !== 0 || client[kSize] !== 0 || session.closed || session.destroyed) { + return + } + + const err = new InformationalError('socket idle timeout') + socket[kError] = err + util.destroy(socket, err) +} + function applyConnectionWindowSize (connectionWindowSize) { try { if (typeof this.setLocalWindowSize === 'function') { @@ -445,6 +497,8 @@ function onHttp2SessionGoAway (errorCode, lastStreamID) { client[kHTTP2Session] = null } + clearHttp2IdleTimeout(this) + if (!this.closed && !this.destroyed) { this.close() } @@ -467,6 +521,8 @@ function onHttp2SessionClose () { client[kHTTP2Session] = null } + clearHttp2IdleTimeout(this) + if (state.ping.interval != null) { clearInterval(state.ping.interval) state.ping.interval = null @@ -542,6 +598,7 @@ function closeStreamSession (stream) { session[kOpenStreams] -= 1 if (session[kOpenStreams] === 0) { session.unref() + setHttp2IdleTimeout(session) } } @@ -711,6 +768,7 @@ function setupUpgradeStream (stream, state) { stream.on('timeout', onUpgradeStreamTimeout) stream.once('close', onUpgradeStreamClose) + clearHttp2IdleTimeout(session) ++session[kOpenStreams] stream.setTimeout(headersTimeout) } @@ -983,6 +1041,7 @@ function writeH2 (client, request) { state.stream = stream // Increment counter as we have new streams open + clearHttp2IdleTimeout(session) ++session[kOpenStreams] stream.setTimeout(headersTimeout) diff --git a/test/issue-5381.js b/test/issue-5381.js new file mode 100644 index 00000000000..2d490194909 --- /dev/null +++ b/test/issue-5381.js @@ -0,0 +1,68 @@ +'use strict' + +const assert = require('node:assert') +const { once } = require('node:events') +const { createSecureServer } = require('node:http2') +const { setTimeout: sleep } = require('node:timers/promises') +const { test } = require('node:test') +const pem = require('@metcoder95/https-pem') + +const { Client } = require('..') + +async function waitFor (predicate, timeout = 1000) { + const deadline = Date.now() + timeout + + while (Date.now() < deadline) { + if (predicate()) { + return true + } + + await sleep(10) + } + + return predicate() +} + +test('HTTP/2 idle sessions honor keepAliveTimeout', async (t) => { + const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } })) + let open = 0 + + server.on('secureConnection', (socket) => { + open++ + socket.once('close', () => { + open-- + }) + }) + server.on('stream', (stream) => { + stream.respond({ ':status': 200 }) + stream.end('hello') + }) + + t.after(() => server.close()) + await once(server.listen(0), 'listening') + + const client = new Client(`https://localhost:${server.address().port}`, { + allowH2: true, + keepAliveTimeout: 100, + connect: { + rejectUnauthorized: false + } + }) + t.after(() => client.close()) + + const disconnected = once(client, 'disconnect') + const response = await client.request({ path: '/', method: 'GET' }) + + response.body.resume() + await once(response.body, 'end') + + assert.strictEqual(open, 1) + + const idleReaped = await Promise.race([ + disconnected.then(() => true), + sleep(1000).then(() => false) + ]) + + assert.strictEqual(idleReaped, true) + assert.strictEqual(await waitFor(() => open === 0), true) +})