From 020578f17eae6b9d5e9a2406520b4f540ebc19ea Mon Sep 17 00:00:00 2001 From: suuuuuuminnnnnn Date: Fri, 6 Feb 2026 13:59:20 +0900 Subject: [PATCH 1/5] http2: fix zombie session crash on socket close --- src/node_http2.cc | 5 + .../test-http2-socket-close-zombie-session.js | 97 +++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 test/parallel/test-http2-socket-close-zombie-session.js diff --git a/src/node_http2.cc b/src/node_http2.cc index 72b52ef575c50c..72f60da844c5cf 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -2095,6 +2095,11 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { if (nread <= 0) { if (nread < 0) { PassReadErrorToPreviousListener(nread); + // Socket has encountered an error or EOF. Close the session to prevent + // zombie state where the session believes the connection is alive but + // the underlying socket is dead. This prevents assertion failures in + // subsequent write attempts. (Ref: https://github.com/nodejs/node/issues/61304) + Close(NGHTTP2_NO_ERROR, true); } return; } diff --git a/test/parallel/test-http2-socket-close-zombie-session.js b/test/parallel/test-http2-socket-close-zombie-session.js new file mode 100644 index 00000000000000..1873494b9dea76 --- /dev/null +++ b/test/parallel/test-http2-socket-close-zombie-session.js @@ -0,0 +1,97 @@ +// Flags: --expose-internals +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/61304 +// When the underlying socket is closed at the OS level without +// sending RST/FIN (e.g., network black hole), the HTTP/2 session +// enters a zombie state where it believes the connection is alive +// but the socket is actually dead. Subsequent write attempts should +// fail gracefully rather than crash with assertion failures. + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const h2 = require('http2'); +const { kSocket } = require('internal/http2/util'); +const fixtures = require('../common/fixtures'); + +const server = h2.createSecureServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}); + +server.on('stream', common.mustCall((stream) => { + stream.respond({ ':status': 200 }); + stream.end('hello'); +})); + +server.listen(0, common.mustCall(() => { + const client = h2.connect(`https://localhost:${server.address().port}`, { + rejectUnauthorized: false + }); + + let firstRequestDone = false; + + // First request to establish connection + const req1 = client.request({ ':path': '/' }); + req1.on('response', common.mustCall(() => { + firstRequestDone = true; + })); + req1.on('data', () => {}); + req1.on('end', common.mustCall(() => { + // Connection is established, now simulate network black hole + // by destroying the underlying socket without proper close + const socket = client[kSocket]; + + // Verify session state before socket destruction + assert.strictEqual(client.closed, false); + assert.strictEqual(client.destroyed, false); + + // Destroy the socket to simulate OS-level connection loss + // This mimics what happens when network drops packets without RST/FIN + socket.destroy(); + + // The session should handle this gracefully + // Prior to fix: this would cause assertion failures in subsequent writes + // After fix: session should close properly + + setImmediate(() => { + // Try to send another request into the zombie session + // With the fix, the session should close gracefully without crashing + try { + const req2 = client.request({ ':path': '/test' }); + // The request may or may not emit an error event, + // but it should not receive a response + req2.on('error', () => { + // Acceptable: error event fires + }); + req2.on('response', common.mustNotCall( + 'Should not receive response from zombie session' + )); + req2.end(); + } catch (err) { + // Also acceptable: synchronous error on request creation + assert(err); + } + + // Verify session eventually closes + client.on('close', common.mustCall(() => { + server.close(); + })); + + // Force cleanup if session doesn't close naturally + setTimeout(() => { + if (!client.destroyed) { + client.destroy(); + } + if (server.listening) { + server.close(); + } + }, 1000); + }); + })); + + req1.end(); +})); From 6da4fe92103b7ca7a0db3825ad93da6efd20f5b8 Mon Sep 17 00:00:00 2001 From: suuuuuuminnnnnn Date: Fri, 6 Feb 2026 22:04:36 +0900 Subject: [PATCH 2/5] http2: prevent assertion failure in OnStreamAfterWrite --- src/node_http2.cc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 72f60da844c5cf..f719617c04ad09 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1766,7 +1766,14 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) { Debug(this, "write finished with status %d", status); MaybeNotifyGracefulCloseComplete(); - CHECK(is_write_in_progress()); + // Guard against write callback being invoked when write is not in progress. + // This can happen with zombie sessions where the underlying socket is closed + // but the session hasn't been properly notified. Instead of crashing, we + // silently handle the inconsistent state. (Ref: https://github.com/nodejs/node/issues/61304) + if (!is_write_in_progress()) { + Debug(this, "write callback invoked but write not in progress, possible zombie session"); + return; + } set_write_in_progress(false); // Inform all pending writes about their completion. @@ -2095,11 +2102,6 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { if (nread <= 0) { if (nread < 0) { PassReadErrorToPreviousListener(nread); - // Socket has encountered an error or EOF. Close the session to prevent - // zombie state where the session believes the connection is alive but - // the underlying socket is dead. This prevents assertion failures in - // subsequent write attempts. (Ref: https://github.com/nodejs/node/issues/61304) - Close(NGHTTP2_NO_ERROR, true); } return; } From 780ec8ab1482aca9ec5f088d28475e92cb0e197e Mon Sep 17 00:00:00 2001 From: suuuuuuminnnnnn Date: Fri, 6 Feb 2026 22:19:01 +0900 Subject: [PATCH 3/5] http2: handle write callback gracefully in zombie sessions --- src/node_http2.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index f719617c04ad09..d3a0b46399ee83 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1771,7 +1771,8 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) { // but the session hasn't been properly notified. Instead of crashing, we // silently handle the inconsistent state. (Ref: https://github.com/nodejs/node/issues/61304) if (!is_write_in_progress()) { - Debug(this, "write callback invoked but write not in progress, possible zombie session"); + Debug(this, "write callback invoked but write not in progress, " + "possible zombie session"); return; } set_write_in_progress(false); From 695b5feab951d369a28daa34ac8e5e003950a5ef Mon Sep 17 00:00:00 2001 From: suuuuuuminnnnnn Date: Fri, 6 Feb 2026 22:51:25 +0900 Subject: [PATCH 4/5] http2: handle write callback gracefully in zombie sessions --- .../test-http2-socket-close-zombie-session.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/test/parallel/test-http2-socket-close-zombie-session.js b/test/parallel/test-http2-socket-close-zombie-session.js index 1873494b9dea76..2c00d9cd60188b 100644 --- a/test/parallel/test-http2-socket-close-zombie-session.js +++ b/test/parallel/test-http2-socket-close-zombie-session.js @@ -32,13 +32,14 @@ server.listen(0, common.mustCall(() => { rejectUnauthorized: false }); - let firstRequestDone = false; + // Verify session eventually closes + client.on('close', common.mustCall(() => { + server.close(); + })); // First request to establish connection const req1 = client.request({ ':path': '/' }); - req1.on('response', common.mustCall(() => { - firstRequestDone = true; - })); + req1.on('response', common.mustCall()); req1.on('data', () => {}); req1.on('end', common.mustCall(() => { // Connection is established, now simulate network black hole @@ -71,16 +72,10 @@ server.listen(0, common.mustCall(() => { 'Should not receive response from zombie session' )); req2.end(); - } catch (err) { + } catch { // Also acceptable: synchronous error on request creation - assert(err); } - // Verify session eventually closes - client.on('close', common.mustCall(() => { - server.close(); - })); - // Force cleanup if session doesn't close naturally setTimeout(() => { if (!client.destroyed) { From 9706c30d365c1eb7fe84ae4935abcb61a6543ee5 Mon Sep 17 00:00:00 2001 From: suuuuuuminnnnnn Date: Sat, 7 Feb 2026 14:35:29 +0900 Subject: [PATCH 5/5] http2: fix zombie session crash on socket close --- src/node_http2.cc | 8 ++++- .../test-http2-socket-close-zombie-session.js | 29 ++++++++++++------- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index d3a0b46399ee83..a268ea32f3e252 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1769,10 +1769,16 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) { // Guard against write callback being invoked when write is not in progress. // This can happen with zombie sessions where the underlying socket is closed // but the session hasn't been properly notified. Instead of crashing, we - // silently handle the inconsistent state. (Ref: https://github.com/nodejs/node/issues/61304) + // terminate the session to prevent resource leaks and further inconsistent + // callbacks. (Ref: https://github.com/nodejs/node/issues/61304) if (!is_write_in_progress()) { Debug(this, "write callback invoked but write not in progress, " "possible zombie session"); + // Force session termination to clean up resources + // Don't attempt to send GOAWAY (socket likely already closed) + if (!is_destroyed()) { + Close(NGHTTP2_INTERNAL_ERROR, true); + } return; } set_write_in_progress(false); diff --git a/test/parallel/test-http2-socket-close-zombie-session.js b/test/parallel/test-http2-socket-close-zombie-session.js index 2c00d9cd60188b..6f8b73ce3d4c56 100644 --- a/test/parallel/test-http2-socket-close-zombie-session.js +++ b/test/parallel/test-http2-socket-close-zombie-session.js @@ -32,11 +32,27 @@ server.listen(0, common.mustCall(() => { rejectUnauthorized: false }); + let cleanupTimer; + const cleanup = () => { + clearTimeout(cleanupTimer); + if (!client.destroyed) { + client.destroy(); + } + if (server.listening) { + server.close(); + } + }; + // Verify session eventually closes client.on('close', common.mustCall(() => { - server.close(); + cleanup(); })); + // Handle errors without failing test + client.on('error', () => { + // Expected - connection closed + }); + // First request to establish connection const req1 = client.request({ ':path': '/' }); req1.on('response', common.mustCall()); @@ -76,15 +92,8 @@ server.listen(0, common.mustCall(() => { // Also acceptable: synchronous error on request creation } - // Force cleanup if session doesn't close naturally - setTimeout(() => { - if (!client.destroyed) { - client.destroy(); - } - if (server.listening) { - server.close(); - } - }, 1000); + // Fallback cleanup if close event doesn't fire within 100ms + cleanupTimer = setTimeout(cleanup, 100); }); }));