[Huang-404-Q]

closeSession() set SESSION_FLAGS_DESTROYED and then called session.setTimeout(0). setStreamTimeout() returns early when the session is destroyed, so the idle timer stayed armed and retained the session until the server timeout fired.

Clear the timeout and remove its listeners before setting the destroyed flag.

Fixes: #66885

Test: test/parallel/test-http2-session-timeout-cleared-on-close.js asserts session.timeout === 0 after close. It fails on current main (actual: 30000, expected: 0) and passes with the reorder.

I verified the behavior against the installed Node v24.14.0 with an emulated patch (the repo's lib/ needs a full build to run in-tree); the test itself was run with that patch.

[nodejs-github-bot]

Review requested:

• @nodejs/http2
• @nodejs/net

[github-actions]

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

• the guide for first-time contributors
• the contribution and automation policies
• the pull request guide
• the AI use policy
• the Code of Conduct

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

Caution

AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context.

[ccamensuli]

Thanks for picking this up so quickly — the reordering is exactly what I had in mind.

One suggestion for the test: it only covers a graceful close (client.close() sends GOAWAY). The case that matters most is a TCP connection dropped without GOAWAY, because the 'goaway' workaround from the issue never runs there. On that path socketOnClose() calls closeSession() directly (not through destroy()), so the fix covers it too, and a second test would lock it in.

Checked on v26.12.1 (unpatched): with the raw socket destroyed, the server session closes with no 'goaway' event, destroyed: true and timeout: 30000. The test below fails there on assert.strictEqual(session.timeout, 0) (actual: 30000).

'use strict';

const common = require('../common');
if (!common.hasCrypto)
  common.skip('missing crypto');
const assert = require('assert');
const http2 = require('http2');
const net = require('net');

// Same as test-http2-session-timeout-cleared-on-close.js, but the client's
// TCP socket is destroyed without sending GOAWAY.
const server = http2.createServer();
server.setTimeout(30_000, common.mustNotCall());

server.on('session', common.mustCall((session) => {
  session.on('goaway', common.mustNotCall());
  session.on('close', common.mustCall(() => {
    assert.strictEqual(session.timeout, 0);
    server.close();
  }));
}));

server.on('stream', common.mustCall((stream) => {
  stream.respond({ ':status': 200 });
  stream.end('ok');
}));

server.listen(0, common.mustCall(() => {
  let socket;
  const client = http2.connect(`http://localhost:${server.address().port}`, {
    createConnection: (url) => (socket = net.connect(url.port, url.hostname)),
  });
  client.on('error', () => {});
  const req = client.request();
  req.on('error', () => {});
  req.resume();
  req.on('end', common.mustCall(() => socket.destroy()));
}));

[Huang-404-Q]

Thanks — added test/parallel/test-http2-session-timeout-cleared-on-close-raw-socket.js using your repro, unchanged. Confirmed locally on an unpatched build (v24.14.0) that it fails on assert.strictEqual(session.timeout, 0) with actual: 30000 and no 'goaway' event, so it locks in the raw-socket path. The fix already reorders setTimeout(0) before the destroyed flag, and CI will run the test against the patched build.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.65%. Comparing base (ef63226) to head (50fff4b).
⚠️ Report is 110 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66886      +/-   ##
==========================================
+ Coverage   90.49%   90.65%   +0.15%     
==========================================
  Files         797      797              
  Lines      278362   278485     +123     
  Branches    53484    53554      +70     
==========================================
+ Hits       251908   252454     +546     
+ Misses      16822    16386     -436     
- Partials     9632     9645      +13     

Files with missing lines	Coverage Δ
lib/internal/http2/core.js	94.92% <100.00%> (-0.03%)	⬇️

... and 89 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Huang-404-Q]

Thanks for taking a look. The first commit's body exceeded 72 columns and was flagged by lint-commit-message; I rewrapped both commit messages to stay within the limit. The check passes on the latest push.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78697/

[nodejs-github-bot]

Landed in 663f666