Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
40 changes: 13 additions & 27 deletions packages/bun-usockets/src/bsd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1617,10 +1617,16 @@ LIBUS_SOCKET_DESCRIPTOR bsd_create_listen_socket_unix(const char *path, size_t l
}


/* Receive-path options every UDP socket needs, whether freshly created or
* adopted from an existing fd: destination-address and TOS reporting for
* recvmmsg, Windows ICMP-reset suppression, and Linux IP_RECVERR (opt-in). */
static void bsd_apply_udp_recv_options(LIBUS_SOCKET_DESCRIPTOR fd, int family, int options) {
/* Receive-path options of a UDP socket we create: destination-address and TOS
* reporting for recvmmsg, and Windows ICMP-reset suppression.
*
* Linux IP_RECVERR is deliberately not set, as in libuv (Node never passes
* UV_UDP_LINUX_RECVERR). With it the kernel records every ICMP error on the
* socket, connected or not, and fails the next recv or send with it whichever
* peer that is for: one client that closes its port ends a reply server.
* Without it only a connected socket gets a (hard) ICMP error, as the pending
* error that the next recvmmsg fails with. */
static void bsd_apply_udp_recv_options(LIBUS_SOCKET_DESCRIPTOR fd) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Linux users of an unconnected Bun.udpSocket no longer get an ECONNREFUSED error callback for a datagram sent to a closed port, a feature released in 1.3.12. bsd.c:1629 sets IP_RECVERR on no socket, so the kernel drops ICMP port-unreachable for unconnected sockets and the handler never runs; on the base branch it ran once per refused datagram. This follows from the stated purpose and the PR flags it for a maintainer. Fix: the call is the maintainer's; within the purpose, arm IP_RECVERR only for a Bun.udpSocket that registers an error handler (never node:dgram or node:quic), accepting the poisoned next send on those sockets.

Why this was flagged

A Linux program calls Bun.udpSocket({ socket: { error(...) {} } }) without connect and sends to a port nothing listens on. The kernel's __udp4_lib_err only records an ICMP error on a socket without IP_RECVERR when the socket is connected and the error is hard, so the unconnected socket never raises EPOLLERR. packages/bun-usockets/src/bsd.c:1629 (bsd_apply_udp_recv_options) no longer sets IP_RECVERR or IPV6_RECVERR, so src/runtime/socket/udp_socket.rs:98 on_recv_error is never reached for that socket. On the base branch us_create_udp_socket set the option for every Rust-created socket, loop.c drained MSG_ERRQUEUE and called on_recv_error for each refused datagram, and the Bun.udpSocket error handler received ECONNREFUSED (the base test at test/js/bun/udp/udp_socket_recv_flags.test.ts asserted exactly that; this PR replaces it with an assertion that errors stay []). The user gets silence instead of a report and no error, warning or type change tells them. The PR's Downsides section names this cost and asks for a maintainer's decision.

Verification: The PR description's "Downsides" section states "On Linux an unconnected Bun.udpSocket no longer gets ECONNREFUSED in error for a closed port". At packages/bun-usockets/src/bsd.c:1629 the IP_RECVERR setsockopt block is deleted. The PR deletes the existing test test/js/bun/udp/udp_socket_recv_flags.test.ts and replaces it with one asserting errors: [] for an unconnected sender.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the intended trade and the maintainer's call. The PR body names it under Downsides. Arming IP_RECVERR only for a Bun.udpSocket with an error handler does not avoid the cost: the kernel records the ICMP error on the socket either way, so the next send to any peer fails with it and a reply server still breaks on the first client that closes its port. libuv and Node do not set the option, and the connected-socket report stays.

/* We need destination address for udp packets in both ipv6 and ipv4 */

/* On FreeBSD this option seems to be called like so */
Expand Down Expand Up @@ -1659,31 +1665,11 @@ static void bsd_apply_udp_recv_options(LIBUS_SOCKET_DESCRIPTOR fd, int family, i
#endif
}
#endif

#if defined(__linux__)
/* IP_RECVERR/IPV6_RECVERR queues ICMP errors on the socket's error queue
* for on_recv_error to drain. libuv gates this on UV_UDP_LINUX_RECVERR
* (Node's dgram never passes it). Opt-in only: on a shared unconnected
* socket (the HTTP/3 fetch client) it also makes a queued ICMP fail the
* next send to a different, live peer. */
if (options & LIBUS_UDP_LINUX_RECVERR) {
#ifdef IP_RECVERR
setsockopt(fd, IPPROTO_IP, IP_RECVERR, &enabled, sizeof(enabled));
#endif
#ifdef IPV6_RECVERR
if (family == AF_INET6) {
setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &enabled, sizeof(enabled));
}
#endif
}
#else
(void) options;
#endif
}

/* Prepares an externally created UDP fd for adoption, matching uv_udp_open:
* nonblock + SO_REUSEADDR only. Recv-path options (IP_RECVERR, PKTINFO,
* SIO_UDP_CONNRESET) are NOT applied — mutating the caller's fd is observable. */
* nonblock + SO_REUSEADDR only. Recv-path options (PKTINFO, SIO_UDP_CONNRESET)
* are NOT applied — mutating the caller's fd is observable. */
int bsd_prepare_adopted_udp_socket(LIBUS_SOCKET_DESCRIPTOR fd) {
/* Refuse to adopt anything that isn't a datagram socket. */
int sock_type = 0;
Expand Down Expand Up @@ -1813,7 +1799,7 @@ LIBUS_SOCKET_DESCRIPTOR bsd_create_udp_socket(const char *host, int port, int op
}
#endif

bsd_apply_udp_recv_options(listenFd, listenAddr->ai_family, options);
bsd_apply_udp_recv_options(listenFd);

/* We bind here as well */
if (bind(listenFd, listenAddr->ai_addr, (socklen_t) listenAddr->ai_addrlen)) {
Expand Down
8 changes: 4 additions & 4 deletions packages/bun-usockets/src/internal/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,10 @@ struct us_udp_socket_t {
void (*on_data)(struct us_udp_socket_t *, void *, int);
void (*on_drain)(struct us_udp_socket_t *);
void (*on_close)(struct us_udp_socket_t *);
/* Called for a receive-path error. is_errqueue == 1 when the errno came
* from Linux's MSG_ERRQUEUE (an ICMP report about an earlier send), 0 when
* recvmmsg itself failed. The socket is NOT closed — caller decides. */
void (*on_recv_error)(struct us_udp_socket_t *, int err, int is_errqueue);
/* Called with the errno of a recvmmsg that failed. For a connected socket
* that is also how an ICMP error arrives. The socket is NOT closed —
* caller decides. */
void (*on_recv_error)(struct us_udp_socket_t *, int err);
void *user;
struct us_loop_t *loop;
/* An UDP socket can only ever be bound to one single port regardless of how
Expand Down
9 changes: 2 additions & 7 deletions packages/bun-usockets/src/libusockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,6 @@ enum {
* Safe for HTTP/TLS where the client always sends first; do not use for protocols where
* the server sends the first bytes. */
LIBUS_LISTEN_DEFER_ACCEPT = 64,
/* Enable IP_RECVERR on a UDP socket so ICMP errors land on the error
* queue for on_recv_error to drain. Off by default: on a shared
* unconnected socket it also makes the next send fail for a datagram
* bound to a different, live peer. */
LIBUS_UDP_LINUX_RECVERR = 128,
/* A socket adopted by us_socket_from_fd, or accepted by a listener created with this option,
* is registered as if us_socket_pause had been called on it, so no extra poll change is needed
* per connection (node:net's pauseOnConnect; cluster adopts every connection this way). Not
Expand Down Expand Up @@ -219,7 +214,7 @@ struct us_udp_packet_buffer_t *us_create_udp_packet_buffer();

//struct us_udp_socket_t *us_create_udp_socket(us_loop_r loop, void (*data_cb)(struct us_udp_socket_t *, struct us_udp_packet_buffer_t *, int), void (*drain_cb)(struct us_udp_socket_t *), char *host, unsigned short port);

struct us_udp_socket_t *us_create_udp_socket(us_loop_r loop, void (*data_cb)(struct us_udp_socket_t *, void *, int), void (*drain_cb)(struct us_udp_socket_t *), void (*close_cb)(struct us_udp_socket_t *), void (*recv_error_cb)(struct us_udp_socket_t *, int, int), const char *host, unsigned short port, int flags, int *err, void *user);
struct us_udp_socket_t *us_create_udp_socket(us_loop_r loop, void (*data_cb)(struct us_udp_socket_t *, void *, int), void (*drain_cb)(struct us_udp_socket_t *), void (*close_cb)(struct us_udp_socket_t *), void (*recv_error_cb)(struct us_udp_socket_t *, int), const char *host, unsigned short port, int flags, int *err, void *user);


void us_udp_socket_close(struct us_udp_socket_t *s);
Expand All @@ -237,7 +232,7 @@ LIBUS_SOCKET_DESCRIPTOR us_udp_socket_fd(struct us_udp_socket_t *s);
/* Adopts an already created (and usually already bound) UDP socket descriptor
* instead of creating a new one. The fd is made non-blocking and the standard
* receive-path options are applied. Returns null with *err set on failure. */
struct us_udp_socket_t *us_create_udp_socket_from_fd(us_loop_r loop, void (*data_cb)(struct us_udp_socket_t *, void *, int), void (*drain_cb)(struct us_udp_socket_t *), void (*close_cb)(struct us_udp_socket_t *), void (*recv_error_cb)(struct us_udp_socket_t *, int, int), LIBUS_SOCKET_DESCRIPTOR fd, int shared, int *err, void *user);
struct us_udp_socket_t *us_create_udp_socket_from_fd(us_loop_r loop, void (*data_cb)(struct us_udp_socket_t *, void *, int), void (*drain_cb)(struct us_udp_socket_t *), void (*close_cb)(struct us_udp_socket_t *), void (*recv_error_cb)(struct us_udp_socket_t *, int), LIBUS_SOCKET_DESCRIPTOR fd, int shared, int *err, void *user);

/* This one is ugly, should be ext! not user */
void *us_udp_socket_user(struct us_udp_socket_t *s);
Expand Down
107 changes: 44 additions & 63 deletions packages/bun-usockets/src/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@
#include <sys/ioctl.h>
#include <unistd.h>
#endif
#ifdef __linux__
#include <netinet/in.h>
#include <linux/errqueue.h>
#endif

#if __has_include("wtf/Platform.h")
#include "wtf/Platform.h"
Expand Down Expand Up @@ -995,54 +991,23 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
}

#if defined(__linux__)
/* On Linux with IP_RECVERR, EPOLLERR fires when an ICMP error
* (port unreachable, host unreachable, TTL exceeded, ...) is
* queued on the socket's error queue. For an *unconnected* UDP
* socket regular recvmmsg does NOT dequeue these — only
* recvmsg(MSG_ERRQUEUE) does — so EPOLLERR stays level-triggered
* until we drain it explicitly. Do that here, surfacing each
* errno via on_recv_error; the socket stays open. On other
* platforms only a *poll* error (kqueue EV_ERROR, Windows) is
* still fatal below; a plain recv failure is reported the same
/* EPOLLERR on a UDP socket means that the socket has a pending
* error (the ICMP error of a connected socket), not that it is
* broken. Run the receive path for it too: recvmmsg fails with
* that error and clears it, on_recv_error reports it, and the
* socket stays open. libuv folds a bare EPOLLERR into EPOLLIN for
* the same reason (uv__io_poll, deps/uv/src/unix/linux.c). On
* other platforms only a *poll* error (kqueue EV_ERROR, Windows)
* is still fatal below; a plain recv failure is reported the same
* way it is here, not treated as fatal. */
int recv_error_surfaced = 0;
const int poll_error = error;
int recv_would_block_only = 0;
if (error) {
struct msghdr eh; char ectrl[512]; char ebuf[1];
struct iovec eiov = { ebuf, sizeof(ebuf) };
while (!u->closed) {
memset(&eh, 0, sizeof(eh));
eh.msg_iov = &eiov; eh.msg_iovlen = 1;
eh.msg_control = ectrl; eh.msg_controllen = sizeof(ectrl);
if (recvmsg(us_poll_fd(p), &eh, MSG_ERRQUEUE) < 0) break;
recv_error_surfaced = 1;
if (u->on_recv_error) {
/* The queued ICMP error is in sock_extended_err,
* not errno. */
int ee = 0;
for (struct cmsghdr *cm = CMSG_FIRSTHDR(&eh); cm; cm = CMSG_NXTHDR(&eh, cm)) {
if ((cm->cmsg_level == IPPROTO_IP && cm->cmsg_type == IP_RECVERR) ||
(cm->cmsg_level == IPPROTO_IPV6 && cm->cmsg_type == IPV6_RECVERR)) {
ee = ((struct sock_extended_err *) CMSG_DATA(cm))->ee_errno;
break;
}
}
u->on_recv_error(u, ee ? ee : ECONNREFUSED, 1);
}
}
}

/* An EPOLLERR with no EPOLLIN and an empty error queue still has to
* run the receive path: that is how a pending sk_err — an ICMP for
* a socket that never enabled IP_RECVERR, e.g. one adopted from an
* external descriptor by bsd_udp_open() and then connected — is
* reaped by recvmmsg and reported instead of the socket being
* closed below. libuv folds a bare EPOLLERR into EPOLLIN for the
* same reason (uv__io_poll, deps/uv/src/unix/linux.c). */
const int run_recv =
(events & LIBUS_SOCKET_READABLE) || (error && !recv_error_surfaced);
const int run_recv = (events & LIBUS_SOCKET_READABLE) || error;
#else
const int run_recv = events & LIBUS_SOCKET_READABLE;
#endif
#if !defined(_WIN32)
int recv_err = 0;
#endif
if (run_recv && !u->closed) {

Expand All @@ -1061,18 +1026,14 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
error = 1;
#else
/* A failing recvmsg is a socket *condition* to
* report, not a fatal event. On the BSDs, which
* have no error queue, this is the ONLY way the
* kernel delivers a connected socket's ICMP
* error (so_error): Node reports it as
* `recvmsg <code>` and keeps the socket open.
* Only a genuine poll error still closes below. */
int recv_err = errno;
* report (after this loop), not a fatal event.
* It is how the kernel delivers a connected
* socket's ICMP error (so_error): Node reports
* it as `recvmsg <code>` and keeps the socket
* open. Only a genuine poll error still closes
* below. */
if (u->on_recv_error) {
#if defined(__linux__)
recv_error_surfaced = 1;
#endif
u->on_recv_error(u, recv_err, 0);
recv_err = errno;
} else {
/* No error handler (QUIC creates its UDP
* sockets with recv_error_cb = NULL): the
Expand Down Expand Up @@ -1100,6 +1061,26 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
} while (!u->closed);
}

#if defined(__linux__)
/* We never set IP_RECVERR, but the creator of an adopted
* descriptor can. The kernel then also queues every ICMP error,
* keeps EPOLLERR raised while one is queued, and only
* recvmsg(MSG_ERRQUEUE) takes an entry off. Drop the entries: the
* pending error that recvmmsg took is what gets reported. After
* recvmmsg, because taking the last entry clears the pending
* error. Before on_recv_error, because the error of a datagram
* that the handler sends belongs to the next event. */
if (poll_error && !u->closed) {
struct msghdr eh = {0};
while (recvmsg(us_poll_fd(p), &eh, MSG_ERRQUEUE) >= 0) {}
}
#endif
#if !defined(_WIN32)
if (recv_err) {
u->on_recv_error(u, recv_err);
}
#endif

if (events & LIBUS_SOCKET_WRITABLE && !u->closed) {
/* Clear WRITABLE before on_drain so a callback that re-arms it
* (e.g. QUIC packets_out hitting EAGAIN) keeps the re-arm. We
Expand All @@ -1117,11 +1098,11 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
#if defined(__linux__)
/* Only close on EPOLLERR if we didn't surface the real errno
* via recvmmsg + on_recv_error above AND recv wasn't just
* EAGAIN (which means the error queue is already drained,
* leaving a residual EPOLLERR). Otherwise the socket stays
* open so the user can keep sending/receiving after a
* EAGAIN (a send already took the pending error, or only
* error-queue entries raised EPOLLERR). Otherwise the socket
* stays open so the user can keep sending/receiving after a
* transient ICMP error. */
if (error && !recv_error_surfaced && !recv_would_block_only && !u->closed) {
if (error && !recv_err && !recv_would_block_only && !u->closed) {
us_udp_socket_close(u);
}
#else
Expand Down
8 changes: 2 additions & 6 deletions packages/bun-usockets/src/udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ struct us_udp_socket_t *us_create_udp_socket_from_fd(
void (*data_cb)(struct us_udp_socket_t *, void *, int),
void (*drain_cb)(struct us_udp_socket_t *),
void (*close_cb)(struct us_udp_socket_t *),
void (*recv_error_cb)(struct us_udp_socket_t *, int, int),
void (*recv_error_cb)(struct us_udp_socket_t *, int),
LIBUS_SOCKET_DESCRIPTOR fd,
int shared,
int *err,
Expand Down Expand Up @@ -225,18 +225,14 @@ struct us_udp_socket_t *us_create_udp_socket(
void (*data_cb)(struct us_udp_socket_t *, void *, int),
void (*drain_cb)(struct us_udp_socket_t *),
void (*close_cb)(struct us_udp_socket_t *),
void (*recv_error_cb)(struct us_udp_socket_t *, int, int),
void (*recv_error_cb)(struct us_udp_socket_t *, int),
const char *host,
unsigned short port,
int flags,
int *err,
void *user
) {

/* IP_RECVERR is only useful when there is an on_recv_error handler to
* drain the error queue; without one it only poisons subsequent sends. */
if (recv_error_cb) flags |= LIBUS_UDP_LINUX_RECVERR;
else flags &= ~LIBUS_UDP_LINUX_RECVERR;
LIBUS_SOCKET_DESCRIPTOR fd = bsd_create_udp_socket(host, port, flags, err);
if (fd == LIBUS_SOCKET_ERROR) {
return 0;
Expand Down
6 changes: 0 additions & 6 deletions src/js/node/dgram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -758,12 +758,6 @@ function startBunSocket(self, state, createOptions, sharedHandle?) {
},
error: error => {
if (error?.syscall === "recv") {
// Drop errqueue-origin ICMP errors on unconnected sockets like
// Node (which never enables IP_RECVERR); always emit real
// recvmmsg failures — the errno namespaces overlap.
if (error.errqueue === true && state.connectState !== CONNECT_STATE_CONNECTED) {
return;
}
// Node reports receive-path failures as `recvmsg` errors; keep
// the native error so its code stays platform-correct.
error.syscall = "recvmsg";
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/node/quic/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ extern "C" fn on_drain(socket: *mut uws::udp::Socket) {
this.schedule_process();
}
extern "C" fn on_close(_socket: *mut uws::udp::Socket) {}
extern "C" fn on_recv_error(_socket: *mut uws::udp::Socket, _errno: c_int, _is_errqueue: c_int) {}
extern "C" fn on_recv_error(_socket: *mut uws::udp::Socket, _errno: c_int) {}

thread_local! {
/// A server may advertise another local endpoint's address as its
Expand Down
19 changes: 4 additions & 15 deletions src/runtime/socket/udp_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,29 +95,18 @@ extern "C" fn on_close(socket: *mut uws::udp::Socket) {
}
}

extern "C" fn on_recv_error(socket: *mut uws::udp::Socket, errno: c_int, is_errqueue: c_int) {
// Reached on every POSIX platform. `is_errqueue` distinguishes an ICMP
// errno drained from Linux's MSG_ERRQUEUE from a real recvmmsg failure —
// which on the BSDs (no error queue) is also how a connected socket's
// ICMP error (so_error) arrives. node:dgram must drop only the former on
// unconnected sockets, and the errno namespaces overlap.
extern "C" fn on_recv_error(socket: *mut uws::udp::Socket, errno: c_int) {
// recvmmsg failed. A connected socket's ICMP error (so_error) arrives this way too.
let this: &UDPSocket = UDPSocket::from_uws(socket);
let _context = this.enter_owners_context();
let sys_err = bun_sys::Error::from_code_int(errno, bun_sys::Tag::recv);
let global_this = this.global_this.get();
// A callback earlier in the same poll dispatch may have left a
// TerminationException pending: loop.c's Linux errqueue drain calls this
// once per queued ICMP in a `while (!u->closed)` loop, and its recv
// do-while can reach this right after an `on_data` iteration's callback.
// `to_js` below and the error handler both enter JS, which trips
// executeCallImpl's assertNoException(). Mirrors on_data / on_drain.
// An `on_data` callback earlier in this poll dispatch may have left a TerminationException
// pending, and entering JS with one trips assertNoException(). Mirrors on_data / on_drain.
Comment thread
robobun marked this conversation as resolved.
Resolved
if global_this.has_exception() {
return;
}
let err_value = sys_err.to_js(global_this);
if is_errqueue != 0 {
err_value.put(global_this, b"errqueue", JSValue::TRUE);
}
this.call_error_handler(JSValue::ZERO, err_value);
Comment thread
robobun marked this conversation as resolved.
Resolved
}

Expand Down
Loading
Loading