Skip to content

udp: send every packet of a sendMany() batch larger than one send buffer - #33391

Closed
robobun wants to merge 2 commits into
mainfrom
farm/195c664b/udp-sendmany-batch-loop
Closed

robobun wants to merge 2 commits into
mainfrom
farm/195c664b/udp-sendmany-batch-loop

Conversation

@robobun

@robobun robobun commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Bun.udpSocket().sendMany() silently drops packets once the batch outgrows the internal sendmmsg send buffer (~204 slots), and the documented backpressure protocol has nothing to resume it.

Repro

const server = await Bun.udpSocket({ socket: { data() {} } });
const client = await Bun.udpSocket({
  connect: { hostname: "127.0.0.1", port: server.port },
  socket: { drain() { drains++; } },
});

let drains = 0;
const packets = Array.from({ length: 300 }, (_, i) => `packet ${i}`);
console.log(client.sendMany(packets)); // 204 on an idle loopback socket, expected 300
// ...and `drain` never fires, so "wait for drain, resend the rest" never resumes.

On 1.4.0: N=300 returns 204, N=500 returns 408 plus one drain with no backlog behind it. Anything at or below 204 works, which is why the existing 100-packet tests never caught it.

Cause

us_udp_socket_send() decremented num by each pass's batch size before the loop condition and the drain guard read it:

num -= count;
int sent = bsd_sendmmsg(fd, buf, MSG_DONTWAIT);
...
total_sent += sent;
if (0 <= sent && sent < num) { /* arm the writable poll */ }

For num = 300: pass one sends 204, num becomes 96, total_sent < num is 204 < 96 so the loop exits with 96 packets never handed to the kernel. sent < num is 204 < 96 too, so the writable poll is never armed and drain never fires. At num = 500 the same comparison fires the other way in pass one (204 < 296) and arms a drain for a send that was not actually blocked.

The same guard is what a real short send lands in, so genuine kernel backpressure could not arm drain either.

Fix

Walk the batch with an offset and leave num alone, then key backpressure off what sendmmsg actually reports:

  • -1 with EWOULDBLOCK is the only recoverable outcome: arm the writable poll, return the count that went out.
  • any other errno goes straight to the caller, which throws.
  • a short count means the datagram at index sent failed. sendmmsg(2) drops that datagram's errno ("If an error occurs after at least one message has been sent... the error code is lost"), so the next pass retries from it, and the errno comes back as -1.

That last part also fixes a sibling bug in the same function: an oversized datagram threw EMSGSIZE at index 0 but came back as a short count ("1 of 3 sent") anywhere after it, so an un-sendable packet was indistinguishable from backpressure. It now throws wherever it sits in the batch.

Two behavior changes worth naming:

  • A full kernel send buffer used to throw EAGAIN out of send()/sendMany(). It now returns false / a short count and arms drain, which is what the docs describe.
  • A fatal errno partway through a batch throws even if earlier datagrams were sent, so the partial count is lost. That matches what already happened when the bad datagram was first, and the alternative (report a short count, surface the error one drain later) is what made oversized packets look like backpressure.

Verification

test/js/bun/udp/udp_socket.test.ts: 8 of the 10 new cases fail on the unfixed build (the two "index 0" cases pass both ways on purpose, they are the control for the position dependence).

(fail) sendMany sends every packet of a batch larger than one send buffer > connected, 205 packets
(fail) sendMany sends every packet of a batch larger than one send buffer > connected, 300 packets
(fail) sendMany sends every packet of a batch larger than one send buffer > connected, 500 packets
(fail) sendMany sends every packet of a batch larger than one send buffer > unconnected, 300 packets
(fail) sendMany surfaces a per-packet error whatever its position > connected, oversized packet at index 1
(fail) sendMany surfaces a per-packet error whatever its position > connected, oversized packet at index 2
(fail) sendMany surfaces a per-packet error whatever its position > unconnected, oversized packet at index 1
(fail) sendMany surfaces a per-packet error whatever its position > unconnected, oversized packet at index 2

All 188 pass with the fix, as do test/js/bun/udp/dgram.test.ts, udp_socket_recv_flags.test.ts, and every test/js/node/test/parallel/test-dgram-*.js. The 500-packet send was run 30 times without a short count.

us_udp_socket_send() subtracted each pass's batch size from num before
testing it, so the loop ended after the first pass and the drain guard
compared the pass's result against the remaining count instead of the
pass's own size. sendMany(300) on an idle socket returned 204, left the
other 96 unsent, and never armed the writable poll, so the documented
"if fewer were sent, wait for drain and resend the rest" protocol had
nothing to wait for; sendMany(500) returned 408 and armed a drain with
no backlog.

Walk the batch with an offset instead, and key backpressure off
sendmmsg's own report: -1 with EWOULDBLOCK arms the writable poll and
returns the count that did go out, any other errno goes to the caller.
sendmmsg(2) turns a datagram that fails mid-batch into a short count and
drops its errno, so a short count now retries from the failed datagram,
which brings the errno back as -1. An oversized payload therefore throws
EMSGSIZE wherever it sits in the batch rather than only at index 0.
@github-actions github-actions Bot added the claude label Jul 5, 2026
@robobun

robobun commented Jul 5, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:54 PM PT - Jul 5th, 2026

✅ @robobun, your commit 2d2cd41bf289b7250a210479c377260598ac8ce0 passed in Build #68660! 🎉


🧪   To try this PR locally:

bunx bun-pr 33391

That installs a local version of the PR into your bun-33391 executable, so you can run:

bun-33391 --bun

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1059a981-8c74-400e-861a-9e4596842f5c

📥 Commits

Reviewing files that changed from the base of the PR and between fb50cce and 1383f3e.

📒 Files selected for processing (2)
  • packages/bun-usockets/src/udp.c
  • test/js/bun/udp/udp_socket.test.ts

Walkthrough

This PR reworks the UDP batch-send loop in us_udp_socket_send to retry remaining datagrams using offset-based pointers and distinguishes would-block errors from fatal errors, and adds tests verifying full-batch send counts and EMSGSIZE error reporting for sendMany.

Changes

UDP send retry logic and tests

Layer / File(s) Summary
Batch send retry loop rewrite
packages/bun-usockets/src/udp.c
Rewrote us_udp_socket_send to repeatedly call bsd_udp_setup_sendbuf with pointer offsets and remaining count (num - total_sent), returning early only on non-would-block errors and registering writable state on would-block, accumulating total_sent on success.
sendMany batch and error tests
test/js/bun/udp/udp_socket.test.ts
Added tests confirming sendMany reports full packet counts when batch size exceeds send buffer capacity (205/300/500 packets, connected/unconnected), and tests confirming EMSGSIZE errors are surfaced correctly regardless of oversized packet position in the batch.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant us_udp_socket_send
  participant bsd_udp_setup_sendbuf
  participant bsd_sendmmsg

  Caller->>us_udp_socket_send: send(payloads, lengths, addresses, num)
  loop while total_sent < num
    us_udp_socket_send->>bsd_udp_setup_sendbuf: setup(payloads+total_sent, num-total_sent)
    us_udp_socket_send->>bsd_sendmmsg: sendmmsg()
    alt sent < 0 and not would_block
      bsd_sendmmsg-->>us_udp_socket_send: error
      us_udp_socket_send-->>Caller: return error
    else would_block
      us_udp_socket_send->>us_udp_socket_send: register writable, break
    else sent >= 0
      bsd_sendmmsg-->>us_udp_socket_send: sent count
      us_udp_socket_send->>us_udp_socket_send: total_sent += sent
    end
  end
Loading

Estimated code review effort: 3/5 (High complexity C-level retry logic change requiring careful review of socket error semantics; tests are straightforward.)

Related issues: None found in provided context.

Related PRs: None found in provided context.

Suggested labels: bun:usockets, needs-tests-verified

Suggested reviewers: None found in provided context.

🥁 A drumroll for datagrams stuck in line,
Retrying sends, one offset at a time,
Would-block waits, no errors misconstrued,
Tests confirm the packets all got through.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: sendMany now handles batches larger than one send buffer without dropping packets.
Description check ✅ Passed The description covers the bug, root cause, fix, and verification, though it doesn't use the exact template headings.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

Comment thread packages/bun-usockets/src/udp.c
@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-05 and it conflicts with main. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

@robobun

robobun commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up for anyone who lands here. #32625 fixed the main finding of this PR (the loop that stopped after the first batch of about 204 datagrams), so this PR stays closed.

The second half of this PR made a mid-batch errno throw at any position. That is not revived: #32602 chose the opposite contract on purpose (a count once one datagram is out, and the errno on the next call), which matches sendmmsg(2). One site of that contract returns a short count without arming drain, so the caller hangs. #42999 fixes that site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant