Skip to content

node:http2: hold the FIN behind output owed to a peer that half-closed - #46380

Draft
robobun wants to merge 9 commits into
mainfrom
robobun/b728aa13/http2-peer-half-close
Draft

robobun wants to merge 9 commits into
mainfrom
robobun/b728aa13/http2-peer-half-close

Conversation

@robobun

@robobun robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Draft. The branch contains #45673: review commit 5f42abe2e05. Two questions for a maintainer open the Notes.

Problem

  • An HTTP/2 peer sends a request, then a FIN, and keeps reading. It gets 2,652,707 of 16,777,216 bytes and no error. Node delivers all. No issue reports this.
  • endNT (src/js/node/net.ts:441) sends the socket's FIN while the session's parser still holds frames.

Fix

  • shutdown() asks the attached parser (H2FrameParser::defer_shutdown). A parser that owes output to a peer that ended its side sends the FIN after that output.
  • endNT reads that from a private helper, isShutdownHeld. It parks the _final callback until the drain that follows the FIN.
  • Verified: test/js/node/http2/node-http2-graceful-close.test.ts (36 of 48 new cases fail on node:http2: keep the unsent frames on a graceful close and send the FIN after them #45673). Also test/js/node/{http2,net,tls}.
  • Self-reviewed: 41 concerns raised, 33 addressed, 5 in part. 3 wait for a maintainer (Notes).

Background

  • A node:http2 session writes through a native parser on the socket, which buffers what the kernel refuses.
  • Socket#shutdown() keeps its type. The helper is a bindgen function, like releaseHeldFlight.
  • Considered holding only the peer's EOF natively: 'finish' would still precede the bytes.

Downsides

  • A peer that sends a FIN and reads nothing keeps the socket and its response, as in Node.
  • A client waits for its upload. After an nginx 413 for 64 MiB it stays 81 to 131 ms. Before: 9 to 24 ms.
  • Every net.Socket pays +57 instructions per end and +4 bytecodes per drain. The binary grows by 2,621 bytes.
Notes

Questions for a maintainer. This PR asks for no review until #45673 has merged and these have an answer. They are also on #43877.

  1. May _final of a net.Socket wait for a FIN that its handle holds? This PR adds a second parked callback to node:net (kshutdownCallback, next to kwriteCallback), completed from seven places. Without it the hold can stay native, with no net.ts change. 'finish' then comes before the kernel has the bytes, and a socket whose peer has ended is destroyed at that 'finish'.
  2. Which output does the held FIN wait for? This PR waits until a writable event finds the session's buffer empty. So it sends what the session writes while the FIN waits. No Node version does that (table below): Node sends what its socket had when the FIN was asked, and nothing later. Node's rule would also cover a raw socket.end(), at the cost of one check in every frame write and of the END_STREAM of a compat response.

What each runtime sends. linux-x64 loopback, release builds, default kernel buffers. A raw peer opens 2^31-1 windows, sends GET /, then a FIN, and reads. all is every byte of the body.

response transport Node v22.23.4, v24.21.0, v26.3.0 Node v24.22.1, v26.12.1 main + #45673 this PR
stream.end(16 MiB) TCP all, END_STREAM all, END_STREAM 2,652,707 all, END_STREAM
TLS all, END_STREAM all, END_STREAM 2,619,957 all, END_STREAM
write(16 MiB); end(16 MiB) TCP all, END_STREAM 16,777,216 2,652,707 all, END_STREAM
TLS 16,777,216 16,777,216 2,619,957 all, END_STREAM
compat res.end(16 MiB) TCP, TLS all, no END_STREAM all, no END_STREAM 2,652,707, 2,619,957 all, END_STREAM
8 MiB in 64 KiB writes on 'drain' TCP 196,608 196,608 2,586,865 all, END_STREAM
TLS 65,536 65,536 2,621,440 all, END_STREAM
socket.end() a turn after stream.end(16 MiB), no FIN from the peer TCP, TLS all, END_STREAM all, END_STREAM 2,652,707, 2,619,957 2,652,707, 2,619,957
  • The two Node columns differ since http2: fix write deadlock exposed by larger window sizes nodejs/node#65440: Node no longer stops reading while a session write is pending, so it reads the FIN earlier.
  • The 'drain' row depends on timing in this PR. A peer in the same process gets all of it. A slower peer can find the buffer empty between two writes, and the FIN goes out there.
  • Peer in a process of its own, stream.end(16 MiB): 0 of 100 runs cut over TCP, 0 of 30 over TLS.
  • Client upload when the server ends its side: 2,718,241 bytes before, 16,777,216 after (TCP, TLS, and TLS on a socket that tls.connect({ socket }) adopted). Unix socket: 219,104 before.

Who sees the bug. No issue reports it. The trigger is a peer that ends its side with a stream open, has granted more window than one kernel write takes, and keeps reading. The tests use a raw-socket peer for that. A server can avoid the cut today: http2.createServer({ allowHalfOpen: true }) and session.socket.once("end", () => session.setTimeout(300, () => session.close())) delivered the whole body on the #45673 head and on Bun 1.4.3. That is a timer, and it does not cover a client.

Details of the fix.

  • The public Socket#shutdown() and its type do not change. NewSocket::shutdown already asks the parser since node:http2: keep the unsent frames on a graceful close and send the FIN after them #45673. node:net then reads isShutdownHeld(handle), a bindgen function in socket_body.bind.ts like releaseHeldFlight and getBufferedAmount.
  • defer_shutdown first writes what the socket takes now: the frames of the turn, the buffer, and DATA that is queued on a stream. Without that write a FIN could pass queued DATA: write(4 MiB); end(12 MiB) lost its tail in 9 of 300 two-process runs, and in 0 of 300 with it. A fault-injection case and an ASAN fixture pin this write and the keepalive around it.
  • The parser asks the socket whether the peer has ended (us_socket_read_ended). A session that gets its socket after the FIN is covered (server.emit("connection", socket)).
  • DATA that waits for the peer's window does not hold the FIN. The peer cannot send WINDOW_UPDATE after its FIN.
  • session.close() or destroy() while the FIN waits goes through release_with_held_fin. A close keeps the FIN behind the output. A destroy sends it at once.
  • The parked callback runs in the async context of the end() call, and completes when the handle drains, closes, is destroyed or dials again.
  • The peer's FIN drops the handle's hold on the event loop. A parked end takes it back, so a client process stays until its upload has left.
  • While an end waits after the peer's FIN, a reset is not reported as a socket error. Node reports none: Http2Session::ClearOutgoing completes the writes with status 0.

Differences from Node that remain. Each one is in a test comment.

  • Everything in the table where this PR sends more than Node: a second write, the END_STREAM of a compat response or of trailers, a body written on 'drain', and TLS 1.2 (Node v26.3.0 sends nothing of the response there).
  • session.close() for a peer that already ended its side: Bun emits 'end' before 'finish' over TCP and sends one GOAWAY. Node v26.3.0 emits 'finish' before 'end' and sends two.
  • An unref'd client whose upload cannot leave: Bun exits, as before. Node stays.
  • A reset under a parked end whose peer has not sent a FIN is reported as ECONNRESET when the read side sees it first. When a send sees it first, the socket finishes and closes without an error, as in Node.

Not covered. This PR changes none of these.

  • Supported calls that still cut. session.destroy() in the turn of stream.end(): 2,652,707 bytes, Node sends 16,777,216. A response that is written after the socket has asked for its FIN, for example by an async handler.
  • Calls on the raw socket of a session, which Node's documentation tells users to avoid. socket.end() or destroySoon() while the peer still has its side open: last row of the table, and an it.todo in the test file. end() on the raw half of a socket that tls.connect({ socket }) adopted.
  • Other engines. Bun.serve({ http2: true }) cuts the same peer at 2,587,190 bytes. Sessions on a JS Duplex transport do not pass through this code.
  • A client socket with allowHalfOpen: true whose process exits with an upload pending.

Order with the open PRs.

Measurements. Release builds, main with #45673 against this PR. gdb single-step, objdump, size, and an LD_PRELOAD counter. strace, perf, valgrind and bloaty are not installed in this environment.

  • .text: 107,351,210 to 107,353,831 bytes (+2,621). Bundled node:net: +1,341 bytes. One more native function object when node:net loads.
  • A net.Socket end with no parser: shutdown() takes 117 instructions before and after, and the new isShutdownHeld() call adds 57.
  • Writable event with no parser: 38 before and after. HTTP/2 writable event: +2 (one mov, one alignment nop). EOF event: no change.
  • node:net drain dispatch with nothing parked: 11 to 15 bytecodes.
  • net.Socket: 41 own keys and 240 shallow bytes before and after. H2FrameParser: 1,496 bytes before and after.
  • GC cells per session teardown, over 1,000 sessions: no growth.
  • Socket calls of the server for a 1,000 byte response: identical (send 3, recv 2, shutdown 1, epoll_ctl 7). For 16 MiB: one shutdown(SHUT_WR) in both, after 6,648,689 bytes before and after 16,786,493 bytes now (measured before the merge of main).
  • nginx 1.31.7 answers a 64 MiB upload with 413 and a lingering close. Before, the client sent 10,971,619 to 21,514,382 bytes and left 9 to 24 ms after the 413. It now sends all 67,146,878 bytes and leaves after 81 to 131 ms. Node v26.3.0: 3 ms.

Suites. test/js/node/http2, net, tls and test/js/bun/net on a debug build of this branch, and test/js/node/http before the merge of main. The 660 vendored node tests test-(http2|net|tls|https)-*: 657 pass before and after, the same 3 fail. cargo check for windows-msvc and aarch64-apple-darwin. Windows and macOS were compiled and not run.

Self-review. Two passes. The first asked for one shutdown contract, a cut-off for a peer that keeps sending, and the flush at the ask. The second raised 41 points.

  • Addressed (33): a private helper in place of a boolean shutdown(), the peer's EOF read from the socket, completion when the handle dials again, the async context, one-line comments, capped socket buffers and 1 MiB bodies in the tests, pinned values in place of loose matchers, TLS rows, two-stream cases, trailers, TLS 1.2, the fault-injection case and the ASAN fixture, and the claims about Node in this text.
  • Wait for question 2 (3): a cut-off for output that is written after the FIN is asked, the hold for a raw socket.end(), and the Node version to follow.
  • In part (5): no Node cross-check file, because Node's own results differ between v26.3 and v26.9. No injected send-fault cases for the reset sequences. No cluster or worker-thread case. No client twin of the request-with-body case. No TLS row where a send sees the reset first.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http2/node-http2-graceful-close.test.ts

A stream counts its frames as sent when they are serialized. On a
graceful close the session was destroyed while part of a body was still
in the parser's write buffer: detach() freed it, and the socket's FIN
followed the bytes the kernel already had.

parser.detach(true) keeps that output. The parser stays in the socket's
native-callback slot, ignores inbound frames, writes the output from
writable events and then lets go of the socket. A shutdown() of the
socket in that time is held back, and the parser issues it after the
last byte.

The session 'close' event of a graceful close now follows the socket's
'close', as in node.
…the slot's own ref

The match repeated the one in on_writable arm for arm (mordant same_match_twice).
defer_shutdown runs no JS, so it does not need the pointer copied out first.
A peer sends its request, ends its side of the connection and keeps
reading. net.Socket answers that FIN with end(). The session still holds
frames that the socket has not taken, and the socket's FIN went out ahead
of them.

node:net now ends the write side of a native handle through a private
helper. The helper asks the attached HTTP/2 parser first. The parser
writes what the socket takes now. If output is still owed and the peer
has ended its side, the parser keeps the FIN and the helper answers
false. The parser sends the FIN from the writable event that finds
nothing owed, and the socket then runs its drain dispatch.

node:net parks the _final callback on `false`. It completes the callback
from that drain, from the close of the handle, from destroy(), or when
the handle dials again. After the peer's FIN a parked end holds the event
loop, as a pending write does. The public Socket#shutdown() is unchanged.
@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:46 PM PT - Oct 21st, 2026

❌ @robobun, your commit aff161e has 2 failures in Build #130350 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 46380

That installs a local version of the PR into your bun-46380 executable, so you can run:

bun-46380 --bun

@robobun
robobun changed the base branch from robobun/953366a6/http2-graceful-close-pending-data to main October 21, 2026 20:42

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant