refactor: rename EventLoop::m_num_clients to m_num_refs #302

pull Sjors wants to merge 1 commits into bitcoin-core:master from Sjors:2026/07/num-clients changing 3 files +15 −11

Sjors commented at 1:46 PM on July 2, 2026: member

Rename the variable and document what it counts, so the loop() exit condition in EventLoop::done() is not misread as "exit when no clients are connected".

I found myself confused by this while reviewing #269.

refactor: rename EventLoop::m_num_clients to m_num_refs

Rename the variable and document what it counts, so the loop() exit
condition in EventLoop::done() is not misread as "exit when no
clients are connected".

d499830415

DrahtBot commented at 1:46 PM on July 2, 2026: none


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK ViniciusCestarii, ryanofsky

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #288 (Create support branch for CI scripts, documentation, and examples by ryanofsky)
- #274 (Add nonunix platform support by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
in include/mp/proxy-io.h:258 in d499830415
```
 254 | @@ -255,7 +255,7 @@ class EventLoop
 255 |  
 256 |      //! Run event loop. Does not return until shutdown. This should only be
 257 |      //! called once from the m_thread_id thread. This will block until
 258 | -    //! the m_num_clients reference count is 0.
 259 | +    //! the m_num_refs reference count is 0.
```
ViniciusCestarii commented at 2:09 PM on July 3, 2026:

In "refactor: rename EventLoop::m_num_clients to m_num_refs" d499830415405ba442f597cb06ed4c56442f774a

nit: "the m_num_refs reference count" is a bit redundant now because "refs" already means references. Could be renamed to "the m_num_refs count is 0" (unrelated but this condition is also incomplete: loop() exits on done(), m_num_refs == 0 and m_async_fns is empty).
ViniciusCestarii commented at 2:12 PM on July 3, 2026: contributor

ACK d499830415405ba442f597cb06ed4c56442f774a, I agree this name is clearer.
ryanofsky approved
ryanofsky commented at 8:37 PM on July 5, 2026: collaborator

Code review ACK d499830415405ba442f597cb06ed4c56442f774a. Thanks for the rename and comment!

This was previously discussed #107 (comment)

Probably the word "client" should be replaced with "use count" here (in num_clients addClient removeClient). The word "client" is supposed to mean "client of the event loop" but that seems unnecessarily confusing because the word client is overloaded so much.

And #107 (comment)

I didn't find this that confusing to be honest, and just by looking at the names was assuming "client of the event loop".

But the word was used more places then so renaming is easier now and makes sense.
ryanofsky merged this on Jul 5, 2026
ryanofsky closed this on Jul 5, 2026

Type	Reviewers
ACK	ViniciusCestarii, ryanofsky

Contributors

Linked (view graph)

#269 proxy: add local connection limit to ListenConnections

refactor: rename EventLoop::m_num_clients to m_num_refs #302

Reviews

Conflicts