Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions packages/bun-usockets/src/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,9 @@
// It's expected that close is called by the caller
}
} else {
#ifndef _WIN32
us_poll_change(&s->p, s->group->loop, LIBUS_SOCKET_READABLE);
#endif
bsd_socket_nodelay(us_poll_fd(&s->p), 1);
us_internal_poll_set_type(&s->p, POLL_TYPE_SOCKET);
us_socket_timeout(s, 0);
Expand All @@ -754,6 +756,28 @@
} else {
us_dispatch_open(s, 1, 0, 0);
}

#ifdef _WIN32
/* Windows (libuv): re-arm for READABLE *after* on_open. uv_poll_start
* submits a fresh AFD poll request; if user code closes the socket
* inside on_open (e.g. `.end()` from a TLS socket's `open` handler,
* which fires pre-handshake when a `handshake` handler is also
* present), the resulting uv_poll_stop + uv_close runs in the same
* poll_cb frame as the just-submitted request and the handle never
* finishes closing — the process hangs. POSIX has no such re-entrancy
* (epoll_ctl(MOD) / kqueue mask change), so the early re-arm above is
* kept to leave the hot path bit-identical there.
*
* last_write_failed: a partial write inside on_open (or the SSL BIO)
* already armed WRITABLE via us_poll_change in us_socket{,_raw}_write;
* preserve it. For TLS, ssl_update_handshake also sets this on
* WANT_READ — that costs one harmless on_writable tick and then the
* regular loop.c writable-disarm clears it. */
if (!us_socket_is_closed(s)) {
us_poll_change(&s->p, s->group->loop,
LIBUS_SOCKET_READABLE | (s->flags.last_write_failed ? LIBUS_SOCKET_WRITABLE : 0));
}

Check failure on line 779 in packages/bun-usockets/src/context.c

View check run for this annotation

Claude / Claude Code Review

Windows TLS connect: WRITABLE busy-loop during handshake (last_write_failed defeats loop.c disarm)

The comment here claims that arming WRITABLE when `last_write_failed` is set "costs one harmless on_writable tick and then the regular loop.c writable-disarm clears it" — but `us_internal_ssl_on_writable` (openssl.c:941) re-runs `ssl_update_handshake`, which re-sets `last_write_failed = 1` on `WANT_READ` *before* loop.c:489 evaluates the disarm gate, so WRITABLE is never disarmed and every Windows TLS client connect spins one core at 100% for the duration of the first handshake RTT. Pre-PR (and
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
#endif
}
}

Expand Down
48 changes: 48 additions & 0 deletions test/js/bun/net/socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -831,3 +831,51 @@ it("reading fd of a TLS listener should not crash", () => {
expect(typeof listener.fd).toBe("number");
expect(listener.fd).toBeGreaterThanOrEqual(0);
});

// Regression: on Windows (libuv), `.end()` from `open` on a TLS socket — which
// fires on TCP-connect (pre-handshake) when a `handshake` handler is also
// present — left a uv_poll_t with a freshly-submitted AFD request racing
// uv_close in the same poll_cb frame. The handle never finished closing →
// loop ref'd → process hung. The fix defers the READABLE re-arm until after
// on_open returns (and skips it if on_open closed the socket).
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
it("TLS .end() inside open (mid-handshake) fires close and doesn't hang", async () => {
using server = Bun.listen({
hostname: "127.0.0.1",
port: 0,
tls,
socket: { data() {}, open() {}, close() {}, error() {} },
});

const events: string[] = [];
const { promise, resolve, reject } = Promise.withResolvers<void>();
await Bun.connect({
hostname: "127.0.0.1",
port: server.port,
tls: { rejectUnauthorized: false },
socket: {
open(s) {
events.push("open");
s.end();
},
// Presence of `handshake` is what makes `open` fire pre-handshake.
handshake(_s, ok, err) {
events.push(`handshake:${ok}:${err?.code ?? "-"}`);
},
close() {
events.push("close");
resolve();
},
data() {},
error(_s, e) {
reject(e);
},
connectError(_s, e) {
reject(e);
},
},
});
await promise;

// Closing mid-handshake surfaces ECONNRESET to `handshake` before `close`.
expect(events).toEqual(["open", "handshake:false:ECONNRESET", "close"]);
});
Loading