02214d98f7 — curl

Subject

conncache: apply multi limits to transfers using a shared pool

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

docs/libcurl/opts/CURLSHOPT_SHARE.md +5 -0 lib/conncache.c +26 -9 tests/http/test_02_download.py +8 -6 tests/http/test_19_shutdown.py +4 -4 tests/libtest/cli_hx_download.c +8 -4

docs/libcurl/opts/CURLSHOPT_SHARE.md+5 -0

@@ -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

lib/conncache.c+26 -9

@@ -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)

tests/http/test_02_download.py+8 -6

@@ -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)

tests/http/test_19_shutdown.py+4 -4

@@ -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)]

tests/libtest/cli_hx_download.c+8 -4

@@ -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 <host>:<port>:<addr> 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);