Skip to content

Try every address candidate when binding and when sending a datagram - #14

Merged
hellerve merged 1 commit into
masterfrom
claude/resolve-all-candidates
Aug 10, 2026
Merged

Try every address candidate when binding and when sending a datagram#14
hellerve merged 1 commit into
masterfrom
claude/resolve-all-candidates

Conversation

@carpentry-agent

Copy link
Copy Markdown
Contributor

resolve_address (src/common.h) took the first addrinfo getaddrinfo returned and freed the rest, so all three call sites inherited whatever the resolver happened to order first. TcpStream.connect already looped over candidates; the passive path and UdpSocket.send-to did not.

Both consequences reproduce on a box where getaddrinfo("localhost", …) returns ::1 before 127.0.0.1 (this one, per /etc/hosts).

A — UdpSocket.send-to ignores the socket's own family. (UdpSocket.bind "127.0.0.1" 0) then (UdpSocket.send-to &s "localhost" p &data) resolved the destination to ::1, so sendto got an AF_INET6 sockaddr on an AF_INET socket:

bound port 60060
send-to localhost  FAILED: Address family not supported by protocol   (errno 97)
send-to 127.0.0.1  OK: 4

B — the passive path gives up after the first candidate. With ::1:P occupied by an unrelated socket, TcpListener.bind "localhost" P failed outright even though 127.0.0.1:P was free:

bind localhost  FAILED: Address already in use
bind 127.0.0.1  OK port=58437

The same shape applies to a host whose /etc/hosts still lists ::1 localhost while IPv6 is unavailable.

What changed

bind_address in common.h owns socket creation — the family is per-candidate, so a pure address-out helper can't serve the bind path — and iterates socket()setsockoptbind(), closing the fd and continuing on failure, failing only when every candidate does. Hints carry AI_PASSIVE. TcpListener_bind_ and UdpSocket_bind_ call it; the getsockname() that reports the real port after binding to 0 is preserved, and listen() stays outside the loop.

resolve_address gains a prefer_family argument and keeps first-candidate behaviour when nothing matches. UdpSocket_send_MINUS_to_ passes u->bound.ss_family.

Details worth flagging for review:

  • SO_REUSEADDR/SO_REUSEPORT stay TCP-only, gated on socktype == SOCK_STREAM, matching what each path did before.
  • bind()'s errno is saved across close(), so (System.error-text) still reports the bind failure rather than whatever close left behind. Verified A/B against master: Address already in use, Permission denied, and Invalid argument for a bogus and an empty host are byte-identical on both.
  • Numeric hosts are unaffected — one candidate, so the loop runs the same socket/setsockopt/bind sequence once. The only literal difference is socket(…, rp->ai_protocol) instead of socket(…, 0), matching the existing TcpStream_connect_ loop; for SOCK_STREAM/SOCK_DGRAM these select the same protocol.
  • IPV6_V6ONLY is deliberately left at the host default. Every candidate this loop can bind to is a specific address, and V6ONLY does not affect a specific-address bind — so nothing here needs it. Pinning a value either way would silently change existing wildcard "::" binds (Linux defaults to dual-stack, the BSDs to v6-only), which is outside this fix. Happy to set it if you'd rather it were explicit.

Tests

Both new tests were confirmed to fail on unmodified src/ (tests applied, src/ stashed → carp -x exits 1 on both suites) and pass on this branch, with no skip path taken.

test/udp_test.carp round-trips a datagram sent to localhost from a socket bound explicitly to 127.0.0.1, and again from one bound to ::1. The pair is order-independent: whichever family the resolver puts first, the other half is the mismatched-family case. Each half is skipped, with a printed reason, if localhost lacks that family.

test/tcp_test.carp occupies whichever candidate the resolver returns first — via test/addr_helpers.h, a plain listening socket with no SO_REUSEADDR/SO_REUSEPORT, on an OS-assigned port — then asserts TcpListener.bind "localhost" port still succeeds. It skips with a printed reason when localhost resolves to a single family. The helper also probes the occupied address with a SO_REUSEADDR|SO_REUSEPORT socket and reports -2 if the duplicate bind is permitted, so the test skips rather than passing vacuously on a platform with laxer reuse semantics (I can verify the EADDRINUSE precondition on Linux, not on macOS).

All four suites built and run exactly as CI does (carp -b then carp -x on tcp/unix/poll/udp, bare exit status): 7/6/6/6 passed, 0 failed. carp-fmt --check and angler clean over the same file set CI lints. docs/ regenerated for the three docstring additions. No CHANGELOG.md in this repo, so none added.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

resolve_address took the first addrinfo getaddrinfo returned and freed the
rest, so all three call sites inherited whatever the resolver happened to
order first. TcpStream.connect already looped over candidates; the passive
path and UdpSocket.send-to did not.

Two user-visible failures follow. A UdpSocket bound to 127.0.0.1 and asked
to send-to "localhost" resolved the destination without regard for its own
family; where getaddrinfo returns ::1 first, sendto got an AF_INET6 sockaddr
on an AF_INET socket and failed with EAFNOSUPPORT. And TcpListener.bind /
UdpSocket.bind created a socket of the first candidate's family and gave up
if socket() or bind() failed, even when a later candidate would have bound
-- a host whose /etc/hosts lists "::1 localhost" while IPv6 is unavailable,
or the ::1 address already occupied.

bind_address owns socket creation, since the family is per-candidate, and
iterates socket() -> setsockopt -> bind(), closing the fd and continuing on
failure, failing only when every candidate does. Hints carry AI_PASSIVE.
SO_REUSEADDR/SO_REUSEPORT stay TCP-only, as before. bind() errno is saved
across close() so the message the caller reports is still the bind failure.
resolve_address gains a prefer_family argument and keeps first-candidate
behaviour when nothing matches, so a numeric host is unaffected.

IPV6_V6ONLY is deliberately left at the host default: a specific-address
bind is unaffected by it, so nothing in this change needs it, and pinning a
value either way would silently alter existing wildcard "::" binds.
@carpentry-agent

Copy link
Copy Markdown
Contributor Author

CI is green on both runners, and the job logs confirm neither new test took a skip path — no skipped: line on either, so both ran for real:

ubuntu-latest / macos-latest
  Test 'bind tries the next candidate when the first address is taken' passed
  Test 'send-to a dual-stack hostname reaches an IPv4-bound socket' passed
  Test 'send-to a dual-stack hostname reaches an IPv6-bound socket' passed

That settles the one caveat in the description: macOS did not report the address as rebindable, so the EADDRINUSE precondition the TCP fallback test depends on holds there too, and the test is doing real work rather than passing vacuously. The -2 guard was not needed in the end, but it is what proved that rather than my assuming it.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Build & Tests

Checked out 5bb78f3 and ran all four suites the way CI does, reading the exit status from the bare command rather than through a pipe: tcp 7 / unix 6 / poll 6 / udp 6 passed, 0 failed, rc=0 on every one. No skipped: line printed anywhere, so nothing took a skip path here either.

CI run 31395570637 has head_sha 5bb78f3, so it tested this head. I read the per-step conclusions rather than the aggregate: Build test targets, Run tests, Install angler, Install carp-fmt, Lint, Format check and Generate docs are each success on ubuntu-latest and on macos-latest, and the workflow carries no continue-on-error, so the green is real per step. I also grepped both job logs for the skip strings rather than taking your follow-up comment at face value: none are present, and all three new tests print passed on both runners. The macOS EADDRINUSE precondition you flagged as unverified does hold there.

Merge-base with master is 512065a, master's tip, so the branch state is the merged state.

Both new tests are load-bearing — checked independently rather than taken from the description. Restoring master's src/ while keeping this branch's tests:

suite branch master src/ + branch tests
tcp_test 7 passed, 0 failed, rc=0 6 passed, 1 failed, rc=1bind tries the next candidate when the first address is taken
udp_test 6 passed, 0 failed, rc=0 5 passed, 1 failed, rc=1send-to a dual-stack hostname reaches an IPv4-bound socket

Exactly one failure each, and it is the new test in both. src/ restored clean afterwards.

Findings

One, and it is about what the passive fallback means rather than how it is written. The C itself I went through looking for the usual failure modes and did not find any: every candidate's fd is closed on the failure path, freeaddrinfo runs on both exits, bind()'s errno is saved across close(), SO_REUSEADDR/SO_REUSEPORT stay gated on SOCK_STREAM exactly as each path did before, getsockname still corrects addr after a port-0 bind on both the TCP and UDP paths, and prefer_family degrades to the old first-candidate behaviour when nothing matches — including the AF_UNSPEC that a zeroed bound would give. All three resolve_address call sites are updated.

Falling back after EADDRINUSE turns a loud failure into a silent misroute

With [::1]:P held by a foreign process — a plain listening socket with no SO_REUSEADDR/SO_REUSEPORT, i.e. nginx, or a Python one-liner, which is what I used:

                                    master              this branch
  TcpListener.bind "localhost" P    Error: Address      Success, bound to 127.0.0.1:P
                                    already in use
  TcpStream.connect "localhost" P   --                  peer ::1 -> "FOREIGN-SERVER"

That last line is the problem. bind reports success, and a client on the same host asking for the same name is served by the other process, sending it whatever it would have sent yours. master refused to bind at all.

This is not an artefact of this box's resolver order. TcpListener_bind_ and TcpStream_connect_ walk the same candidate list, and I checked that AI_PASSIVE does not reorder it — getaddrinfo("localhost", …) returns ::1 127.0.0.1 with and without the flag here, as POSIX requires when nodename is non-NULL. connect stops at the first candidate that connects. So whenever the bind loop skips a candidate because something is already listening on it, that skipped candidate is by construction the one a same-host client reaches first, and something is listening there to answer. Reverse the resolver order and the same argument applies to whichever family comes first.

The two scenarios in the description are not the same shape, and only one of them has this problem.

  • "a host whose /etc/hosts still lists ::1 localhost while IPv6 is unavailable"socket() or bind() fails with EAFNOSUPPORT/EADDRNOTAVAIL, nothing can ever be listening on that candidate, and continuing is unambiguously right. This is the case that motivates the change and it is pure win.
  • "With ::1:P occupied by an unrelated socket" — the case the Address already in use transcript in the description is taken from — is the one above. For that scenario the fix replaces a diagnosable error with traffic going somewhere else.

So the question is whether the loop should continue on this address is taken at all, or only on this family is unusable. Narrowing it is small — save err, and stop the loop when it is EADDRINUSE — and it costs nothing that motivated the PR. It would, however, invert bind tries the next candidate when the first address is taken, which is built on exactly this case: the test occupies the first candidate with a real EADDRINUSE and asserts the fallback fires. So the decision decides the test, not the other way round.

Scope, honestly stated. The only non-test caller of TcpListener.bind in the org is Web.serve (web.carp:2885), and every documented example passes "0.0.0.0" — a single candidate, where none of this can arise. So this is a latent hazard in the API rather than a live bug in anything shipping today, and it needs a hostname, a foreign occupant, and a same-host client to bite. It is also what Go and Python do. I am raising it because it is silent, because the transcript in the description is the bad case, and because web.carp:2912 prints Listening on %s:%d with the requested host — so after a fallback the log says localhost while the socket is on 127.0.0.1 only.

UdpSocket.send-to is untouched by all of this and is the better half of the PR — preferring the socket's own family with a fall-back to first-candidate is strictly a superset of the old behaviour, and I could not construct a case where it is worse. The UDP bind path shares bind_address, so it inherits the same question, not a different one.

Verdict: revise

Everything mechanical is right — it builds, all four suites pass locally and on both CI runners per step, the two new tests demonstrably fail without the fix, and I found no defect in the C. What I want before this lands is a decision on one line: should the bind loop continue past a candidate that is merely occupied, given that a client asking for the same hostname then reaches the occupant? If you are happy with the trade-off, say so and the only change is a sentence in the two bind docstrings; if not, the EADDRINUSE guard is a couple of lines and the TCP test changes with it.

@hellerve
hellerve merged commit fe3bbab into master Aug 10, 2026
2 checks passed
@hellerve
hellerve deleted the claude/resolve-all-candidates branch August 10, 2026 23:04
@carpentry-agent carpentry-agent Bot mentioned this pull request Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant