Repository navigation
usockets: drain + back off on accept() EMFILE instead of spinning #32705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,34 @@ extern const size_t Bun__lock__size; | |
| #endif | ||
|
|
||
| extern void Bun__internal_ensureDateHeaderTimerIsEnabled(struct us_loop_t *loop); | ||
| extern void Bun__warnAcceptEMFILE(int err); | ||
|
|
||
| /* accept() returned an error that means "out of file descriptors" (per-process | ||
| * or system-wide). The listener poll is level-triggered, so leaving it armed | ||
| * would spin the loop re-failing accept() at full speed. */ | ||
| static int us_internal_is_emfile(int err) { | ||
| #ifdef _WIN32 | ||
| return err == WSAEMFILE || err == WSAENOBUFS; | ||
| #else | ||
| return err == EMFILE || err == ENFILE; | ||
| #endif | ||
| } | ||
|
|
||
| /* Re-arm every listener that was paused on EMFILE. Runs from the backoff | ||
| * deadline (POSIX) or the sweep timer (libuv). Returns how many were resumed. */ | ||
| static int us_internal_resume_emfile_paused_listeners(struct us_loop_t *loop) { | ||
| int resumed = 0; | ||
| for (struct us_socket_group_t *g = loop->data.head; g; g = g->next) { | ||
| for (struct us_listen_socket_t *ls = g->head_listen_sockets; ls; ls = ls->next) { | ||
| if (ls->emfile_paused) { | ||
| ls->emfile_paused = 0; | ||
| us_poll_change((struct us_poll_t *) ls, loop, LIBUS_SOCKET_READABLE); | ||
| resumed++; | ||
| } | ||
| } | ||
| } | ||
| return resumed; | ||
| } | ||
|
|
||
| #ifdef LIBUS_USE_LIBUV | ||
|
|
||
|
|
@@ -70,6 +98,21 @@ void us_internal_disable_sweep_timer(struct us_loop_t *loop) { | |
| } | ||
| } | ||
|
|
||
| /* Windows: no reserve-fd trick (sockets are a separate table from file | ||
| * handles). Pause the listener poll and let the sweep timer re-arm it. */ | ||
| void us_internal_accept_emfile(struct us_listen_socket_t *ls, struct us_loop_t *loop) { | ||
| Bun__warnAcceptEMFILE(LIBUS_ERR); | ||
| if (ls->emfile_paused) return; | ||
| us_poll_change((struct us_poll_t *) ls, loop, 0); | ||
| ls->emfile_paused = 1; | ||
| us_internal_enable_sweep_timer(loop); | ||
| } | ||
|
Comment on lines
+103
to
+109
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) Windows servers that hit a transient WSAEMFILE/WSAENOBUFS now stop accepting for up to 4 seconds, where the base branch resumed as soon as a handle freed. The libuv arm of us_internal_accept_emfile at loop.c:103-109 unpolls the listener and relies on sweep_timer_cb to re-arm it, but that timer fires on the LIBUS_TIMEOUT_GRANULARITY cadence (4s, libusockets.h:100), so queued clients wait 0-4s even after handles are available. Fix: re-arm paused listeners on the same ~200ms backoff as POSIX on every backend, e.g. a dedicated one-shot us_create_timer with LIBUS_ACCEPT_REARM_NS, instead of piggybacking on the 4s sweep timer. [also at: packages/bun-usockets/src/loop.c:108 - On Windows a listener that hits WSAEMFILE/WSAENOBUFS stops accepting for up to 4 seconds, not the ~200 ms the PR describes.] Why this was flaggedOn Windows, bsd_accept_socket fails with WSAENOBUFS or WSAEMFILE (us_internal_is_emfile at loop.c:55-61) during a burst, reached from us_internal_dispatch_ready_poll at loop.c:701-705. The libuv us_internal_accept_emfile at loop.c:103-109 calls us_poll_change(ls, loop, 0) and us_internal_enable_sweep_timer. us_internal_enable_sweep_timer at loop.c:86-92 arms sweep_timer with period LIBUS_TIMEOUT_GRANULARITY * 1000 = 4000 ms (libusockets.h:100), and if sweep_timer_count was already >0 the existing 4s cadence is untouched. The only re-arm is us_internal_accept_rearm_libuv called from sweep_timer_cb at loop.c:551, so the listener stays unpolled until the next sweep tick, anywhere from 0 to 4 s later. The base branch kept the listener polled and accepted the queued connection on the very next loop iteration after a handle freed, so clients that arrived during a short handle shortage now see several seconds of connect latency instead of milliseconds. POSIX uses LIBUS_ACCEPT_REARM_NS (200 ms) at loop.c:169, so this is platform-parity only on paper. Verification: Trigger: on Windows (LIBUS_USE_LIBUV), bsd_accept_socket fails with WSAEMFILE or WSAENOBUFS while the backlog is non-empty. The only path that re-arms a libuv-paused listener is sweep_timer_cb (loop.c:551-553), with period 4000 ms (libusockets.h:100). On base the listener stayed polled and accepted the instant a handle freed; after this change Windows clients wait 0-4 s after recovery. |
||
|
|
||
| static void us_internal_accept_rearm_libuv(struct us_loop_t *loop) { | ||
| int resumed = us_internal_resume_emfile_paused_listeners(loop); | ||
| while (resumed--) us_internal_disable_sweep_timer(loop); | ||
| } | ||
|
|
||
| #else | ||
|
|
||
| #define LIBUS_TIMEOUT_GRANULARITY_NS ((long long) LIBUS_TIMEOUT_GRANULARITY * 1000000000LL) | ||
|
|
@@ -96,12 +139,17 @@ void us_internal_disable_sweep_timer(struct us_loop_t *loop) { | |
| } | ||
|
|
||
| long long us_internal_sweep_timeout_ns(struct us_loop_t *loop) { | ||
| if (loop->data.sweep_next_tick_ns < 0) { | ||
| long long deadline = loop->data.sweep_next_tick_ns; | ||
| long long rearm = loop->data.accept_rearm_next_tick_ns; | ||
| if (rearm >= 0 && (deadline < 0 || rearm < deadline)) { | ||
| deadline = rearm; | ||
| } | ||
| if (deadline < 0) { | ||
| return -1; | ||
| } | ||
| /* Its own reading, deliberately: this bounds the poll so the sweep is not | ||
| * starved, and a caller's older reading would round the deadline up. */ | ||
| long long diff = loop->data.sweep_next_tick_ns - (long long) us_internal_monotonic_ns(); | ||
| long long diff = deadline - (long long) us_internal_monotonic_ns(); | ||
| return diff > 0 ? diff : 0; | ||
| } | ||
|
|
||
|
|
@@ -118,6 +166,67 @@ void us_internal_sweep_if_due(struct us_loop_t *loop) { | |
| us_internal_timer_sweep(loop); | ||
| } | ||
|
|
||
| #define LIBUS_ACCEPT_REARM_NS (200LL * 1000000LL) | ||
|
|
||
| static int us_internal_open_accept_reserve_fd(void) { | ||
| int fd; | ||
| do { | ||
| fd = open("/dev/null", O_RDONLY | O_CLOEXEC); | ||
| } while (fd == -1 && errno == EINTR); | ||
| return fd; | ||
| } | ||
|
|
||
| /* accept() hit EMFILE/ENFILE. First try the libuv uv__emfile_trick: close the | ||
| * reserved fd, accept() as many queued connections as that one slot allows and | ||
| * close each immediately so the peer sees a clean refusal instead of a | ||
| * connection that never answers. Draining the backlog clears the | ||
| * level-triggered readable. If the drain exhausts before the backlog does (or | ||
| * the reserve is spent), fall back to unpolling the listener and scheduling a | ||
| * re-arm via accept_rearm_next_tick_ns so the loop idles instead of spinning. */ | ||
| void us_internal_accept_emfile(struct us_listen_socket_t *ls, struct us_loop_t *loop) { | ||
| int err = LIBUS_ERR; | ||
| Bun__warnAcceptEMFILE(err); | ||
|
|
||
| if (err == EMFILE && loop->data.accept_reserve_fd != -1) { | ||
| close(loop->data.accept_reserve_fd); | ||
| loop->data.accept_reserve_fd = -1; | ||
| struct bsd_addr_t addr; | ||
| LIBUS_SOCKET_DESCRIPTOR fd; | ||
| while ((fd = bsd_accept_socket(us_poll_fd((struct us_poll_t *) ls), &addr)) != LIBUS_SOCKET_ERROR) { | ||
| bsd_close_socket(fd); | ||
| } | ||
|
Comment on lines
+195
to
+197
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) An fd-exhausted server under a connection flood now stalls every existing connection and timer, where the base branch returned to the event loop on each failed accept(). The drain loop at loop.c:195-197 runs inside one dispatch until accept() errors; as long as new connections arrive faster than accept()+close() completes, us_internal_dispatch_ready_poll never returns, so no other poll, JS timer, or response on the fds that are actually held gets serviced. Fix: bound the drain to a fixed number of accept()+close() iterations per dispatch and fall through to the timed pause at loop.c:207-213 when the bound is hit; apply the same bound to any future libuv drain. Why this was flaggedTrigger: a Bun.serve at RLIMIT_NOFILE (an attacker can get it there by holding connections open) receiving a sustained connect rate that exceeds the per-iteration cost of accept4()+close() on the listener; arrives through us_internal_dispatch_ready_poll at loop.c:702-706 into us_internal_accept_emfile. On the base branch the loop body was one failed accept() then return to epoll_pwait2, so other ready polls, Bun timers and the held connections' reads/writes were still dispatched every iteration (the spin cost CPU but not progress). After this change loop.c:195-197 is Verification: packages/bun-usockets/src/loop.c:195-197 is |
||
| int drain_err = LIBUS_ERR; | ||
| loop->data.accept_reserve_fd = us_internal_open_accept_reserve_fd(); | ||
| if (!us_internal_is_emfile(drain_err)) { | ||
| /* Backlog drained to EAGAIN (or some other terminal error); the | ||
| * level-triggered readable is clear, no backoff needed. */ | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (ls->emfile_paused) return; | ||
| us_poll_change((struct us_poll_t *) ls, loop, 0); | ||
| ls->emfile_paused = 1; | ||
| long long deadline = (long long) us_internal_monotonic_ns() + LIBUS_ACCEPT_REARM_NS; | ||
| if (loop->data.accept_rearm_next_tick_ns < 0 || deadline < loop->data.accept_rearm_next_tick_ns) { | ||
| loop->data.accept_rearm_next_tick_ns = deadline; | ||
|
Comment on lines
+206
to
+212
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 The timed backoff that actually stops the spin when the reserve-fd trick cannot help, and the whole Windows/libuv path, ship with no test, so a break there merges green. The new test only reaches the drain branch at loop.c:190-203 (reserve fd still held, EMFILE); the unpoll + accept_rearm_next_tick_ns re-arm at loop.c:206-212 and us_internal_accept_emfile at loop.c:103-109 never run under test. Fix: add a case that forces the fallback, e.g. socketFaultInjection.set({ syscall: "accept", action: "errno", errno: , repeat: -1 }) from bun:internal-for-testing, assert low CPU while armed and that a client connects after fault.clear(), and run it on Windows too (no ulimit needed). [also at: packages/bun-usockets/src/loop.c:213 - nit: The timed-backoff fallback and its re-arm path are not exercised by any test, so a regression there would land silently. serve-listen-emfile.test.ts only reaches the reserve-fd drain (loop.c:190-204), which returns before loop.c:207; the pause at loop.c:208-213, us_internal_accept_rearm_if_due, and the libuv sweep-timer branch could be deleted and the suite still passes.; test/js/bun/http/serve-listen-emfile.test.ts:97 - Maintainers get no test for the pause/re-arm half of this fix, so a regression there (listener never resumed) merges green.] Why this was flaggedtest/js/bun/http/serve-listen-emfile.test.ts:14 is skipIf(!isPosix) and exhausts fds with openSync while the loop's reserve fd is still open, so in the child the first EMFILE takes the drain branch at loop.c:190-203, bsd_accept_socket succeeds once, and drain_err is EAGAIN, returning before loop.c:206. Nothing exercises us_poll_change(ls, loop, 0) at loop.c:208, the deadline at loop.c:210-213, us_internal_accept_rearm_if_due at loop.c:216-228, or the libuv arm at loop.c:103-114 and sweep_timer_cb at loop.c:550-553. Deleting the rearm call at epoll_kqueue.c:467/560 leaves every paused listener dead forever and no test fails. On base there is no such code; after merging, a regression in the fallback silently turns a spinning server into one that never accepts again (ENFILE, reserve open failure, or any Windows WSAEMFILE). Verification: In test/js/bun/http/serve-listen-emfile.test.ts:14 ( |
||
| } | ||
| } | ||
|
|
||
| void us_internal_accept_rearm_if_due(struct us_loop_t *loop) { | ||
| if (loop->data.accept_rearm_next_tick_ns < 0) { | ||
| return; | ||
| } | ||
| if ((long long) us_internal_monotonic_ns() < loop->data.accept_rearm_next_tick_ns) { | ||
| return; | ||
| } | ||
| loop->data.accept_rearm_next_tick_ns = -1; | ||
| if (loop->data.accept_reserve_fd == -1) { | ||
| loop->data.accept_reserve_fd = us_internal_open_accept_reserve_fd(); | ||
| } | ||
| us_internal_resume_emfile_paused_listeners(loop); | ||
|
Comment on lines
+223
to
+227
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) A server whose listener was paused on EMFILE can silently stop accepting forever if the re-arm's epoll/kqueue registration fails, whereas the base branch never unregisters the listener. us_internal_accept_rearm_if_due at loop.c:223 clears accept_rearm_next_tick_ns and loop.c:70-71 clears emfile_paused before us_poll_change, whose return code is discarded. On kqueue the pause deleted the EVFILT_READ knote (kqueue_change EV_DELETE, epoll_kqueue.c:629), so the re-arm is a fresh EV_ADD that can fail with ENOMEM under exactly the resource pressure that caused the pause. Fix: check the us_poll_change rc on every re-arm site (POSIX loop.c:71 and the libuv arm loop.c:112) and, on failure, keep emfile_paused set and re-schedule the deadline so the listener is retried instead of abandoned. Why this was flaggedTrigger: a POSIX listener enters the pause path at loop.c:211-218 (reserve fd absent or ENFILE), then 200 ms later us_internal_accept_rearm_if_due runs from us_loop_run_bun_tick (epoll_kqueue.c:560). It sets accept_rearm_next_tick_ns = -1 at loop.c:223, then us_internal_resume_emfile_paused_listeners sets ls->emfile_paused = 0 at loop.c:70 and calls us_poll_change at loop.c:71, ignoring its return. On kqueue the pause ran kqueue_change(old=READABLE,new=0,keep=0) which is EV_DELETE (epoll_kqueue.c:629), so the re-arm is a new EV_ADD that kevent can reject with ENOMEM. After the failure no state records the listener as paused and no deadline is pending, so nothing ever re-arms it: the server keeps running, the port stays bound, and every new connection sits in the backlog until it times out, with no error. The base branch never removes the listener's read interest, so it cannot reach this state. Remedy: on a non-zero rc keep emfile_paused and re-schedule the deadline. Verification: The re-arm clears accept_rearm_next_tick_ns = -1 first (loop.c:223), then sets ls->emfile_paused = 0 (loop.c:70) and calls us_poll_change(READABLE) (loop.c:71) discarding the result. If the EV_ADD fails, nothing records it, so the listener is never re-registered and silently accepts nothing until close. Base branch never deletes the listener knote, so this is a new failure mode. |
||
| } | ||
|
|
||
| #endif | ||
|
|
||
|
|
||
|
|
@@ -129,6 +238,8 @@ int us_internal_loop_data_init(struct us_loop_t *loop, void (*wakeup_cb)(struct | |
| loop->data.sweep_timer = us_create_timer(loop, 1, 0); | ||
| #else | ||
| loop->data.sweep_next_tick_ns = -1; | ||
| loop->data.accept_rearm_next_tick_ns = -1; | ||
| loop->data.accept_reserve_fd = us_internal_open_accept_reserve_fd(); | ||
| #endif | ||
| loop->data.sweep_timer_count = 0; | ||
| loop->data.recv_buf = us_malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2); | ||
|
|
@@ -167,6 +278,11 @@ void us_internal_loop_data_free(struct us_loop_t *loop) { | |
| #ifdef LIBUS_USE_LIBUV | ||
| us_timer_close(loop->data.sweep_timer, 0); | ||
| if (loop->data.quic_timer) us_timer_close(loop->data.quic_timer, 0); | ||
| #else | ||
| if (loop->data.accept_reserve_fd != -1) { | ||
| close(loop->data.accept_reserve_fd); | ||
| loop->data.accept_reserve_fd = -1; | ||
| } | ||
| #endif | ||
| us_internal_async_close(loop->data.wakeup_async); | ||
| } | ||
|
|
@@ -432,6 +548,7 @@ void us_internal_free_closed_sockets(struct us_loop_t *loop) { | |
|
|
||
| #ifdef LIBUS_USE_LIBUV | ||
| void sweep_timer_cb(struct us_internal_callback_t *cb) { | ||
| us_internal_accept_rearm_libuv(cb->loop); | ||
| us_internal_timer_sweep(cb->loop); | ||
| } | ||
| #endif | ||
|
|
@@ -519,13 +636,7 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in | |
| struct bsd_addr_t addr; | ||
|
|
||
| LIBUS_SOCKET_DESCRIPTOR client_fd = bsd_accept_socket(us_poll_fd(p), &addr); | ||
| if (client_fd == LIBUS_SOCKET_ERROR) { | ||
| /* Todo: start timer here */ | ||
|
|
||
| } else { | ||
|
|
||
| /* Todo: stop timer if any */ | ||
|
|
||
| if (client_fd != LIBUS_SOCKET_ERROR) { | ||
| do { | ||
| struct us_poll_t *accepted_p = us_create_poll(loop, 0, sizeof(struct us_socket_t) - sizeof(struct us_poll_t) + listen_socket->socket_ext_size); | ||
| us_poll_init(accepted_p, client_fd, POLL_TYPE_SOCKET); | ||
|
|
@@ -587,6 +698,14 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in | |
|
|
||
| } while ((client_fd = bsd_accept_socket(us_poll_fd(p), &addr)) != LIBUS_SOCKET_ERROR); | ||
| } | ||
| /* EAGAIN is normal (backlog drained). EMFILE/ENFILE with a | ||
| * connection still pending would spin the level-triggered | ||
| * poll; drain it with the reserve fd and/or back off. */ | ||
| if (client_fd == LIBUS_SOCKET_ERROR | ||
| && !us_socket_is_closed(&listen_socket->s) | ||
| && us_internal_is_emfile(LIBUS_ERR)) { | ||
| us_internal_accept_emfile(listen_socket, loop); | ||
| } | ||
| } | ||
| break; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟣 pre-existing, not blocking: Linux servers whose accept() fails with ENOBUFS or ENOMEM still peg a core, the same spin this change claims to remove for EMFILE. us_internal_is_emfile at loop.c:59 only matches EMFILE and ENFILE, so the gate at loop.c:705 sends those errors back to the level-triggered poll with the connection still queued, and the Windows arm at loop.c:57 does include WSAENOBUFS, so the two platforms disagree on the same error. Fix: make the backoff cover every accept() errno that leaves the pending connection in the queue (ENOBUFS, ENOMEM alongside EMFILE/ENFILE), routing them to the pause/re-arm path rather than only the two names, so the spin is fixed as a class rather than for one errno.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Trigger: accept4() on Linux returns ENOBUFS or ENOMEM (socket buffer / kernel memory exhaustion, documented in accept(2)) while a connection is pending; reached from us_internal_dispatch_ready_poll loop.c:703-706 on every Bun.serve listener. The new predicate us_internal_is_emfile at loop.c:59 returns 0 for those errnos, so us_internal_accept_emfile is never called, the listener keeps EPOLLIN level-triggered readable, and epoll_pwait2 returns immediately on every tick exactly as the PR's strace shows for EMFILE. The base branch spun on every accept error too, but this PR's purpose is to end that spin and it adds a per-errno gate that leaves siblings unhandled; the Windows branch at loop.c:57 treats WSAENOBUFS as a backoff case, so the omission is a platform-parity gap in the same function. Population: memory-constrained containers under connection bursts. Remedy: extend the predicate to ENOBUFS/ENOMEM (or invert it to 'not EAGAIN/EWOULDBLOCK/ECONNABORTED/EINTR') so any persistent accept failure backs off.
Verification: On Linux accept4() fails with ENOMEM while a connection is still queued in the backlog. packages/bun-usockets/src/loop.c:56-62
us_internal_is_emfilereturns true only forerr == EMFILE || err == ENFILE, and the only backoff gate is loop.c:703-707. For ENOMEM/ENOBUFS no drain happens and epoll_pwait2 returns immediately every tick. The base branch already spins on these errnos by the same route.