← Commit list

73c0180cb6

Max Dymond

2026-09-03

SUCCESS Duration: 399s

Subject

multi: remove redundant completion message list

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/internals/MID.md +6 -5 lib/multi.c +28 -50 lib/multihandle.h +3 -10 lib/urldata.h +1 -1 tests/data/Makefile.am +1 -0 tests/data/test2453 +52 -0 tests/libtest/Makefile.inc +1 -0 tests/libtest/lib2453.c +118 -0

docs/internals/MID.md+6 -5

@@ -52,8 +52,9 @@ The table is implemented in `uint-table.[ch]`. More details in [`UINT_SETS`](UIN

	There are several places where transfers need to be tracked:

-	* the multi tracks `process`, `pending` and `msgsent` transfers. A transfer
-	is in at most one of these at a time.
+	* the multi tracks `process`, `pending` and `msgsent` transfers. A transfer is
+	in at most one of these at a time. A `dirty` set, which may overlap with
+	`process`, tracks transfers that need to run immediately.
	* connections track the transfers that are attached to them.
	* multi event handling tracks transfers interested in a specific socket.
	* DoH handles track the handle they perform lookups for (and vice versa).
@@ -63,9 +64,9 @@ The first is a bitset optimal for storing a large number of unsigned int values.
	The second one is a "sparse" variant good for storing a small set of numbers.
	More details about these in [`UINT_SETS`](UINT_SETS.md).

-	A multi uses `uint_bset`s for `process`, `pending` and `msgsent`. Connections
-	and sockets use the sparse variant as both often track only a single transfer
-	and at most 100 on an HTTP/2 or HTTP/3 connection/socket.
+	A multi uses `uint_bset`s for `process`, `dirty`, `pending` and `msgsent`.
+	Connections and sockets use the sparse variant as both often track only a
+	single transfer and at most 100 on an HTTP/2 or HTTP/3 connection/socket.

	These sets allow safe iteration while being modified. This allows a multi
	to iterate over its "process" set while existing transfers are removed

lib/multi.c+28 -50

@@ -210,14 +210,13 @@ static void ph_freeentry(void *p)
	/*
	* multi_addmsg()
	*
-	* Called when a transfer is completed. Adds the given msg pointer to
-	* the list kept in the multi handle.
+	* Called when a transfer is completed. Marks its message as unread.
	*/
-	static void multi_addmsg(struct Curl_multi multi, struct Curl_message msg)
+	static void multi_addmsg(struct Curl_multi multi, struct Curl_easy data)
	{
-	if(!Curl_llist_count(&multi->msglist))
+	if(Curl_uint32_bset_empty(&multi->msgsent))
	CURLM_NTFY(multi->admin, CURLMNOTIFY_INFO_READ);
-	Curl_llist_append(&multi->msglist, msg, &msg->list);
+	Curl_uint32_bset_add(&multi->msgsent, data->mid);
	}

	static void multi_timeouts_init(struct Curl_easy *data);
@@ -262,7 +261,6 @@ struct Curl_multi *Curl_multi_handle(uint32_t xfer_table_size,
	Curl_uint32_bset_init(&multi->msgsent);
	Curl_hash_init(&multi->proto_hash, 23,
	Curl_hash_str, curlx_str_key_compare, ph_freeentry);
-	Curl_llist_init(&multi->msglist, NULL);

	multi->multiplexing = TRUE;
	multi->max_concurrent_streams = 100;
@@ -765,7 +763,6 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi,
	{
	CURLMcode mresult;
	bool premature;
-	struct Curl_llist_node *e;
	uint32_t mid;

	/* Prevent users from trying to remove same easy handle more than once */
@@ -805,8 +802,8 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi,
	called. Do it after multi_done() in case that sets another time! */
	Curl_expire_clear_all(data);

-	/* If in `msgsent`, it was deducted from `multi->xfers_alive` already. */
-	if(!Curl_uint32_bset_contains(&multi->msgsent, data->mid))
+	/* In MSGSENT, it was deducted from `multi->xfers_alive` already. */
+	if(data->mstate != MSTATE_MSGSENT)
	--multi->xfers_alive;

	if(data->state.really_alive) {
@@ -857,18 +854,6 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi,
	data->psl = NULL;
	#endif

-	/* make sure there is no pending message in the queue sent from this easy
-	handle */
-	for(e = Curl_llist_head(&multi->msglist); e; e = Curl_node_next(e)) {
-	struct Curl_message *msg = Curl_node_elem(e);
-
-	if(msg->extmsg.easy_handle == data) {
-	Curl_node_remove(e);
-	/* there can only be one from this specific handle */
-	break;
-	}
-	}
-
	/* clear the association to this multi handle */
	mid = data->mid;
	DEBUGASSERT(Curl_uint32_tbl_contains(&multi->xfers, mid));
@@ -2432,8 +2417,10 @@ static void handle_completed(struct Curl_multi *multi,
	struct Curl_easy *data,
	CURLcode result)
	{
-	if(data->master_mid != UINT32_MAX) {
-	/* A sub transfer, not for msgsent to application. Is anyone still
+	bool msg_to_app = data->master_mid == UINT32_MAX;
+
+	if(!msg_to_app) {
+	/* A sub transfer, not reported to the application. Is anyone still
	* interested in processing its results? */
	if(data->sub_xfer_done) {
	struct Curl_easy *master = Curl_multi_get_easy(multi, data->master_mid);
@@ -2446,23 +2433,23 @@ static void handle_completed(struct Curl_multi *multi,
	}
	}
	else {
-	/* now fill in the Curl_message with this info */
-	struct Curl_message *msg = &data->msg;
+	/* now fill in the CURLMsg with this info */
+	struct CURLMsg *msg = &data->msg;

-	msg->extmsg.msg = CURLMSG_DONE;
-	msg->extmsg.easy_handle = data;
-	msg->extmsg.data.result = result;
+	msg->msg = CURLMSG_DONE;
+	msg->easy_handle = data;
+	msg->data.result = result;

-	multi_addmsg(multi, msg);
	DEBUGASSERT(!data->conn);
	}
	multistate(data, MSTATE_MSGSENT);

-	/* remove from the other sets, add to msgsent */
+	/* remove from the other sets */
	Curl_uint32_bset_remove(&multi->process, data->mid);
	Curl_uint32_bset_remove(&multi->dirty, data->mid);
	Curl_uint32_bset_remove(&multi->pending, data->mid);
-	Curl_uint32_bset_add(&multi->msgsent, data->mid);
+	if(msg_to_app)
+	multi_addmsg(multi, data);
	if(data->state.really_alive) {
	data->state.really_alive = FALSE;
	--multi->xfers_really_alive;
@@ -3097,10 +3084,7 @@ out:
	* curl_multi_info_read()
	*
	* This function is the primary way for a multi/multi_socket application to
-	* figure out if a transfer has ended. We MUST make this function as fast as
-	* possible as it will be polled frequently and we MUST NOT scan any lists in
-	* here to figure out things. We must scale fine to thousands of handles and
-	* beyond. The current design is fully O(1).
+	* figure out if a transfer has ended.
	*/

	CURLMsg curl_multi_info_read(CURLM m, int *msgs_in_queue)
@@ -3111,22 +3095,16 @@ CURLMsg curl_multi_info_read(CURLM m, int *msgs_in_queue)
	msgs_in_queue = 0; / default to none */
	if(CURL_MAPI_ENTER(&guard, m, multi_info_read, NULL)) {
	struct Curl_multi *multi = m;
-	if(Curl_llist_count(&multi->msglist)) {
-	/* there is one or more messages in the list */
-	struct Curl_llist_node *e;
-	struct Curl_message *msg;
-
-	/* extract the head of the list to return */
-	e = Curl_llist_head(&multi->msglist);
-
-	msg = Curl_node_elem(e);
-
-	/* remove the extracted entry */
-	Curl_node_remove(e);
-
-	*msgs_in_queue = curlx_uztosi(Curl_llist_count(&multi->msglist));
+	uint32_t mid;
+	if(Curl_uint32_bset_first(&multi->msgsent, &mid)) {
+	struct Curl_easy *data = Curl_multi_get_easy(multi, mid);

-	msg_result = &msg->extmsg;
+	DEBUGASSERT(data);
+	Curl_uint32_bset_remove(&multi->msgsent, mid);
+	*msgs_in_queue =
+	curlx_uztosi(Curl_uint32_bset_count(&multi->msgsent));
+	if(data)
+	msg_result = &data->msg;
	}
	}
	CURL_MAPI_LEAVE(&guard);

lib/multihandle.h+3 -10

@@ -41,12 +41,6 @@
	struct connectdata;
	struct Curl_easy;

-	struct Curl_message {
-	struct Curl_llist_node list;
-	/* the 'CURLMsg' is the part that is visible to the external user */
-	struct CURLMsg extmsg;
-	};
-
	/* NOTE: if you add a state here, add the name to the statenames[] array
	* in curl_trc.c as well!
	*/
@@ -103,11 +97,12 @@ struct Curl_multi {
	curl_off_t xfers_total_ever; /* total of added transfers, ever. */

	struct uint32_tbl xfers; /* transfers added to this multi */
-	/* Each transfer's mid may be present in at most one of these */
+	/* A transfer's mid may be present in at most one of
+	* process/pending/msgsent. The dirty set may overlap with process. */
	struct uint32_bset process; /* transfer being processed */
	struct uint32_bset dirty; /* transfer to be run NOW, e.g. ASAP. */
	struct uint32_bset pending; /* transfers in waiting (conn limit etc.) */
-	struct uint32_bset msgsent; /* transfers done with message for application */
+	struct uint32_bset msgsent; /* transfers with unread application messages */

	struct Curl_mapi_stack callstack; /* multi api calls ongoing */

@@ -116,8 +111,6 @@ struct Curl_multi {
	/* expiration times for all attached easy handles */
	struct Curl_timeouts timeouts;

-	struct Curl_llist msglist; /* a list of messages from completed transfers */
-
	struct Curl_easy admin; / internal easy handle for admin operations.
	gets assigned `mid` 0 on multi init */

lib/urldata.h+1 -1

@@ -1179,7 +1179,7 @@ struct Curl_easy {
	struct Curl_multi multi_easy; / if non-NULL, points to the multi handle
	struct to which this "belongs" when used
	by the easy interface */
-	struct Curl_message msg; /* A single posted message. */
+	struct CURLMsg msg; /* A single posted message. */

	/* once an easy handle is tied to a connection pool a non-negative number to
	distinguish this transfer from other using the same pool. For easier

tests/data/Makefile.am+1 -0

@@ -1960,6 +1960,7 @@ TESTCASES = \
	test2412 \
	test2413 \
	test2414 \
+	test2453 \
	\
	test2500 \
	test2501 \

tests/data/test2453+52 -0

@@ -0,0 +1,52 @@
+	<?xml version="1.0" encoding="US-ASCII"?>
+	<testcase>
+	<info>
+	<keywords>
+	HTTP
+	HTTP GET
+	CURLMINFO_XFERS_DONE
+	curl_multi_info_read
+	</keywords>
+	</info>
+
+	# Server-side
+	<reply>
+	<data crlf="headers" nocheck="yes">
+	HTTP/1.1 200 OK
+	Date: Tue, 09 Nov 2010 14:49:00 GMT
+	Server: test-server/fake
+	Content-Length: 0
+	Connection: close
+
+	</data>
+	</reply>
+
+	# Client-side
+	<client>
+	<server>
+	http
+	</server>
+	<features>
+	http
+	</features>
+	<name>
+	CURLMINFO_XFERS_DONE tracks unread completion messages
+	</name>
+	<tool>
+	lib%TESTNUMBER
+	</tool>
+	<command>
+	http://%HOSTIP:%HTTPPORT/%TESTNUMBER
+	</command>
+	</client>
+
+	# Verify data after the test has been "shot"
+	<verify>
+	<protocol crlf="headers">
+	GET /%TESTNUMBER HTTP/1.1
+	Host: %HOSTIP:%HTTPPORT
+	Accept: /
+
+	</protocol>
+	</verify>
+	</testcase>

tests/libtest/Makefile.inc+1 -0

@@ -306,6 +306,7 @@ TESTS_C = \
	lib2405.c \
	lib2412.c \
	lib2414.c \
+	lib2453.c \
	lib2502.c \
	lib2504.c \
	lib2505.c \

tests/libtest/lib2453.c+118 -0

@@ -0,0 +1,118 @@
+	/***************************************************************************
+	* _ _ ____ _
+	* Project ___\| \| \| \| _ \\| \|
+	* / __\| \| \| \| \|_) \| \|
+	* \| (__\| \|_\| \| _ <\| \|___
+	* \___\|\___/\|_\| \_\_____\|
+	*
+	* Copyright (C) Daniel Stenberg, <[email protected]>, et al.
+	*
+	* This software is licensed as described in the file COPYING, which
+	* you should have received as part of this distribution. The terms
+	* are also available at https://curl.se/docs/copyright.html.
+	*
+	* You may opt to use, copy, modify, merge, publish, distribute and/or sell
+	* copies of the Software, and permit persons to whom the Software is
+	* furnished to do so, under the terms of the COPYING file.
+	*
+	* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+	* KIND, either express or implied.
+	*
+	* SPDX-License-Identifier: curl
+	*
+	***************************************************************************/
+	#include "first.h"
+
+	static CURLcode expect_xfers_done(CURLM *multi, curl_off_t expected)
+	{
+	curl_off_t value = -1;
+	CURLMcode mresult = curl_multi_get_offt(multi, CURLMINFO_XFERS_DONE,
+	&value);
+
+	if(mresult != CURLM_OK) {
+	curl_mfprintf(stderr, "curl_multi_get_offt() failed with code %d\n",
+	(int)mresult);
+	return TEST_ERR_MULTI;
+	}
+	if(value != expected) {
+	curl_mfprintf(stderr,
+	"Expected %" CURL_FORMAT_CURL_OFF_T
+	" unread completion messages, got %"
+	CURL_FORMAT_CURL_OFF_T "\n",
+	expected, value);
+	return TEST_ERR_FAILURE;
+	}
+	return CURLE_OK;
+	}
+
+	static CURLcode test_lib2453(const char *URL)
+	{
+	CURL *curl = NULL;
+	CURLM *multi = NULL;
+	CURLcode result = CURLE_OK;
+	CURLMsg *msg;
+	int msgs_left;
+	int still_running;
+
+	start_test_timing();
+	global_init(CURL_GLOBAL_ALL);
+	multi_init(multi);
+	easy_init(curl);
+
+	result = expect_xfers_done(multi, 0);
+	if(result)
+	goto test_cleanup;
+
+	easy_setopt(curl, CURLOPT_URL, URL);
+	multi_add_handle(multi, curl);
+
+	do {
+	CURLMcode mresult;
+	int numfds;
+
+	multi_perform(multi, &still_running);
+	if(!still_running)
+	break;
+
+	mresult = curl_multi_poll(multi, NULL, 0, TEST_HANG_TIMEOUT, &numfds);
+	if(mresult != CURLM_OK) {
+	curl_mfprintf(stderr, "curl_multi_poll() failed with code %d\n",
+	(int)mresult);
+	result = TEST_ERR_MULTI;
+	goto test_cleanup;
+	}
+	abort_on_test_timeout();
+	} while(still_running);
+
+	result = expect_xfers_done(multi, 1);
+	if(result)
+	goto test_cleanup;
+
+	msg = curl_multi_info_read(multi, &msgs_left);
+	if(!msg \|\| msg->msg != CURLMSG_DONE \|\| msg->easy_handle != curl \|\|
+	msg->data.result != CURLE_OK \|\| msgs_left) {
+	curl_mfprintf(stderr, "Unexpected completion message\n");
+	result = TEST_ERR_FAILURE;
+	goto test_cleanup;
+	}
+
+	result = expect_xfers_done(multi, 0);
+	if(result)
+	goto test_cleanup;
+
+	multi_remove_handle(multi, curl);
+	multi_perform(multi, &still_running);
+	if(still_running) {
+	curl_mfprintf(stderr, "Completed transfer still counted as running\n");
+	result = TEST_ERR_FAILURE;
+	}
+
+	test_cleanup:
+	if(multi && curl)
+	curl_multi_remove_handle(multi, curl);
+	curl_easy_cleanup(curl);
+	curl_multi_cleanup(multi);
+	curl_global_cleanup();
+
+	return result;
+	}