Skip to content
Closed
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
39 changes: 24 additions & 15 deletions packages/bun-usockets/src/bsd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1507,21 +1507,6 @@ LIBUS_SOCKET_DESCRIPTOR bsd_create_udp_socket(const char *host, int port, int op
}
#endif

#if defined(__linux__)
/* Linux suppresses ICMP errors (port unreachable, host unreachable, TTL
* exceeded, etc.) on unconnected UDP sockets by default. Enabling
* IP_RECVERR/IPV6_RECVERR surfaces them as errors on the next send/recv,
* rather than silently dropping them. Matches libuv. */
#ifdef IP_RECVERR
setsockopt(listenFd, IPPROTO_IP, IP_RECVERR, &enabled, sizeof(enabled));
#endif
#ifdef IPV6_RECVERR
if (listenAddr->ai_family == AF_INET6) {
setsockopt(listenFd, IPPROTO_IPV6, IPV6_RECVERR, &enabled, sizeof(enabled));
}
#endif
#endif

/* We bind here as well */
if (bind(listenFd, listenAddr->ai_addr, (socklen_t) listenAddr->ai_addrlen)) {
if (err != NULL) {
Expand All @@ -1543,6 +1528,28 @@ LIBUS_SOCKET_DESCRIPTOR bsd_create_udp_socket(const char *host, int port, int op
return listenFd;
}

/* Linux drops ICMP errors (port unreachable, host unreachable, TTL exceeded)
* on unconnected UDP sockets unless IP_RECVERR is set, and node relies on that:
* only a connected socket has a peer to attribute the error to. Disabling it
* purges whatever the kernel already queued. */
static void bsd_udp_socket_set_recverr(LIBUS_SOCKET_DESCRIPTOR fd, int enabled) {
Comment thread
robobun marked this conversation as resolved.
Resolved
#if defined(__linux__)
#ifdef IP_RECVERR
setsockopt(fd, IPPROTO_IP, IP_RECVERR, &enabled, sizeof(enabled));
#endif
#ifdef IPV6_RECVERR
int domain = AF_INET;
socklen_t domain_len = sizeof(domain);
if (getsockopt(fd, SOL_SOCKET, SO_DOMAIN, &domain, &domain_len) == 0 && domain == AF_INET6) {
setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &enabled, sizeof(enabled));
}
#endif
#else
(void) fd;
(void) enabled;
#endif
}
Comment thread
robobun marked this conversation as resolved.
Resolved

int bsd_connect_udp_socket(LIBUS_SOCKET_DESCRIPTOR fd, const char *host, int port) {
struct addrinfo hints, *result;
memset(&hints, 0, sizeof(struct addrinfo));
Expand All @@ -1566,6 +1573,7 @@ int bsd_connect_udp_socket(LIBUS_SOCKET_DESCRIPTOR fd, const char *host, int por
for (struct addrinfo *rp = result; rp != NULL; rp = rp->ai_next) {
if (connect(fd, rp->ai_addr, rp->ai_addrlen) == 0) {
freeaddrinfo(result);
bsd_udp_socket_set_recverr(fd, 1);
return 0;
}
}
Expand All @@ -1591,6 +1599,7 @@ int bsd_disconnect_udp_socket(LIBUS_SOCKET_DESCRIPTOR fd) {
errno == EAFNOSUPPORT
#endif
) {
bsd_udp_socket_set_recverr(fd, 0);
return 0;
} else {
return -1;
Expand Down
32 changes: 23 additions & 9 deletions packages/bun-usockets/src/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -744,15 +744,15 @@ 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 (kqueue EV_ERROR, Windows) an error event is fatal —
* preserve close-on-error there. */
/* On Linux, connected UDP sockets have IP_RECVERR armed (see
* bsd_connect_udp_socket), so EPOLLERR fires with the ICMP error
* (port unreachable, host unreachable, TTL exceeded, ...) sitting
* on the socket's error queue, where only recvmsg(MSG_ERRQUEUE)
* dequeues it — 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 (kqueue
* EV_ERROR, Windows) an error event is fatal — preserve
* close-on-error there. */
int recv_error_surfaced = 0;
int recv_would_block_only = 0;
if (error) {
Expand All @@ -778,6 +778,20 @@ void us_internal_dispatch_ready_poll(struct us_poll_t *p, int error, int eof, in
u->on_recv_error(u, ee ? ee : ECONNREFUSED);
}
}
/* sk_err outlives the error queue: disabling IP_RECVERR on
* disconnect purges the queue but not sk_err, and a socket
* whose IP_RECVERR setup failed never queues at all. Consume
* it so EPOLLERR clears instead of closing the socket below. */
if (!recv_error_surfaced && !u->closed) {
int so_error = 0;
socklen_t so_error_len = sizeof(so_error);
if (getsockopt(us_poll_fd(p), SOL_SOCKET, SO_ERROR, &so_error, &so_error_len) == 0 && so_error) {
recv_error_surfaced = 1;
if (u->on_recv_error) {
u->on_recv_error(u, so_error);
}
}
}
}
#endif

Expand Down
13 changes: 7 additions & 6 deletions src/runtime/socket/udp_socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,13 @@ extern "C" fn on_close(socket: *mut uws::udp::Socket) {
}

extern "C" fn on_recv_error(socket: *mut uws::udp::Socket, errno: c_int) {
// Only called on Linux via IP_RECVERR — loop.c guards the recv-on-error
// path with #if defined(__linux__) to preserve the pre-existing
// close-on-error behavior on kqueue/Windows (where an error event is a
// fatal socket condition, not a drainable error queue). Builds a
// SystemError from the ICMP errno (ECONNREFUSED, EHOSTUNREACH,
// ENETUNREACH, EMSGSIZE, ...) and dispatches through the 'error' handler.
// Only called on Linux. loop.c surfaces the errno here when EPOLLERR fires:
// from the IP_RECVERR error queue (armed on connect in bsd_connect_udp_socket)
// or from SO_ERROR when that queue is already empty, e.g. right after a
// disconnect purged it. The recv-on-error path is guarded with
// #if defined(__linux__) to preserve the pre-existing close-on-error
// behavior on kqueue/Windows (where an error event is a fatal socket
// condition, not a drainable error queue).
let this: &UDPSocket = UDPSocket::from_uws(socket);
let sys_err = bun_sys::Error::from_code_int(errno, bun_sys::Tag::recv);
let global_this = this.global_this.get();
Expand Down
68 changes: 68 additions & 0 deletions test/js/bun/udp/dgram-unconnected-icmp-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

116 changes: 115 additions & 1 deletion test/js/bun/udp/dgram.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { describe, expect, jest, test } from "bun:test";
import { createSocket } from "dgram";

import { disableAggressiveGCScope } from "harness";
import { bunEnv, bunExe, disableAggressiveGCScope, isLinux } from "harness";
import path from "path";
import { nodeDataCases } from "./testdata";

Expand Down Expand Up @@ -304,3 +304,117 @@
expect(onError).not.toHaveBeenCalled();
});
});

// An unconnected socket has no single peer, so node never reports ICMP errors
// on one: sending to a peer that has gone away is a no-op, and nothing listens
// for 'error'.
describe("ICMP errors on an unconnected socket", () => {
test("a vanished peer does not emit 'error'", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const dgram = require("node:dgram");
// No 'error' listener, which is what node's contract allows.
const socket = dgram.createSocket("udp4");
socket.bind(0, "127.0.0.1", () => {
// Bind and close a probe to get a port nothing is listening on. It
// is bound after 'socket' so the kernel cannot hand it the same one.
const probe = dgram.createSocket("udp4");
probe.bind(0, "127.0.0.1", () => {
const deadPort = probe.address().port;
probe.close(() => {
let sent = 0;
const tick = () => {
socket.send("ping", deadPort, "127.0.0.1");
if (++sent === 20) {
console.log("sent " + sent);
socket.close();
return;
}
setImmediate(tick);
};
tick();
});
});
});
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("ECONNREFUSED");
expect(stdout.trim()).toBe("sent 20");
expect(exitCode).toBe(0);
});

test("a vanished peer does not poison the next send()", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), path.join(import.meta.dir, "dgram-unconnected-icmp-fixture.ts")],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("ECONNREFUSED");
expect(JSON.parse(stdout)).toEqual({
sendErrors: [],
received: ["live-0", "live-1", "live-2", "live-3", "live-4"],
});
expect(exitCode).toBe(0);
});

// A connected socket records the ICMP error in the kernel's sk_err, which
// outlives the error queue that disconnecting purges. Stranding it there
// leaves the poll permanently in an error state.
test.skipIf(!isLinux)("disconnect() with a peer's ICMP error pending keeps the socket usable", async () => {
function bind(socket: ReturnType<typeof createSocket>): Promise<number> {
const { promise, resolve, reject } = Promise.withResolvers<number>();
const onError = (err: Error) => reject(err);
socket.once("error", onError);
socket.bind(0, "127.0.0.1", () => {
socket.removeListener("error", onError);
resolve(socket.address().port);
});
return promise;
}

await using live = createSocket("udp4");
const livePort = await bind(live);

await using socket = createSocket("udp4");
await bind(socket);

// Bound last so the kernel cannot hand this port to one of the sockets above.
const probe = createSocket("udp4");
const deadPort = await bind(probe);
await new Promise<void>(resolve => probe.close(() => resolve()));

const { promise: connected, resolve: onConnect } = Promise.withResolvers<void>();
socket.connect(deadPort, "127.0.0.1", onConnect);
await connected;

// The error belongs to the peer we were connected to, so node still reports
// it after disconnect(). What must not happen is the socket quietly closing.
const { promise: errored, resolve: onError } = Promise.withResolvers<NodeJS.ErrnoException>();
socket.once("error", onError);

// localhost answers this sendto() with ICMP port unreachable before it
// returns, so the error is already pending when disconnect() runs.
socket.send("dead");
socket.disconnect();
expect((await errored).code).toBe("ECONNREFUSED");

Check warning on line 411 in test/js/bun/udp/dgram.test.ts

View check run for this annotation

Claude / Claude Code Review

disconnect() ICMP test: 'await errored' can hang — no failure wiring and relies on unguaranteed softirq timing

The `await errored` promise only settles via `socket.once('error', ...)`, so if the error never fires — either on regression (pre-86e9520c the socket silently closed) or if the loopback ICMP is deferred to ksoftirqd past `setsockopt(IP_RECVERR, 0)` under CI load — the test hangs on the framework timeout instead of failing with a diagnosable assertion, and the actual "socket stays usable" invariant never runs. Consider driving connect→send→disconnect in a bounded retry until `gotError` (matching
Comment thread
robobun marked this conversation as resolved.
Resolved

const { promise: received, resolve: onMessage } = Promise.withResolvers<string>();
live.once("message", msg => onMessage(msg.toString()));
const { promise: sent, resolve: onSent, reject: onSendError } = Promise.withResolvers<void>();
socket.send("alive", livePort, "127.0.0.1", err => (err ? onSendError(err) : onSent()));
await sent;
expect(await received).toBe("alive");
});
});
Loading
Loading