Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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_force(&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: post-on_open re-arm overrides pause() called inside open handler

On Windows this re-arm now runs *after* `on_open`, but it's only guarded by `!us_socket_is_closed(s)` — if user code calls `socket.pause()` inside the `open` handler (reachable from JS via `pauseFromJS` → `us_socket_pause`, socket.c:658), it sets `flags.is_paused = 1` and arms WRITABLE-only, then this block immediately force-re-arms READABLE and silently undoes the pause. Before this PR (and still on POSIX) the re-arm happens *before* `on_open` so a pause inside `open` sticks; the guard here sho

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

View check run for this annotation

Claude / Claude Code Review

us_poll_change_force is undefined — Windows build will fail

`us_poll_change_force` is called here but is not declared or defined anywhere in the repository — the only hit repo-wide is this call site. Since this is inside `#ifdef _WIN32`, POSIX builds (which you tested) compile fine, but the Windows build — the platform this PR targets — will fail with an implicit-declaration error / unresolved external. Either add the `_force` variant to `eventing/libuv.c` (skipping the `us_poll_events(p) != events` short-circuit at libuv.c:109), or clear the stale `POLL
Comment thread
claude[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