From: Graham Campbell Date: Sat, 4 Jul 2026 19:45:31 +0000 (+0100) Subject: conncache: apply multi limits to transfers using a shared pool X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=02214d98f7ae3e7dad099e807c1ed549ef551014;p=thirdparty%2Fcurl.git conncache: apply multi limits to transfers using a shared pool - Determine max host and total connection limits from the transfer's own multi handle and test that it works for shared connections. Prior to this change, since df67269 (precedes 8.13.0), Curl_cpool_check_limits() took the limits from the multi handle that owns the pool. A share-owned pool is not owned by any multi, so the limit check always passed. Prior to df67269 the limits came from the transfer's own multi handle. Fixes https://github.com/curl/curl/issues/22265 Closes https://github.com/curl/curl/pull/22266 --- diff --git a/docs/libcurl/opts/CURLSHOPT_SHARE.md b/docs/libcurl/opts/CURLSHOPT_SHARE.md index 69b59779ad..827ba77ab6 100644 --- a/docs/libcurl/opts/CURLSHOPT_SHARE.md +++ b/docs/libcurl/opts/CURLSHOPT_SHARE.md @@ -79,6 +79,11 @@ in different threads using a shared connection. Note that when you use the multi interface, all easy handles added to the same multi handle share the connection cache by default without using this option. +Connection limits set with CURLMOPT_MAX_HOST_CONNECTIONS(3) and +CURLMOPT_MAX_TOTAL_CONNECTIONS(3) also apply to transfers using a shared +connection cache. Each transfer applies the limits of the multi handle it +runs on to the shared cache. + ## CURL_LOCK_DATA_PSL The Public Suffix List stored in the share object is made available to all diff --git a/lib/conncache.c b/lib/conncache.c index 6fc29be4b4..17e8d715b0 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -367,6 +367,22 @@ static struct connectdata *cpool_get_oldest_idle(struct cpool *cpool, return oldest_idle; } +/* Evict an idle connection to make room in the pool. A pool owned by + * a share has no multi that could perform a controlled shutdown of the + * connection; terminate it right away. Otherwise, hand it to the + * transfer's multi for shutdown. Expects the pool to be locked. */ +static void cpool_evict_conn(struct cpool *cpool, + struct Curl_easy *data, + struct connectdata *conn) +{ + if(cpool->share) { + cpool_remove_conn(cpool, conn); + Curl_cshutdn_terminate(cpool->idata, conn, TRUE); + } + else + Curl_conn_terminate(data, conn, FALSE); +} + int Curl_cpool_check_limits(struct Curl_easy *data, struct connectdata *conn) { @@ -380,9 +396,10 @@ int Curl_cpool_check_limits(struct Curl_easy *data, if(!cpool) return CPOOL_LIMIT_OK; - if(cpool->idata->multi) { - dest_limit = cpool->idata->multi->max_host_connections; - total_limit = cpool->idata->multi->max_total_connections; + /* multi determines the limits, no matter who owns the pool */ + if(data->multi) { + dest_limit = data->multi->max_host_connections; + total_limit = data->multi->max_total_connections; } if(!dest_limit && !total_limit) @@ -416,13 +433,13 @@ int Curl_cpool_check_limits(struct Curl_easy *data, " from %zu to reach destination limit of %zu", oldest_idle->connection_id, Curl_llist_count(&bundle->conns), dest_limit); - Curl_conn_terminate(cpool->idata, oldest_idle, FALSE); + cpool_evict_conn(cpool, data, oldest_idle); /* in case the bundle was destroyed in disconnect, look it up again */ bundle = cpool_find_bundle(cpool, conn); live = bundle ? Curl_llist_count(&bundle->conns) : 0; } - shutdowns = Curl_cshutdn_dest_count(cpool->idata, conn->destination); + shutdowns = Curl_cshutdn_dest_count(data, conn->destination); } if((live + shutdowns) >= dest_limit) { res = CPOOL_LIMIT_DEST; @@ -431,7 +448,7 @@ int Curl_cpool_check_limits(struct Curl_easy *data, } if(total_limit) { - shutdowns = Curl_cshutdn_count(cpool->idata); + shutdowns = Curl_cshutdn_count(data); while((cpool->num_conn + shutdowns) >= total_limit) { if(shutdowns) { /* close one connection in shutdown right away, if we can */ @@ -448,9 +465,9 @@ int Curl_cpool_check_limits(struct Curl_easy *data, FMT_OFF_T " from %zu to reach total " "limit of %zu", oldest_idle->connection_id, cpool->num_conn, total_limit); - Curl_conn_terminate(cpool->idata, oldest_idle, FALSE); + cpool_evict_conn(cpool, data, oldest_idle); } - shutdowns = Curl_cshutdn_count(cpool->idata); + shutdowns = Curl_cshutdn_count(data); } if((cpool->num_conn + shutdowns) >= total_limit) { res = CPOOL_LIMIT_TOTAL; @@ -582,7 +599,7 @@ bool Curl_cpool_conn_now_idle(struct Curl_easy *data, oldest_idle = cpool_get_oldest_idle(cpool, Curl_pgrs_now(data)); kept = (oldest_idle != conn); if(oldest_idle) { - Curl_conn_terminate(data, oldest_idle, FALSE); + cpool_evict_conn(cpool, data, oldest_idle); } } if(do_lock) diff --git a/tests/http/test_02_download.py b/tests/http/test_02_download.py index c75f932b72..de2f1a3e09 100644 --- a/tests/http/test_02_download.py +++ b/tests/http/test_02_download.py @@ -586,7 +586,8 @@ class TestDownload: @pytest.mark.parametrize("proto", Env.http_h1_h2_protos()) @pytest.mark.parametrize("max_host_conns", [0, 1, 5]) - def test_02_33_max_host_conns(self, env: Env, httpd, nghttpx, proto, max_host_conns): + @pytest.mark.parametrize("share_connect", [False, True]) + def test_02_33_max_host_conns(self, env: Env, httpd, nghttpx, proto, max_host_conns, share_connect): if not env.curl_is_debug(): pytest.skip('only works for curl debug builds') if not env.curl_is_verbose(): @@ -601,6 +602,7 @@ class TestDownload: client = LocalClient(name='cli_hx_download', env=env, run_env=run_env) if not client.exists(): pytest.skip(f'example client not built: {client.name}') + extra_args = ['-S'] if share_connect else [] # share connections r = client.run(args=[ '-n', f'{count}', '-m', f'{max_parallel}', @@ -608,8 +610,7 @@ class TestDownload: '-x', # always use a fresh connection '-M', str(max_host_conns), # limit conns per host '-r', f'{env.domain1}:{port}:127.0.0.1', - '-V', proto, url - ]) + ] + extra_args + ['-V', proto, url]) r.check_exit_code(0) srcfile = os.path.join(httpd.docs_dir, docname) self.check_downloads(client, srcfile, count) @@ -625,7 +626,8 @@ class TestDownload: @pytest.mark.parametrize("proto", Env.http_h1_h2_protos()) @pytest.mark.parametrize("max_total_conns", [0, 1, 5]) - def test_02_34_max_total_conns(self, env: Env, httpd, nghttpx, proto, max_total_conns): + @pytest.mark.parametrize("share_connect", [False, True]) + def test_02_34_max_total_conns(self, env: Env, httpd, nghttpx, proto, max_total_conns, share_connect): if not env.curl_is_debug(): pytest.skip('only works for curl debug builds') if not env.curl_is_verbose(): @@ -640,6 +642,7 @@ class TestDownload: client = LocalClient(name='cli_hx_download', env=env, run_env=run_env) if not client.exists(): pytest.skip(f'example client not built: {client.name}') + extra_args = ['-S'] if share_connect else [] # share connections r = client.run(args=[ '-n', f'{count}', '-m', f'{max_parallel}', @@ -647,8 +650,7 @@ class TestDownload: '-x', # always use a fresh connection '-T', str(max_total_conns), # limit total connections '-r', f'{env.domain1}:{port}:127.0.0.1', - '-V', proto, url - ]) + ] + extra_args + ['-V', proto, url]) r.check_exit_code(0) srcfile = os.path.join(httpd.docs_dir, docname) self.check_downloads(client, srcfile, count) diff --git a/tests/http/test_19_shutdown.py b/tests/http/test_19_shutdown.py index 4ed9ad27fb..f8a948acd6 100644 --- a/tests/http/test_19_shutdown.py +++ b/tests/http/test_19_shutdown.py @@ -185,7 +185,8 @@ class TestShutdown: # run connection pressure, many small transfers, not reusing connections, # limited total @pytest.mark.parametrize("proto", ['http/1.1']) - def test_19_07_shutdown_by_curl(self, env: Env, httpd, proto): + @pytest.mark.parametrize("share_connect", [False, True]) + def test_19_07_shutdown_by_curl(self, env: Env, httpd, proto, share_connect): if not env.curl_is_debug(): pytest.skip('only works for curl debug builds') count = 500 @@ -197,15 +198,14 @@ class TestShutdown: }) if not client.exists(): pytest.skip(f'example client not built: {client.name}') + extra_args = ['-S'] if share_connect else [] # share connections r = client.run(args=[ '-n', f'{count}', # that many transfers '-C', env.ca.cert_file, '-f', # forbid conn reuse '-m', '10', # max parallel '-T', '5', # max total conns at a time - '-V', proto, - url - ]) + ] + extra_args + ['-V', proto, url]) r.check_exit_code(0) shutdowns = [line for line in r.trace_lines if re.match(r'.*SHUTDOWN] shutdown, done=1', line)] diff --git a/tests/libtest/cli_hx_download.c b/tests/libtest/cli_hx_download.c index f964067f15..6ad18e124a 100644 --- a/tests/libtest/cli_hx_download.c +++ b/tests/libtest/cli_hx_download.c @@ -285,6 +285,7 @@ static void usage_hx_download(const char *msg) " -M number max concurrent connections to a host\n" " -P number pause transfer after `number` response bytes\n" " -r :: resolve information\n" + " -S share connections between easy handles\n" " -T number max concurrent connections total\n" " -V http_version (http/1.1, h2, h3) http version to use\n" " -6 use IPv6 for resolving the FIRST URL\n" @@ -314,13 +315,14 @@ static CURLcode test_cli_hx_download(const char *URL) size_t max_host_conns = 0; size_t max_total_conns = 0; int fresh_connect = 0; + int share_connect = 0; char *cafile = NULL; bool first_ipv6 = FALSE; CURLcode result = CURLE_OK; (void)URL; - while((ch = cgetopt(test_argc, test_argv, "aefhm:n:xA:C:F:M:P:r:T:V:6")) + while((ch = cgetopt(test_argc, test_argv, "aefhm:n:xA:C:F:M:P:r:ST:V:6")) != -1) { const char *opt = coptarg; curl_off_t num; @@ -373,6 +375,9 @@ static CURLcode test_cli_hx_download(const char *URL) curlx_free(resolve); resolve = curlx_strdup(coptarg); break; + case 'S': + share_connect = 1; + break; case 'T': if(!curlx_str_number(&opt, &num, LONG_MAX)) max_total_conns = (size_t)num; @@ -430,9 +435,8 @@ static CURLcode test_cli_hx_download(const char *URL) curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE); curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); -#if 0 - curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); -#endif + if(share_connect) + curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_PSL); curl_share_setopt(share, CURLSHOPT_SHARE, CURL_LOCK_DATA_HSTS);