multi: remove redundant completion message list

[cmeister2]

Use the msgsent bitset to track unread application messages and scan it
from curl_multi_info_read(). Remove entries when read or when their easy
handle is removed.

This removes the linked-list node from each easy handle. Completion order
is not API-defined.

Add regression coverage for CURLMINFO_XFERS_DONE.

Closes #22805

[Copilot]

🟢 Approval recommended

The implementation is consistent with the API contract and includes focused regression coverage.

Pull request overview

Removes redundant completed-transfer tracking while aligning CURLMINFO_XFERS_DONE with unread completion messages.

Changes:

• Uses MSTATE_MSGSENT and the message queue for completion accounting.
• Removes the msgsent bitset and related allocation logic.
• Adds regression coverage and updates internal documentation.

File summaries

File	Description
lib/multi.c	Simplifies completion tracking and counts queued messages.
lib/multihandle.h	Removes the redundant bitset.
docs/internals/MID.md	Updates transfer-set documentation.
tests/libtest/lib2453.c	Tests completion-message accounting.
tests/data/test2453	Defines the regression scenario.
tests/libtest/Makefile.inc	Registers the test source.
tests/data/Makefile.am	Registers the test case.

Review details

• Files reviewed: 7/7 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]

What about keeping the bitset but doing away with multi->msglist? That would also shrink the easy handle some more.

[cmeister2]

What about keeping the bitset but doing away with multi->msglist? That would also shrink the easy handle some more.

When codex had a look it said "@icing's “bitset only” variant is not behavior-neutral: it would naturally return completions by handle ID instead of completion order, and it would turn the deliberately O(1) info-read path into a bitset scan".

If that's a thing we are ok with then it's a possibility.

[icing]

The order in which finished transfers are returned is not defined anywhere. Just because the time of finishing is determined by the network/server.

One could possibly construct a scenerio where an application after having seen something in a callback, expects the multi info read to return exactly that transfer. Possibly. But that is brittle code, likely to fail on some other future change as well.

The bitset scan can be more costly, yes. Not by much. But the list manipulations also cost, which the bitset makes cheaper. The cleanup on removing a handle is also cheaper.

Personally, I prefer having less pointer lists in easy management.

Update: rechecked the uint-bset.c code and it has optimizations for checking/counts on empty bitsets, which is the common case for the pending set.

[Copilot]

🟡 Changes recommended

Counting the bitset for every message makes draining large completion queues effectively quadratic.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Balanced

[icing]

Not quite true. More like O(log(N)) as the firstused slot index will increase.

[cmeister2]

@icing : Curl_uint32_bset_count doesn't use the first slot index:

uint32_t Curl_uint32_bset_count(struct uint32_bset *bset)
{
  uint32_t i;
  uint32_t n = 0;
  for(i = 0; i < bset->nslots; ++i) {
    if(bset->slots[i])
      n += CURL_POPCOUNT64(bset->slots[i]);
  }
  return n;
}

I'm having a look to see if it can use first_slot_used as part of that calc.

[icing]

Oops, my mistake. Why doesn't it though...hmm....should work, I think.

[cmeister2]

🤖

Historically, count() predates first_slot_used; that field was later added specifically to optimize empty() and first(). There is no correctness reason for counting from zero—it just was not updated.

[icing]

[cmeister2]

Ok, have made that fix. It doesn't resolve the quadratic worst case but realistically we're not going to be hitting that.

We could add a cached count to uint32_bset but that would likely grow the structure from 16 to 24 bytes.

[cmeister2]

Alternatively adding a count alongside the bitset itself would only increase the count by 8 bytes (but would make the _count call pointless really)

[icing]

I doubt that this is worth it (but I am just guessing):

• on an empty bset, count does not loop at all
• on many platform, counting is one instruction per 64bit slot and the data subsequently is from the same cache line. The set would need to grow quite large for this to make a difference.

[cmeister2]

Agree.