node:tls: report a peer reset on a wrap of an accepted socket

Merged
RO
2mo ago
+89-9

Problem

  • A server-side wrap of an accepted socket (new tls.TLSSocket(raw, { isServer: true }), tlsServer.emit('connection', raw)) no longer reports a peer RST. The TLS socket emits 'close' with hadError === false and no 'error'. During the handshake the tls.Server gets no 'tlsClientError'. Bun 1.4.2 and Node report ECONNRESET.
  • Regression from #39066: the wrapped socket's 'close' now destroys the TLS socket (onUpgradedClose, src/js/node/net.ts:273). The native on_close (src/runtime/socket/socket_body.rs:2122) runs the raw socket's close callback, drains the tick queue (raw 'close', so the destroy), and only then runs the TLS socket's own close callback with ECONNRESET.

Fix

  • on_close holds the event loop entered (EventLoop::enter_scope) across both close callbacks of an upgradeTLS pair. The tick queue drains after both ran, not between them.
  • The TLS socket's own close callback now destroys it with ECONNRESET before the raw socket's 'close' event runs. That listener then finds the socket destroyed. net.ts does not change.
  • The JS events keep the 1.4.2 order: raw error, raw close, tls error, tls close.
  • Verified: test/js/node/tls/node-tls-socket-allow-half-open-option.test.ts, 2 new tests, both fail on main's src/. Also test/js/node/tls/, net/, http2/, test/js/bun/net/, vendored test-tls-* and test-https-*.

Background

  • A wrap is TLS over an existing socket. For a connected net.Socket Bun adopts the fd: upgradeTLS returns a raw handle, which stays on the wrapped socket, and a TLS handle for the TLS socket.
  • Only the TLS handle gets native events. Its on_close first calls the raw handle's close callback (the "twin") as a full dispatch of its own.
  • Each dispatch brackets its JS callback with enter() and exit() on the event loop. The outermost exit() drains process.nextTick and microtasks. The twin's dispatch was an outermost pair, so it drained before the TLS callback ran.
Notes

Scope

The raw socket needs an 'error' listener of its own for this to show, the usual STARTTLS server shape. Without one it is not destroyed on the reset, so it emits no 'close'.

CloseTeardown is now created ahead of the scope, so it drops after it. The drain still comes ahead of the teardown (mark_inactive, the final deref), as it does for a socket with no twin. The three early returns lose their explicit drop(cleanup) for the same reason: the natural drop order is the scope first, then the teardown.

The change applies to every upgradeTLS pair, also the ones from Bun.connect and socket.upgradeTLS(). What moves is when the ticks queued by the raw socket's close handler run: after the TLS socket's close handler, not before it.

An earlier revision of this PR deferred the destroy in net.ts with a setImmediate when it saw the state between the two callbacks. Review asked to fix the ordering at its source, which is this version.

Since #39066 a wrap takes allowHalfOpen from the wrapped socket. A half-open wrap does not end itself after EOF, so the wrapped socket's 'close' must destroy it. This change keeps that.

Measurements

Linux x64. "main" is a debug+ASAN build of 81f97bb0. "before" is bun 1.4.3-canary.1+4ff919377, which predates #39066 and prints what 1.4.2 prints. Node is v26.3.0. The server wraps the accepted socket and also listens for 'error' on it. The client does resetAndDestroy().

casenodebeforemainthis PR
new TLSSocket(raw), after the handshaketls error ECONNRESET, raw close, tls close:trueraw error, raw close, tls error ECONNRESET, tls close:trueraw error, raw close, tls close:falsesame as before
tlsServer.emit('connection', raw), after the handshakesamesamesamesame as before
new TLSSocket(raw), during the handshaketls error ECONNRESET, raw close, tls close:trueraw error, raw close, tls error ECONNRESET, tls close:trueraw error, raw close, tls close:falsesame as before
tlsServer.emit('connection', raw), during the handshaketlsClientError ECONNRESETtlsClientError ECONNRESETnothingtlsClientError ECONNRESET

tls.connect({ socket }) over a net.Socket it dialed itself was not affected. Its raw socket keeps its handle through the close callback, so _destroy takes the kAdoptedTLSRaw branch and 'close' comes two check phases later (closeAdoptedTLSRawNT). An accepted socket's callback (ServerHandlers.close) detaches the handle first, so 'close' comes from process.nextTick.

The trace

BUN_DEBUG_Socket=1 BUN_DEBUG_JS=1 on main, after the client's RST:

[socket] onClose C            <- TLS handle
[socket] onClose S            <- raw handle, dispatched from inside the first
[net] Bun.Server close        <- raw socket's callback: destroy(ECONNRESET)
[events] Socket.emit error
[events] Socket.emit close    <- onUpgradedClose: tlsSocket.destroy()
[net] Socket.prototype._destroy
[net] Bun.Server close        <- TLS socket's callback, socket already destroyed

Relation to #42176

#42176 now carries the other face of the #39066 regression: a TLS socket over a stream.Duplex that loses unread data when the duplex closes. That is the stream-engine path and it changes onUpgradedClose. This PR touches neither that function nor net.ts, so the two do not conflict.


[human-review] gate passed · iteration 0 · 2 files touched

fails on main (without fix)
ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-socket-allow-half-open-option.test.ts
bun test v1.4.3 (4ff919377)

test/js/node/tls/node-tls-socket-allow-half-open-option.test.ts:
(pass) TLSSocket allowHalfOpen > new TLSSocket(socket) takes the wrapped socket's allowHalfOpen, ignoring the option [228.65ms]
(pass) TLSSocket allowHalfOpen > without a socket to wrap, the option is honored [10.30ms]
(pass) TLSSocket allowHalfOpen > tls.connect({ socket }) takes the given socket's allowHalfOpen, ignoring the option [207.34ms]
(pass) TLSSocket allowHalfOpen > over a connection > a server-side wrap of a half-open socket stays writable after the peer ends [946.31ms]
(pass) TLSSocket allowHalfOpen > over a connection > a server-side wrap of a regular socket ends itself after the peer ends, even with allowHalfOpen: true [836.51ms]
(pass) TLSSocket allowHalfOpen > over a connection > a socket injected into a tls.Server keeps its own allowHalfOpen [845.82ms]
(pass) TLSSocket allowHalfOpen > over a connection > tls.connect({ socket }) over a half-open socket stays writab
... (truncated)

release without fix: all passed
bun test v1.4.3-canary.1 (00fab6991)

test/js/node/tls/node-tls-socket-allow-half-open-option.test.ts:
(pass) TLSSocket allowHalfOpen > new TLSSocket(socket) takes the wrapped socket's allowHalfOpen, ignoring the option [8.83ms]
(pass) TLSSocket allowHalfOpen > without a socket to wrap, the option is honored [0.16ms]
(pass) TLSSocket allowHalfOpen > tls.connect({ socket }) takes the given socket's allowHalfOpen, ignoring the option [4.39ms]
(pass) TLSSocket allowHalfOpen > a peer reset under a server-side wrap > a socket injected into a tls.Server: a reset during the handshake is a 'tlsClientError' [20.27ms]
(pass) TLSSocket allowHalfOpen > the wrapped socket closing > new TLSSocket(duplex, { isServer }): the duplex closing [36.67ms]
(pass) TLSSocket allowHalfOpen > over a connection > a server-side wrap of a half-open socket stays writable after the peer ends [48.42ms]
(pass) TLSSocket allowHalfOpen > over a connection > a server-side wrap of a regular socket ends itself after the peer ends, even with allowHalfOpen: true [44.45ms]
(pass) TLSSocket allowHalfOpen > over a connection > a socket injected into a tls.Server keeps its own allowHalfOpen [44.65ms]
(pass) TL
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-socket-allow-half-open-option.test.ts
bun test v1.4.3 (4ff919377)

test/js/node/tls/node-tls-socket-allow-half-open-option.test.ts:
(pass) TLSSocket allowHalfOpen > new TLSSocket(socket) takes the wrapped socket's allowHalfOpen, ignoring the option [215.09ms]
(pass) TLSSocket allowHalfOpen > without a socket to wrap, the option is honored [9.47ms]
(pass) TLSSocket allowHalfOpen > tls.connect({ socket }) takes the given socket's allowHalfOpen, ignoring the option [206.47ms]
(pass) TLSSocket allowHalfOpen > over a connection > a server-side wrap of a half-open socket stays writable after the peer ends [953.62ms]
(pass) TLSSocket allowHalfOpen > over a connection > a server-side wrap of a regular socket ends itself after the peer ends, even with allowHalfOpen: true [840.71ms]
(pass) TLSSocket allowHalfOpen > over a connection > a socket injected into a tls.Server keeps its own allowHalfOpen [839.34ms]
(pass) TLSSocket allowHalfOpen > over a connection > tls.connect({ socket }) over a half-open socket stays writabl
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 692ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/6] gen generated_host_exports.rs
generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited
[1/6] cargo bun_runtime → libbun_runtime.a
   Compiling bun_core v0.0.0 (/workspace/bun/src/bun_core)
   Compiling bun_errno v0.0.0 (/workspace/bun/src/errno)
   Compiling bun_ptr v0.0.0 (/workspace/bun/src/ptr)
   Compiling bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
   Compiling bun_safety v0.0.0 (/workspace/bun/src/safety)
   Compiling bun_base64 v0.0.0 (/workspace/bun/src/base64)
   Compiling bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
   Compiling bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
   Compiling bun_zstd v0.0.0 (/workspace/bun/src/zstd)
   Compiling bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
   Compiling bun_paths v0.0.0 (/workspace/bun/src/paths)
   Compiling b
... (truncated)
diff hotspot
src/runtime/socket/socket_body.rs                  | 21 +++---
 .../node-tls-socket-allow-half-open-option.test.ts | 77 ++++++++++++++++++++++
 2 files changed, 89 insertions(+), 9 deletions(-)

gate history · 2 passed · 0 rejected · iteration 0

evidence per changed file
file                                                      reads  edits  tests
src/runtime/socket/socket_body.rs                             4      1     23
…node/tls/node-tls-socket-allow-half-open-option.test.ts      4      4     23