Skip to content

Commit 9706c30

Browse files
http2: fix zombie session crash on socket close
1 parent 695b5fe commit 9706c30

File tree

2 files changed

+26
-11
lines changed

2 files changed

+26
-11
lines changed

src/node_http2.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1769,10 +1769,16 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
17691769
// Guard against write callback being invoked when write is not in progress.
17701770
// This can happen with zombie sessions where the underlying socket is closed
17711771
// but the session hasn't been properly notified. Instead of crashing, we
1772-
// silently handle the inconsistent state. (Ref: https://github.com/nodejs/node/issues/61304)
1772+
// terminate the session to prevent resource leaks and further inconsistent
1773+
// callbacks. (Ref: https://github.com/nodejs/node/issues/61304)
17731774
if (!is_write_in_progress()) {
17741775
Debug(this, "write callback invoked but write not in progress, "
17751776
"possible zombie session");
1777+
// Force session termination to clean up resources
1778+
// Don't attempt to send GOAWAY (socket likely already closed)
1779+
if (!is_destroyed()) {
1780+
Close(NGHTTP2_INTERNAL_ERROR, true);
1781+
}
17761782
return;
17771783
}
17781784
set_write_in_progress(false);

test/parallel/test-http2-socket-close-zombie-session.js

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,27 @@ server.listen(0, common.mustCall(() => {
3232
rejectUnauthorized: false
3333
});
3434

35+
let cleanupTimer;
36+
const cleanup = () => {
37+
clearTimeout(cleanupTimer);
38+
if (!client.destroyed) {
39+
client.destroy();
40+
}
41+
if (server.listening) {
42+
server.close();
43+
}
44+
};
45+
3546
// Verify session eventually closes
3647
client.on('close', common.mustCall(() => {
37-
server.close();
48+
cleanup();
3849
}));
3950

51+
// Handle errors without failing test
52+
client.on('error', () => {
53+
// Expected - connection closed
54+
});
55+
4056
// First request to establish connection
4157
const req1 = client.request({ ':path': '/' });
4258
req1.on('response', common.mustCall());
@@ -76,15 +92,8 @@ server.listen(0, common.mustCall(() => {
7692
// Also acceptable: synchronous error on request creation
7793
}
7894

79-
// Force cleanup if session doesn't close naturally
80-
setTimeout(() => {
81-
if (!client.destroyed) {
82-
client.destroy();
83-
}
84-
if (server.listening) {
85-
server.close();
86-
}
87-
}, 1000);
95+
// Fallback cleanup if close event doesn't fire within 100ms
96+
cleanupTimer = setTimeout(cleanup, 100);
8897
});
8998
}));
9099

0 commit comments

Comments
 (0)