Skip to content

tls: keep a graceful close waiting for the peer's close_notify - #46349

Draft
robobun wants to merge 3 commits into
mainfrom
robobun/676146dd/tls-close-waits-for-reply
Draft

robobun wants to merge 3 commits into
mainfrom
robobun/676146dd/tls-close-waits-for-reply

Conversation

@robobun

@robobun robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • end() on a TLS socket can close the fd while the peer still sends. That close is a reset: the peer loses bytes that write() reported. CI: "TLS end() while the peer is still sending", ["close: ECONNRESET"].
  • It closed early when the kernel refused our close_notify (packages/bun-usockets/src/crypto/openssl.c:2127), and when it was asked again (:2104).

Fix

  • A refused close_notify stays queued. The loop keeps the poll for writable, and the writable event sends it.
  • us_internal_ssl_close returns early for a code-0 close while one waits. The peer's close_notify or FIN, a forced close or the 10 s deadline ends the wait.
  • Verified: test/js/bun/net/socket.test.ts (16 rows fail on main). Other suites: Notes.
  • Self-reviewed: 35 concerns raised, 32 addressed. Rejected: two PRs (Windows needs both), a us_socket_timeout guard (+11 instructions per HTTP request), a ruling on plain TCP first.

Background

Downsides

Notes

What changes. A graceful close (code 0) of a TLS socket waits for the peer's close_notify. Two verdicts ended that wait early, and both are gone:

  • The kernel refused the close_notify (return ret == 1 || s->ssl_spilled). usockets: don't defer TLS close when close_notify flush fails #30368 chose "close now without it" and tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618 kept it, because a socket that is shut down lost its poll for writable and nothing sent the alert later. Now the alert stays in the per-socket spill like any other record, loop.c keeps the poll for writable while a shut-down TLS socket holds a spill, and ssl_on_writable sends it. That event is not dispatched to the owner while its close waits: fetch answers a writable event on a socket that it closed with a reset. A close with code 1 or 2 does not wait for the alert.
  • The close was asked again. us_internal_ssl_close now returns for a code-0 close while one waits.

Three owners got "close now" from the second ask and now say so. us_socket_group_close_all_ex ends a waiting close as the deadline does. Http2Context::closeAll uses us_socket_close_now (the same line as #45990). HttpResponse::closeIfIdle sends its FIN before the close (shutdown(); close();, as the other closes of a finished HTTP/1 connection). Http2Connection::touch leaves the deadline of a waiting close alone, and Http2Context::closeIdle counts a connection once.

Evidence

  • No user reported this. On windows 11 aarch64 the CI row fails in 33 of 312 builds and passes on retry.
  • Windows, build 130170: the repeat-ask part alone, with that row repeated 60 times per attempt. On windows 11 aarch64 8 of 240 runs fail, each closed inside end() with nothing read after it: the refused close_notify. On windows 2019 x64 three of the new rows fail in every attempt the same way: the write ends in a drain event, end() follows, and the peer misses the last 131072 bytes.
  • Linux, the row's own handlers with the peer in another process, one end(), nothing injected: the release canary 1.4.4-canary.1 (9b4284f) resets the peer or cuts its upload in 32 of 3,749 runs, from 4 MiB up (20 of 586 at 4 MiB, 0 of 100 at 1 MiB). Under gdb all 41 early closes of a debug build are the second ask: a refused send() leaves the poll for writable armed, end() runs outside a writable event, a data event attaches the JS wrapper again, and the writable event reaches NewSocket::on_writable -> internal_flush -> us_socket_close(s, 0). None of 1,400 closes had a refused close_notify there.
  • The same handlers on debug builds of main 2d6d445 and of the repeat-ask part, interleaved, with a 40 ms busy pause after a short write and a 7 MiB write: main resets the peer in 18 of 40 runs (the client gets ["end", "close: ECONNRESET"], the server has read 49152 of 8388608 bytes). The branch: 0 of 40.
  • macOS was not run.

Measured on Linux x64, main 2d6d445 against this branch. Objects are compiled with the release flags (x64, -O3) and no LTO.

  • us_socket_t: no new field and no new bit.
  • Per TLS write and read: +0 instructions. In openssl.o three functions differ: us_internal_ssl_close (195 -> 228 instructions), ssl_handle_shutdown (60 -> 58) and ssl_on_writable (126 -> 130). Two are new: us_internal_ssl_close_waits (39) and ssl_close_awaits_peer (21). In loop.o only the block behind a failed write in us_internal_dispatch_ready_poll differs. socket.o is identical (llvm-objdump).
  • A first graceful close of a TLS socket: +52 instructions (26 in us_internal_ssl_close, then 7, 7 and 12 in us_socket_is_closed, us_internal_poll_type and SSL_get_shutdown, +2 for the spill check, -2 in ssl_handle_shutdown). A close with code 1 or 2: +6. A plain TCP close: +0.
  • A writable event of a TLS socket: +44 instructions (4 in ssl_on_writable, 21 in ssl_close_awaits_peer, 7 and 12 in the two callees). A writable event with no failed write runs nothing new in loop.c.
  • Per TLS connection (connect, write 1 KiB, end()): sendto 5, recvfrom 4, epoll_ctl 5, close 2, shutdown 0 on both builds (gdb catch syscall, slope over 10 and 30 connections).
  • A refused close_notify, both ends in one process: main 4 sendto and 8 epoll_ctl per connection, this branch 6 and 9. The two more sends are the alert and the peer's answer to it.
  • HTTP/2: +2 instructions per Http2Connection::touch() before a GOAWAY. HTTP/1 request path: +0.
  • us_socket_group_close_all_ex: 82 -> 96 instructions, and one call to us_internal_ssl_close_waits per TLS socket.
  • One idle sweep over TLS keep-alive connections whose client answers close_notify, per connection: sendto 3 -> 3, recvfrom 3 -> 2, shutdown 0 -> 1, close 1 -> 1. RSTs sent: 0 -> 1 per connection (20 of 20, /proc/net/snmp), with 0 client errors. A Connection: close response on main draws the same RST (20 of 20). Plain HTTP sweep: 0 -> 0.
  • size text: openssl.o +282, loop.o +29, context.o +48, the uWS C API unit (src/uws_sys) +310.
  • LeakSanitizer with the CI settings: 0 reports in 10 runs of the new exit row and in 5 runs of node-https-checkServerIdentity.test.ts, the test that found the leak behind usockets: don't defer TLS close when close_notify flush fails #30368.

Closes of a TLS connection whose client never answers close_notify. The client is node v26.3.0 with allowHalfOpen. The time is from the last ask to the close of the accepted fd, on debug builds.

Flow main this branch
Bun.serve HTTP/1 closeIdleConnections() twice at the second call, returns 1 and 1 at the first call, returns 1 and 0
Bun.serve HTTP/1 stop() 11.8 s 25 ms
node:https close() 5.4 s 138 ms
node:https close() then closeIdleConnections() at the second call at the first call
Bun.serve HTTP/2 closeIdleConnections() twice 19 ms, returns 1 and 1 11.5 s, returns 1 and 0
Bun.serve HTTP/2 stop() then closeIdleConnections() 24 ms 11.5 s
Bun.serve HTTP/2 closeIdleConnections() in an abort listener 17 ms 11.7 s
the same with a PING in the read of the RST_STREAM, idleTimeout: 0 still open after 20 s 11.8 s
Bun.serve HTTP/2 stop() 11.8 s 11.8 s
Bun.serve HTTP/2 stop() then stop(true) 20 ms 19 ms
Bun.serve HTTP/2 stop(true) 11.8 s 25 ms

On main an HTTP/2 connection closes at once when something asks twice, and after 11.8 s when it is asked once. On this branch it waits for the client or the deadline in both cases. A client that answers close_notify closes at once on both builds.

The code-0 closes of a TLS socket, and what a second one did

Who asks again main this branch
NewSocket::internal_flush after end(): the writable event, flush(), a second end(), [Symbol.dispose](), the other half of an upgradeTLS() pair closes over the peer's data: a reset keeps waiting
us_internal_ssl_on_data: the peer's close_notify while the close waits for the kernel drops the unsent ciphertext sends it, then closes
us_socket_group_close_all_ex: stop(true) of a listener, teardown clean close clean close, through the new branch
Http2Context::closeAll: stop(true), free() closes us_socket_close_now
HttpResponse::closeIfIdle: the HTTP/1 idle sweep closes at the second sweep closes at the first sweep, with a FIN
Http2Context::closeIdle, sweep, epilogue closes keeps waiting
A sweep after AsyncSocket::close() at HttpContext.h:506 (a pipelined request while a response is pending) or :577 (no route) closes keeps waiting
HTTPClient::fail twice (WebSocket upgrade through an https proxy) closes keeps waiting, the failure is already reported

The other code-0 callers ask once: fetch (HTTPContext::close_socket), the node:http2 transport (h2_frame_parser.rs:3056), the closes after a FIN (shutdown(); close(); in HttpContext.h and HttpResponse.h), and the closes on the peer's FIN.

Tests. Linux x64, debug builds of main 2d6d445 and of this branch.

  • test/js/bun/net/socket.test.ts: 16 rows fail on main, 12 of them with no fault injection. The peer sends until the ender has read after its end(), so the rows do not depend on buffer sizes, and each row asserts that the peer heard of the end() before the ender closed: a close at the deadline also delivers, 10 s late. Five rows start the close behind unsent ciphertext. The existing fixed-deadline test now asks again every 3 s.
  • Rows that pass on main and pin one hunk each, checked on a build without that hunk: the two stop(true) rows (us_socket_group_close_all_ex), the fetch row (no writable dispatch while a close waits: without it the server reads 81769 of 1048576 bytes, 3 of 3), "in a drain event" (loop.c: without it the close comes at the deadline), "close() ... closes at once" (the spill check in us_internal_ssl_close), and the exit row under LeakSanitizer.
  • test/js/bun/http/serve-http2.test.ts: the count row and the HTTP/1.1 stop() row fail on main. The stop(true) after stop() row and the abort listener row pass on main and fail without closeAll and the touch() guard.
  • test/js/bun/http/serve-syscall-fault.test.ts: two sweeps behind a response that the kernel refused. Main delivers 0 of 65536 bytes.
  • test/js/node/http/node-http-server-close-drain.test.ts: https.Server.close(cb) with a silent client.

Suites. 91 test files on a debug build with a 30 s test timeout: test/js/node/tls, test/js/node/http2, test/js/bun/net, the serve*, bun-server, node-http*, node-https* and node-net* files, fetch over TLS, WebSocket and ws, and eight *-syscall-fault files. 5,483 tests pass. 12 tests in 6 files fail, and the same 12 fail on a build of main in this container: they need the public network or a non-root user, or time out in a debug build.

Not fixed here

  • us_internal_ssl_shutdown (socket.shutdown(), node:tls end()) still drops a close_notify that the kernel refuses, and sends a bare FIN.
  • A peer's FIN behind unsent ciphertext closes over it (loop.c, the close after on_end).
  • The deadline closes over a peer that still sends after 10 s: that peer gets EPIPE (measured, main and this branch).
  • Plain TCP end() closes at once. In the row's scenario the peer gets 65536 of 1048576 bytes with clean end and close events, 3 of 3, on main and on this branch. Bun.serve http2: close a connection gracefully with a FIN, not with close(2) #44874 asks where an HTTP/2 close lingers, and Bun.connect: make end()/shutdown() a write-side half-close; fix timeout() docs #35947 (closed with no ruling) did it for Bun sockets. A maintainer ruling on one place for both transports would replace the TLS-only predicate of this PR.
  • The deadline is the socket's timeout, and an owner that sets the timeout after its close moves it. Http2Connection::touch no longer does. socket.timeout(0) in a data event after end(), and an HTTP/1 response that resets its timeout after the close at HttpContext.h:506, still leave the socket open until the peer closes, as on main. A guard in us_socket_timeout and us_socket_long_timeout covers every owner at +11 instructions per HTTP request and +17 per HTTPS request (measured in the self-review, no LTO).

Self-review. It asked for two PRs, the repeat-ask part first. The Windows lanes of build 130170 show that the rows of that part need the other part. The PR is not stacked on #45990: the shared line is identical, and git merge-tree reports no conflict.

Related open PRs

A graceful close (code 0) of a TLS socket sends close_notify and reads
on for the peer's. A second code-0 close closed the fd at once. When
the peer still sends, that close is a reset, and the kernel drops bytes
that write() reported.

us_internal_ssl_close now returns early for a code-0 close while one
already waits, for the kernel to take unsent ciphertext or for the
peer's close_notify. The peer's close_notify or FIN, a close with code
1 or 2, us_socket_close_now and the 10 s deadline end the wait.

The owners that got "close now" from the second ask now say so:
- us_socket_group_close_all ends a waiting close as its deadline does.
- Http2Context::closeAll uses us_socket_close_now.
- The HTTP/1 idle sweep sends its FIN: shutdown, then close.
- Http2Connection::touch leaves the deadline of a waiting close alone,
  and Http2Context::closeIdle counts a connection once.
The row "TLS end() while the peer is still sending delivers a write
that the kernel took at once" fails on the windows 11 aarch64 lane in
about 1 of 10 builds of main and passes on retry. This commit repeats
that row 60 times on the Windows lanes, with the shape of the ender's
write in the failure message. It is reverted before the merge.
@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 5:05 PM PT - Oct 21st, 2026

❌ @robobun, your commit d1dec4b has some failures in Build #130241 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 46349

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

bun-46349 --bun

@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: draft. #46349

How it was reproduced:

  • Linux x64, debug build of main 2d6d445: bun bd test test/js/bun/net/socket.test.ts fails 16 rows, 12 of them with no fault injection. The peer gets close: ECONNRESET and misses bytes that write() reported. 2 rows of serve-http2.test.ts, 1 of serve-syscall-fault.test.ts and 1 of node-http-server-close-drain.test.ts fail too. All of them pass on this branch.
  • Windows, CI build 130170, the first commit alone: the row that flakes on windows 11 aarch64 failed 8 of 240 runs, each closed inside end() with nothing read after it. That is the close_notify that the kernel refuses. The commit d1dec4b adds that part.

The commit "test: temporary lane probe" repeats that row 60 times on the Windows lanes. It is reverted once those lanes have run with both parts.

A graceful close of a TLS socket closed the fd at once when the kernel
refused the close_notify. With input of the peer unread, that close is
a reset, and the kernel drops what it has not sent. On Windows a
write() followed by end() gets there: the kernel takes the write and
refuses the next send.

ssl_handle_shutdown no longer reports a spilled close_notify as a
finished shutdown. The alert stays in the spill like any other record,
the loop keeps the poll for writable of a shut-down TLS socket that
still holds a spill, and the writable event sends it. That event is
not dispatched to an owner whose close waits for the peer: the owner
let go, and fetch answers such an event with a reset. A forced close
does not wait for the alert.

This reverses "close now without it" from #30368, which #44618 kept.
The reason was that nothing sent the alert later.
@robobun robobun changed the title tls: keep a graceful close waiting when it is asked again tls: keep a graceful close waiting for the peer's close_notify Oct 21, 2026

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