Skip to content

happy eyeballs: bound work from many duplicate/failing addresses - #22897

Closed
MegaManSec wants to merge 2 commits into
curl:masterfrom
MegaManSec:worktree-agent-a915e3a9caca0ba70
Closed

MegaManSec wants to merge 2 commits into
curl:masterfrom
MegaManSec:worktree-agent-a915e3a9caca0ba70

Conversation

@MegaManSec

Copy link
Copy Markdown
Contributor

Frees failed connection attempts immediately, adds a tail pointer for O(1) appends, checks the connection timeout on every loop iteration, and caps/deduplicates addresses converted from a c-ares response.

A DNS response with many duplicate addresses that fail to connect
synchronously could stall a transfer: failed attempts were never
pruned from the running list (cf_ip_ballers_prune only removes
still-ongoing ones), so every new address triggered a full rescan and
an O(n) tail-walk to append, and the connection timeout was only
checked after the loop, not during it.

Free hard-failed attempts immediately instead of accumulating them,
track a tail pointer for O(1) appends, and check the timeout on each
iteration. Also cap and deduplicate the addresses converted from a
c-ares response, closing off the unbounded input side.
Rename ares_addr_seen() to async_ares_addr_seen() and ARES_MAX_ADDRS to
CARES_MAX_ADDRS to match the other statics and defines in the file and
to stay out of c-ares' own ares_*/ARES_* namespace. Make the lookup
arguments const.

Also assert in cf_ip_ballers_run() that the running list is empty once
no attempts are left, which is the invariant the last_dead_result
reporting relies on.
@bagder
bagder requested review from icing and a balanced review from Copilot September 10, 2026 12:26

Copilot AI 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.

🟢 Approval recommended

The bounded address processing and connection-attempt lifecycle changes are internally consistent and preserve list invariants.

Pull request overview

Bounds Happy Eyeballs resource usage for large or failure-heavy address lists.

Changes:

  • Deduplicates and caps c-ares results at 256 addresses per response.
  • Immediately removes failed connection attempts.
  • Adds O(1) list appends and loop-level timeout checks.
File summaries
File Description
lib/vdns/asyn-ares.c Caps and deduplicates converted addresses.
lib/cf-ip-happy.c Improves attempt cleanup, timeout handling, and list performance.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@icing icing left a comment

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.

lgtm!

@bagder bagder closed this in 3341da7 Sep 11, 2026
@bagder

bagder commented Sep 11, 2026

Copy link
Copy Markdown
Member

Thanks!

@MegaManSec
MegaManSec deleted the worktree-agent-a915e3a9caca0ba70 branch September 14, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants