Skip to content

http2: fix zombie session crash on socket close#61702

Open
suuuuuuminnnnnn wants to merge 1 commit intonodejs:mainfrom
suuuuuuminnnnnn:fix-http2-zombie-session
Open

http2: fix zombie session crash on socket close#61702
suuuuuuminnnnnn wants to merge 1 commit intonodejs:mainfrom
suuuuuuminnnnnn:fix-http2-zombie-session

Conversation

@suuuuuuminnnnnn
Copy link

Summary

This PR fixes a crash caused by HTTP/2 zombie sessions when the underlying socket is dead but the HTTP/2 session remains “alive” in Node.js.

  • The underlying socket can become closed at the OS level without Node.js receiving a close event (e.g. packet drop “black hole”).
  • Subsequent writes can hit internal invariants and crash the process:
    • CHECK(is_write_in_progress()) in Http2Session::OnStreamAfterWrite
    • CHECK(!current_write_) in TLSWrap::DoWrite
  • Fix: Close the HTTP/2 session when a read error occurs (nread < 0) in Http2Session::OnStreamRead.

Fixes: #61304


What is the current behavior?

When the network enters a “black hole” state (packets dropped without RST/FIN), the OS may consider the TCP socket dead/closed, but Node.js can fail to observe the close.

In this case:

  • The HTTP/2 session stays in a zombie state:
    • session.closed === false
    • session.destroyed === false
    • outbound queue grows (session.state.outboundQueueSize increases continuously)
  • Later write attempts can break invariants and crash the process via assertions.

Relevant code path (before):

// Only pass data on if nread > 0
if (nread <= 0) {
  if (nread < 0) {
    PassReadErrorToPreviousListener(nread);
  }
  return;  // session not closed -> zombie possible
}

What is the new behavior?

On read error (nread < 0), we still notify the previous listener, then close the HTTP/2 session to prevent zombie state and subsequent assertion crashes.

if (nread <= 0) {
  if (nread < 0) {
    PassReadErrorToPreviousListener(nread);
    // Close the session to prevent zombie state when the underlying socket is dead.
    Close(NGHTTP2_NO_ERROR, true);
  }
  return;
}

Key points:

  • Minimal change: only affects the read-error path.
  • Close() is idempotent / safe to call redundantly.
  • Preserves callback ordering (previous listener notified first).

Why is this correct?

A negative nread indicates an error/EOF condition at the stream/socket read layer. Keeping the HTTP/2 session alive after a read error allows the session to continue queueing outbound frames while the transport is effectively dead, which eventually leads to inconsistent internal write state and assertions.

Closing the session on read error restores the expected lifecycle invariant: dead transport ⇒ closed session.


How was this tested?

New test (CI-friendly)

Adds a new parallel test that reproduces the “zombie” behavior without OS firewall rules by destroying the underlying socket directly:

  • client[kSocket].destroy() to kill the transport.
  • Attempt follow-up requests.
  • Verify: no process crash, session cleans up, and the close path is taken.

New file:

  • test/parallel/test-http2-zombie-session-61304.js

Commands

Built locally (macOS arm64):

./configure
make -j1

Ran test:

out/Release/node test/parallel/test-http2-zombie-session-61304.js

Spot-checked a related HTTP/2 test:

out/Release/node test/parallel/test-http2-client-socket-destroy.js

Additional context / reproduction (original issue)

The original issue can be reproduced reliably on macOS using pf firewall rules to drop packets (reported 100% reproduction), creating a network “black hole” where the socket becomes dead without Node observing a clean close.

Issue: #61304


Files changed

  • src/node_http2.cc (small change: close session on nread < 0)
  • test/parallel/test-http2-zombie-session-61304.js (new)

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion failure crash in TLSWrap::DoWrite with zombie HTTP/2 session (close event not propagated from OS-level CLOSED socket)

2 participants