Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe PR replaces pointer-based SSL spill ownership with per-socket IDs. It stores parked handshake errors in SSL ex_data, removes the relocation callback, updates socket layout checks, and initializes ChangesSSL socket lifecycle
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status: ready for review. Current revision is 879a612 (rebased onto main after #37675 and #31829 landed in the same files; no conflicts).
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it reworks ownership tracking in the native TLS layer (per-loop spill slot + parked handshake reason) and changes struct us_socket_t layout with no new test, a human look is still worthwhile.
Checked: ssl_id_counter starts at 0 via us_calloc, so the first id handed out is 1 and 0 never matches. All four plain-socket init sites set ssl_id = 0; ssl_owns_slot refuses 0. us_poll_resize memcpy's the whole header, so the id survives adoption and dropping us_internal_ssl_socket_relocated is sound. The keyed ssl_clear_parked_reason after ssl_close in on_data reads s->group->loop post-dispatch, which us_internal_ssl_detach already does one frame down, so no new lifetime hazard. The epoll/kqueue _Static_assert (80 == 5×16) matches the layout math.
Extended reasoning...
Overview
Refactor of per-loop TLS scratch ownership in packages/bun-usockets: the ciphertext spill slot and parked handshake reason in loop_ssl_data are re-keyed from raw us_socket_t* to a monotonic uint64_t ssl_id handed out by us_internal_ssl_attach. Adds ssl_id to struct us_socket_t (fills tail padding on epoll/kqueue, +16B on libuv), removes us_internal_ssl_socket_relocated, consolidates release into ssl_spill_free / ssl_clear_parked_reason / ssl_owns_slot, and moves both releases to the top of us_internal_ssl_detach. Five files touched, all in packages/bun-usockets/src/.
Security risks
None identified. The change tightens (not loosens) the condition under which one connection can touch another's per-loop scratch: address-keying could in principle alias across allocator recycling if a teardown site were missed; id-keying cannot. No new user-controlled input reaches the changed code, and no security gates are removed.
Level of scrutiny
High. This is native C in the TLS write/close path — the exact area REVIEW.md flags as the most-blocked category (per-loop shared state, teardown ordering, adoption/relocation). A miskeyed spill would drain one connection's sealed records into another's fd. The change also touches struct layout with a platform-gated static assert (epoll/kqueue only; libuv/Windows grows and has no assert), and ships without a new test on the argument that current behavior is already correct. That justification is reasonable, but per REVIEW.md this is a maintainer call.
Other factors
I traced the specific paths the PR body describes and they check out: us_calloc zero-inits ssl_id_counter; the four plain-socket init sites (us_internal_init_listen_socket, us_internal_init_connect_socket, accept in loop.c, us_socket_from_fd) all set ssl_id = 0; us_poll_resize copies the whole old block so the id survives adoption; ssl_owns_slot guards against 0-matching-0 for plain sockets on the unconditional us_internal_ssl_detach call. The one line that changed shape — ssl_clear_parked_reason(s->group->loop, s) after ssl_close in us_internal_ssl_on_data — now dereferences s->group after JS dispatch, but that's the same pattern us_internal_ssl_detach already uses one frame down in close_raw, so it introduces no new lifetime dependency. The design decision itself (id-keying vs. address-keying, and shipping a source-only invariant refactor without a test) is what needs a maintainer sign-off.
|
Nothing actionable in the review above; the points it traced (counter starts at 1 via On the libuv build there is deliberately no size assert: the poll header there is 24 bytes, so the struct goes from 80 to 96 (the same growth a 4-byte field would cause, given the 16-byte alignment), and nothing reads the size except the |
|
Updated 1:05 PM PT - Aug 12th, 2026
❌ @robobun, your commit f9fe907 has some failures in 🧪 To try this PR locally: bunx bun-pr 37679That installs a local version of the PR into your bun-37679 --bun |
robobun
changed the title
|
Revised in 8eaa718 after a self-review pass. Two things changed:
The PR body also answers why the slot is keyed by an id rather than by |
|
Automated review of 8eaa718 came back with no findings; nothing to change from it. CI for the revised push is running as build 92997 (https://buildkite.com/bun/bun/builds/92997), the previous revision was green across the matrix in build 92636. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it refactors ownership bookkeeping in the usockets TLS layer — changing us_socket_t's layout, moving the parked handshake reason into SSL ex_data, and re-keying the per-loop spill slot — a human look would still be worthwhile.
What was reviewed:
ssl_idinit at all four plain-socket sites and assignment inus_internal_ssl_attach; counter starts fromus_calloc'd 0 so pre-increment never hands out 0, andssl_owns_spillrefuses 0.us_poll_resize(epoll/kqueue and libuv) doesmemcpythe whole header, so the id survives adoption; all references to the removedus_internal_ssl_socket_relocatedare gone.- Every prior
ssl_spill_owner == s/!= ssite now goes throughssl_owns_spillor the== 0free-slot check;ERR_peek_error()returnsuint32_tin BoringSSL andERR_ERROR_STRING_BUF_LENis its own header constant. - The parked reason on ex_data is a packed
uint32_tin avoid*, cleared before the JS dispatch runs, and freed withSSL_free— no separate release needed.
Extended reasoning...
Overview
This PR refactors two pieces of per-loop TLS scratch state in packages/bun-usockets/src/crypto/openssl.c: the ciphertext spill slot and the parked handshake-failure reason. Both previously recorded their owner as a raw us_socket_t *; the spill slot now records a 64-bit per-loop connection id, and the parked reason moves off the loop entirely onto the connection's own SSL* via ex_data. A new uint64_t ssl_id field is added to struct us_socket_t (fits existing padding on epoll/kqueue, pinned at 80 bytes by a new _Static_assert; grows the libuv-build struct from 80 to 96). The now-unnecessary us_internal_ssl_socket_relocated hook and its call in us_socket_adopt are removed. Five files touched: openssl.c, internal.h, context.c, loop.c, socket.c.
Security risks
None identified. This is internal bookkeeping; no user-controlled input flows into the new id assignment or the ex_data slot, and the packed error code is only ever read back and formatted through ERR_error_string_n.
Level of scrutiny
High. This is C in a hot, memory-safety-sensitive path (TLS handshake and ciphertext batching), it changes the layout of a core struct that every socket allocation depends on, and it ships intentionally without a new test (the PR argues, reasonably, that there is no observable behavior on main a test could distinguish). The reasoning in the PR body is thorough and I traced the invariants it relies on, but layout changes to us_socket_t and the design decision to spend 8 header bytes per socket (vs. e.g. keeping the pointer key and auditing teardown sites) are the kind of thing a maintainer should sign off on.
Other factors
- The PR description is slightly out of date relative to the second commit (it describes
ssl_last_fatal_error_ownerbecoming an id and assl_clear_parked_reasonhelper; the final state instead moves the parked reason to SSL ex_data, which is simpler). Not a correctness concern. - No CODEOWNERS entry for
packages/bun-usockets. - CI build #92997 is in flight; the author reports 22 TLS suites pass identically with and without the change under ASAN.
|
No code changes from the review above. Its one note, that the description still described the per-loop reason owner becoming an id, was read against the body as it stood when 8eaa718 was pushed; the body was rewritten right after and now describes the final shape (reason on the SSL via ex_data, ssl_id keying only the spill slot), with the old per-loop fields mentioned only in the description of what main does today. |
…nection id loop_ssl_data holds two pieces of per-loop scratch that one TLS connection owns at a time: the ciphertext spill slot and the parked handshake reason. Both recorded their owner as the us_socket_t address, which only works as long as every teardown path clears the slot before the allocator can hand the address to a new socket, and as long as adoption re-points the owner when it moves a connection to a new block. Give each TLS connection a 64-bit id instead: us_internal_ssl_attach takes it from a per-loop counter and stores it in a new us_socket_t.ssl_id, and both slots record that id. An id is never handed out twice on a loop, so a connection can only ever touch a slot it filled itself, whatever address it has; plain sockets carry id 0, which never matches an occupied slot. The id travels with the header copy in us_poll_resize, so us_internal_ssl_socket_relocated is no longer needed and is removed. us_internal_ssl_detach now releases both slots up front, so every teardown path gives the scratch back in one place. ssl_id takes the place of tail padding on epoll/kqueue; a static assert pins sizeof(struct us_socket_t) at its previous 80 bytes there.
The reason is per-connection state, so store it where this file keeps the rest of that state: as the packed error code in an SSL ex_data slot, the same shape as the inline-reject verdict. ssl_dispatch_parked_reason takes it off the SSL and formats it with ERR_error_string_n at dispatch time, which is a pure function of the code, so the reported string is unchanged. This removes the per-loop reason buffer, its owner field and the clears on every path that used to have to give it back; the spill slot is now the only loop-level state keyed by ssl_id, and the comments describe that.
ssl_id is set by us_internal_ssl_attach like the other ssl_* header fields, so read it only behind s->ssl: us_internal_ssl_detach, the one release path that also runs for plain sockets, now skips them instead of every socket constructor zeroing the field. Every spill owner still has s->ssl set when it reaches the release, so the ordering is unchanged, and the id-0 special case in ssl_owns_spill goes away with the init lines. Also spell out why the spill slot is per loop (it is the bound on ciphertext reported as written but not yet handed to the kernel), which is what makes it need an owner in the first place.
8eaa718 to
879a612
Compare
|
Third revision, 879a612, after another self-review pass over the ex_data version (the automated summaries above have nothing to act on; the walkthrough still describes the
|
There was a problem hiding this comment.
I reviewed this PR and the automated bug hunt found no issues. Because it refactors ownership bookkeeping in the TLS write-batching and handshake-error paths (native C, crypto/openssl.c) and changes struct us_socket_t layout, a maintainer sign-off is still worthwhile.
What was reviewed:
ssl_owns_spillreads ofs->ssl_idare all reached only whens->ssl != NULL(us_internal_ssl_detachmoved the release inside the guard;us_socket_ssl_spill_pendinggates ons->ssl), so uninitializedssl_idon plain sockets is never compared.loop_ssl_dataisus_calloc'd, sossl_id_counterstarts at 0 and the first id handed out is 1;ssl_spill_owner == 0correctly means "free".us_poll_resizeheader copy carriesssl_idacross adoption, so the deletedus_internal_ssl_socket_relocatedhas no remaining callers or purpose.- The parked-error ex_data slot uses no free func and stores a packed
uint32_tas(void*)(uintptr_t), matching the existingus_ssl_inline_reject_err_ex_idxpattern in the same file.
Extended reasoning...
Overview
This PR refactors two pieces of per-loop TLS scratch state in packages/bun-usockets/src/crypto/openssl.c:
- The ciphertext spill slot's owner changes from a raw
us_socket_t *to a monotonic per-loopuint64_tconnection id (ssl_id), assigned inus_internal_ssl_attachand stored in a new field onstruct us_socket_t. - The parked handshake-failure reason moves from a per-loop 256-byte buffer + owner pointer onto the connection's own
SSL *via a new ex_data slot, formatted withERR_error_string_nat dispatch time.
The now-unnecessary us_internal_ssl_socket_relocated (which re-pointed the address-keyed owner after us_poll_resize) is deleted along with its call site in context.c. internal.h gains the ssl_id field plus a _Static_assert pinning sizeof(struct us_socket_t) == 80 on epoll/kqueue.
Security risks
The change is entirely inside the TLS layer — write batching, spill draining, and handshake error reporting. A mistake here could misdirect spilled ciphertext to the wrong fd or misattribute a handshake error to the wrong connection. I traced every read of ssl_id through ssl_owns_spill and confirmed each is gated on s->ssl != NULL, so an uninitialized ssl_id on a plain socket never participates in a comparison. The counter is zero-initialized via us_calloc and pre-incremented, so 0 is never assigned. The ex_data pattern for the parked error is copied directly from the adjacent us_ssl_inline_reject_err_ex_idx slot. I did not find a path where the refactor weakens an existing check.
Level of scrutiny
High. This is native C in crypto/openssl.c — the review guide calls out native memory-ownership changes as the most-blocked category, and this file is on the wire path for every TLS connection. The refactor is conceptually tight and the PR description traces every invariant it depends on, but it is a design change (new struct field, state relocated to SSL ex_data, deletion of a relocation hook) rather than a mechanical fix, and it ships with no new test on the grounds that main has no observable defect to test against. That reasoning is sound, but the combination of struct-layout change + TLS ownership rewrite + no new coverage is exactly the kind of thing a maintainer should confirm.
Other factors
- The PR description claims "the four plain-socket init sites set it to 0", but the final 3-file diff does not add
ssl_id = 0to any init site — that appears to be stale wording from before commit 879a612 movedssl_release_spillinside theif (s->ssl)guard, which made zero-init unnecessary. The code as shipped is consistent; only the description is slightly out of date. - No prior human review on the thread; CodeRabbit and a prior automated pass found nothing actionable.
- Existing TLS test suites (listed in the PR body) exercise all rewritten paths and reportedly pass identically with and without the change under ASAN.
|
Nothing to change from the review above. Its one note (the description mentioning four init sites zeroing CI for 879a612 (build 93254): every lane is green except Windows 2019 x64, where |
|
f9fe907 is an empty commit to re-run CI once after the unrelated Windows crash in build 93254 (see the status comment); nothing in the diff changed, so there is nothing new for the automated reviews to look at. The re-run is build 93292 (https://buildkite.com/bun/bun/builds/93292). |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it refactors ownership bookkeeping in the usockets TLS layer (struct layout change, spill-slot keying, SSL ex_data lifecycle) and deliberately ships without new tests, a human look from someone who owns this subsystem would still be worthwhile.
What was reviewed:
ssl_idcounter:loop_ssl_dataisus_calloc'd so the counter starts at 0, first id is 1, free slot is owner 0 — no attached socket can match a free slot.ssl_release_spillmoving insideif (s->ssl): onlyssl_flush_write_batchfills the slot and it's only reached withs->sslset; every constructor NULLss->ssl, so plain sockets correctly skip.us_poll_resizememcpys the whole header, sossl_idtravels on adoption; the retired old block is freed byus_internal_free_closed_socketswithout calling detach, so no double-release.us_internal_ssl_socket_relocatedhas no remaining references;ERR_error_string_nintoERR_ERROR_STRING_BUF_LENis bounded and NUL-terminated.
Extended reasoning...
Overview
This PR refactors two pieces of per-loop TLS scratch state in packages/bun-usockets: the parked handshake failure reason moves from a per-loop buffer keyed by us_socket_t* onto the connection's own SSL via a new ex_data slot, and the ciphertext spill slot's owner changes from a raw socket pointer to a monotonically-assigned per-loop 64-bit connection id stored in a new us_socket_t.ssl_id field. us_internal_ssl_socket_relocated and its call site in us_socket_adopt are deleted, ssl_release_spill moves inside the if (s->ssl) gate in us_internal_ssl_detach, and a _Static_assert pins sizeof(struct us_socket_t) to 80 on epoll/kqueue. Files touched: crypto/openssl.c, context.c, internal/internal.h.
Security risks
The change is in TLS record delivery ordering and handshake error attribution — a wrong spill-owner match could send one connection's ciphertext to another fd, and a wrong parked-reason attribution could report a stale error. I traced the invariants (counter never hands out 0; us_calloc zeros the slot; us_poll_resize memcpys the id; retired adopted blocks are us_poll_free'd without detach; s->ssl is NULL'd by every constructor and only set by attach) and they hold. The ERR_peek_error() cast to uint32_t matches BoringSSL's return type, and the (void*)(uintptr_t) round-trip through ex_data is lossless. No new untrusted-input parsing.
Level of scrutiny
High. This is hand-written C in the networking/TLS core, where memory-safety bugs are the most-blocked review category per REVIEW.md. The change is a design refactor rather than a bug fix, alters struct layout (grows us_socket_t by 16 bytes on the libuv/Windows build), and makes an architectural choice (id vs SSL* vs per-connection buffer) that the PR body explicitly weighs. That is exactly the kind of decision a maintainer who owns this subsystem should ratify.
Other factors
The PR is source-only by design — the body argues no test can distinguish before/after because every current teardown path already clears both slots on main. REVIEW.md is explicit that every behavioral change ships an automated test in the same PR; whether this qualifies as a no-behavior-change refactor that is exempt is a maintainer call, not an automated one. The author has done thorough A/B verification and CI is green on prior revisions, but the combination of no new tests, struct layout change, and a subtle ordering change in us_internal_ssl_detach puts this outside what I'd approve without a human familiar with the usockets TLS layer signing off.
|
No findings to act on in the review above; the diff is unchanged since 879a612 (f9fe907 is empty). On its one open question, whether a refactor with no observable behavior change ships without a new test: that is deliberately left to the maintainer, and the body sets out both the reasoning and the existing coverage that exercises every rewritten path. Re-run build 93292 has passed 177 of 181 jobs with no failures, including the Windows 2019 x64 lane that hit the unrelated |
2 participants
Problem
mainis complete, so there is nothing to reproduce.Fix
SSL, as the packed error code in a BoringSSL ex_data slot, and is formatted into a string at dispatch time. It dies with the SSL and can only be read back through the SSL that parked it, so the per-loop buffer, its owner field and the clears on teardown go away. The string JS receives is unchanged._Static_assert); on the libuv build it grows from 80 to 96, and nothing outside usockets depends on the size.main, with matching results; temporary local instrumentation showed existing tests exercise the park and dispatch paths, including a client and a server of one failed handshake on the same loop reporting different reasons.Background
struct us_socket_tis its per-socket header, and the caller's ext data is laid out directly after it, which is why the header's size matters.loop_ssl_datais scratch shared by every TLS socket on one event loop (shared BIOs and read buffers), so anything kept there for a single connection has to record which connection it belongs to.wrong version number) the way Node does.us_socket_adopt): a live socket whose ext size changes, such as a WebSocket upgrade over TLS, is copied into a new allocation, so a connection's address can change mid-life and its old address becomes free for reuse.Original description
What does this PR do?
Tightens the ownership bookkeeping of the per-loop TLS scratch in
packages/bun-usockets/src/crypto/openssl.c. No behavior change is intended; this is design debt in how two pieces of state were attributed to a connection.struct loop_ssl_dataheld two things that belong to one connection at a time, both recording their owner as the rawus_socket_t *:ssl_spill_owner/ssl_spill/ssl_spill_len/ssl_spill_off), filled byssl_flush_write_batchwhen a batched write only partially reaches the kernel and drained from the owner's writable event and before its later writes, shutdown and close;ssl_last_fatal_error/ssl_last_fatal_error_owner), written byssl_park_fatal_reasonand consumed byssl_dispatch_parked_reason.Keying by address is only sound while every teardown path clears the slot before the allocator can hand the same address to a new socket, and while adoption re-points the owner whenever
us_poll_resizemoves a live connection to a new block (us_internal_ssl_socket_relocated, called fromus_socket_adopt). I traced those sites and they all hold onmain, which is also why there is nothing to reproduce; but the slots' correctness rested on each of them staying complete, and a missed one would mean a new connection on a recycled address draining a dead connection's spilled records into its own fd, or reporting a dead connection's handshake reason as its own.Change. The two pieces of state get different treatment, because only one of them is loop-level by design:
SSL, where this file already keeps that kind of state (reneg counters, SNI state, the inline-reject verdict).ssl_park_fatal_reasonstores the packedERR_peek_error()code in a new free-func-less ex_data slot, exactly likeus_ssl_inline_reject_err_ex_idx;ssl_dispatch_parked_reasontakes it off the SSL and formats it withERR_error_string_nat dispatch time. That function is a pure function of the packed code, so the string JS receives is unchanged (the longest possible output with the vendored BoringSSL tables is 106 bytes, insideERR_ERROR_STRING_BUF_LEN). The per-loop buffer, its owner field, and the clears on detach, on the inline-reject dispatch and after the fatal-read close all go away: the state dies with the SSL and can only be read back through the SSL it was parked on. (The inline-reject dispatch needs no clear:ssl_trigger_handshakeflips the state toHANDSHAKE_COMPLETEDfirst, after which neither dispatch site runs again.)ssl_flush_write_batchandus_internal_ssl_write, which this PR keeps). A shared slot needs an owner, and the owner is now an explicit connection id:us_internal_ssl_attach, the one place every TLS socket passes through (accept, connect,us_socket_from_fd,us_socket_adopt_tls), assignss->ssl_idfrom a per-loop 64-bit counter, and every match goes throughssl_owns_spill. The id travels with the header copy inus_poll_resize, sous_internal_ssl_socket_relocatedand its call incontext.care deleted. An id is never handed out twice on a loop, so a connection can only drain or release a spill it made itself, whatever address it currently has. Like the otherssl_*header fields,ssl_idis set by attach and only read behinds->ssl;us_internal_ssl_detach, the one release path that also runs for plain sockets (us_internal_socket_close_rawcalls it unconditionally), now skips sockets that never attached instead of every socket constructor having to zero the field. Theloop_ssl_datacomment now says why the slot is per loop, so the owner key is not mistaken for an accident of the current storage.Two alternatives I considered and did not take. Keying the slot on
s->sslinstead of a new id would also survive adoption, but anSSL *is itself a recycled heap address, so it would only remove the relocation dependency and keep the address-reuse one; the id removes both, and lets a stale slot, should a future teardown path ever miss the release, degrade to "batching stays off on this loop" instead of misdirected ciphertext. Giving each connection its own spill buffer (theus_ssl_pending_session_tshape) would remove the owner key entirely, but it changes the policy above (one bounded spill per stalled connection instead of one per loop, and batching staying on for everyone else), which is a behavior change with its own memory and throughput trade-offs; it would be its own PR, and this one keeps the policy as is.Layout.
ssl_idsits wherestruct us_socket_thad tail padding on epoll/kqueue (72 bytes of content rounded to 80 by the 16-byte alignment of the poll header), so the struct and the ext offset behind it are unchanged there; a_Static_assertnext to the existingus_socket_flagsassert pins the size at 80 (checked with anoffsetof/sizeofprobe:ssl_idat 24,sizeof80), and the layout comments ininternal.hthat the field made inaccurate now describe the current layout in one place. On the libuv build the poll header is 24 bytes and the struct goes from 80 to 96, which any added field would cause; nothing outside usockets depends on the size (the Rust side treatsus_socket_tas opaque and reaches ext data throughus_socket_ext).Tests
This PR is source-only on purpose. Every current teardown path clears both slots, so there is no behavior on
maina test can fail against; the change removes what the slots' correctness depended on rather than fixing an observable bug, and a test added here would pass with and without it.To make sure the rewritten code is actually exercised by what exists, I temporarily instrumented
ssl_park_fatal_reasonandssl_dispatch_parked_reason(locally, not in this PR) and counted distinct connections that park and then report their own reason:test/js/node/tls/node-tls-server.test.ts13,node-tls-cert.test.ts11,node-tls-ecdh-curve.test.ts6,node-tls-connect.test.ts5 (including the two early-writeERR_SSL_NO_SUPPORTED_VERSIONS_ENABLEDcases that go through theus_internal_ssl_writepark site),test/js/bun/net/socket.test.ts3 (adoptedupgradeTLSsockets), andfetch-tls-cert.test.ts"fetch applies tls.sigalgs" on the fetch client's own loop. The vendoredtest-tls-alert.js,test-tls-min-max-version.js,test-tls-server-failed-handshake-emits-clienterror.js,test-tls-socket-failed-handshake-emits-error.js,test-tls-junk-server.jsandtest-tls-close-error.jseach have the client and the server of one failed handshake on the same loop reporting different reasons (for exampleUNSUPPORTED_PROTOCOLon the server andTLSV1_ALERT_PROTOCOL_VERSIONon the client), which is the attribution property this PR is about. The spill slot is covered by the deferred spill-close and destroySoon tests innode-tls-server.test.ts, the TLS variants innode-http-backpressure.test.ts/node-http-pinned-write.test.ts, andtls-syscall-fault.test.ts.How did you verify your code works?
Ran the suites above plus the rest of the TLS-related files (
tcp-server,socket-retention,fetch.tls,serve(tls cases),node-tls-upgrade,node-tls-context,tls-connect-socket-churn,tls-syscall-fault,renegotiation,ssl-ctx-cache, hostname verification, half-open, duplex close,test/regression/issue/12117.test.ts, andtest-tls-destroy-whilst-write.js/test-tls-close-event-after-write.js) with the debug (ASAN) build on this branch and on the same tree withpackages/bun-usocketsreset tomain; the results match.test/js/web/websocket/websocket.test.js"should connect many times over https" (300 WebSocket upgrades over TLS, i.e. adoption of live TLS sockets with address churn),websocket-syscall-fault.test.tsandtest/js/node/http2/node-http2.test.jspass on the branch. The failures present in both runs are this container'slocalhostresolving to::1while the tests connect to127.0.0.1, tests that need the public internet, and two tests that sit at the 5 s default timeout under ASAN here with and without the change (the destroySoon test measured 4.6 to 6.6 s onmainand 5.0 to 6.7 s on the branch, delivering every byte in both; reported separately as a test-budget issue); none of them involve the changed code.