cf-ip-happy: limit concurrent attempts#21252
Closed
icing wants to merge 2 commits intocurl:masterfrom
Closed
Conversation
Introduce a limit on the concurrent connect attempts of 6: - document this in CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS - close the oldest attempt before opening a new one that would exceed the limit - closing failed attempts early to avoid sockets use beyong their usefulness - add tests for limits in unit2600 These changes are externally visible as file descriptors will be reassigned where we previously kept the old one around and started a new socket, allocating always a new descriptor.
bagder
approved these changes
Apr 7, 2026
There was a problem hiding this comment.
Pull request overview
This PR introduces a hard cap on concurrent “Happy Eyeballs” connection attempts in cf-ip-happy (limit: 6), closing/discarding older or failed attempts earlier to reduce socket/FD usage and making the behavior externally visible via FD reuse. It also updates the related libcurl option documentation and extends unit coverage in unit2600.
Changes:
- Add a maximum concurrent connect-attempt limit (6) and prune the oldest ongoing attempt before starting a new one when at the limit.
- Discard failed attempt filter chains earlier to avoid holding sockets beyond their usefulness.
- Document the new limit and add a unit test that asserts the maximum concurrency behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/cf-ip-happy.c |
Adds concurrent-attempt limiting via pruning and discards failed attempt chains earlier; adds null-safety around attempts whose cfilters were discarded. |
tests/unit/unit2600.c |
Tracks and asserts maximum concurrent attempts during tests; adds a new test case that must hit the concurrency cap. |
docs/libcurl/opts/CURLOPT_HAPPY_EYEBALLS_TIMEOUT_MS.md |
Documents the new concurrent socket/attempt limit and its effect on attempt lifetime. |
Comments suppressed due to low confidence (1)
tests/unit/unit2600.c:243
current_tr->ongoingis incremented beforeCurl_cf_create(), but on the error path (Curl_cf_createreturning non-zero) the counter is never decremented. This can leaveongoing/max_concurrentincorrect (and can later underflow on destroy). Consider only incrementing after successfulCurl_cf_create(), or decrementingongoingin theif(result)cleanup block before returning.
ctx->started = curlx_now();
current_tr->ongoing++;
if(current_tr->ongoing > current_tr->max_concurrent)
current_tr->max_concurrent = current_tr->ongoing;
#ifdef USE_IPV6
if(ctx->ai_family == AF_INET6) {
ctx->stats = ¤t_tr->cf6;
ctx->fail_delay_ms = current_tc->cf6_fail_delay_ms;
curl_msprintf(ctx->id, "v6-%d", ctx->stats->creations);
ctx->stats->creations++;
}
else
#endif
{
ctx->stats = ¤t_tr->cf4;
ctx->fail_delay_ms = current_tc->cf4_fail_delay_ms;
curl_msprintf(ctx->id, "v4-%d", ctx->stats->creations);
ctx->stats->creations++;
}
created_at = curlx_timediff_ms(ctx->started, current_tr->started);
if(ctx->stats->creations == 1)
ctx->stats->first_created = created_at;
ctx->stats->last_created = created_at;
infof(data, "%04dms: cf[%s] created, now %d ongoing",
(int)created_at, ctx->id, current_tr->ongoing);
result = Curl_cf_create(&cf, &cft_test, ctx);
if(result)
goto out;
Curl_expire(data, ctx->fail_delay_ms, EXPIRE_TIMEOUT);
out:
*pcf = (!result) ? cf : NULL;
if(result) {
curlx_free(cf);
curlx_free(ctx);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce a limit on the concurrent connect attempts of 6:
These changes are externally visible as file descriptors will be reassigned where we previously kept the old one around and started a new socket, allocating always a new descriptor.