fix: reap idle HTTP/2 sessions

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)
 

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)
+})