Deno Individual Contributor License Agreement

All contributors have signed the CLA. Thank you!

• @tomas-zijdemans

Re-run CLA check

This is an automated message from CLA Assistant

Thanks for the careful review.

Deferred close_notify (main concern): fixed. The terminal end() of the underlying stream is now gated on the handshake being complete (res._owner?._secureEstablished), so .end() mid-handshake no longer tears
the transport down before the deferred close_notify is produced.
The ordering holds up: in dispatch_clear_out_callbacks, _secureEstablished is set (via onhandshakedone) before the deferred send_close_notify(), so the pump drains/writes the close_notify first and only ends the stream once secure.

Tests: added the symmetric client-side .end() and the end()-during-handshake (deferred) cases.

Worth flagging: writing those tests showed my first versions didn't actually reproduce the bug, which sharpened the root cause:

• It only surfaces when both ends are JS-backed. With a native peer the TCP FIN delivers EOF regardless, so the missing close_notify is invisible and the test passes even unpatched.
• It only surfaces on an idle connection. Ending at secure (or right after a write) lets the close_notify ride that write's flush; you have to .end() a macrotask later for it to get stranded in pending_enc_out.

That's exactly the tedious/Prisma pool path: a connection is used, goes idle, then closed. The tests now use both-JS-backed peers + setImmediate(() => sock.end()). They hang without the patch, pass with it, and the handshake
test fails against a naive ungated end().

(Re the .apply/fast-call note: agreed, left as-is since it's a one-shot shutdown.)

Took a close look — the fix is correct and well-scoped (verified the root cause: UnderlyingStream::Js::shutdown() is a no-op so the buffered close_notify was never flushed/ended on the JS path, and the wrapper drives the existing pump to end the underlying stream once drained + handshake-established). The deferred mid-handshake path and the res._owner._secureEstablished gate both check out. A few non-blocking questions before it lands:

• TLS 1.3 not covered. All three tests pin maxVersion: "TLSv1.2", but Deno's default is TLS 1.3. The fix logic is version-agnostic, and the pump re-checks drain state after every write so post-handshake NewSessionTicket traffic should be handled — but it's unverified. Worth confirming the fix works under 1.3 (and ideally one test without the version pin), since real clients may negotiate either. Why is 1.2 pinned — determinism, or a known 1.3 issue?

• Already-ended underlying stream. If the peer has already ended/destroyed the Duplex when our .end() fires, this relies on Node's end()-after-end being a safe no-op. It almost certainly is (guarded by underlyingEnded), but a one-line confirmation would be reassuring.

• Failure mode is a timeout, not an assertion. If the regression returns, the tests hang until the harness timeout rather than failing fast. The comments acknowledge this and it's acceptable for this class of bug, but a per-test deadline would make CI failures clearer.

Thanks.

(on "why 1.2 pinned", it mirrors tedious/MSSQL. TDS 7.x caps at TLS 1.2)

What I pushed in the latest commit:

1. TLS 1.3 coverage: Parameterized all three JS-backed-duplex tests over ["TLSv1.2", "TLSv1.3"], passing maxVersion into both ends.

2. end()-after-destroy guard (ext/node/polyfills/internal_binding/tls_wrap.ts?): Added && !jsStreamOwner.stream.destroyed to the shutdown gate, with a one-line note that end()-after-destroy is a no-op anyway.

3. Per-test deadline: Imported deadline from @std/async/deadline and wrapped each await promise in await deadline(promise, 10_000), so a regression fails fast with a TimeoutError instead of hanging to the harness timeout.

Fix CI regression surfaced by test-tls-net-socket-keepalive[-12].js.

Those tests call tls.connect({ socket }) and socket.connect() after, so the
socket is wrapped via JSStreamSocket (our path), then .end() half-closes
while still reading the server's reply. The earlier version eagerly ended the
underlying stream after the handshake, which raced — and usually preempted —
that read, so the peer's response was lost (deterministic locally, flaky on the
slower CI runner).

Fix: on shutdown, flush the close_notify through the pump but do not end
the underlying stream. The close_notify bytes already give the peer a clean
TLS-level EOF (peer_has_closed()), so ending the transport was redundant and
is what broke half-open peers.

The original hang is still fixed (verified by reverting on the same commit:
unpatched hangs, patched delivers EOF), unit_node::tls_test passes (TLS
1.2/1.3), and the keepalive tests now pass.