util: cap Sock::WaitMany timeout to fix -rpcclienttimeout=0 on macOS #36340

1. 
   kriss39 commented at 9:40 AM on September 26, 2026: contributor

   bitcoin-cli -rpcclienttimeout=0 is documented as "no timeout", but on macOS it fails with "Could not connect to the server" since #34342 (it's in v32.0rc1/rc2).

   bitcoin-cli turns 0 into std::chrono::years(5) and passes it to Sock::Wait. On platforms without USE_POLL (everything except Linux) WaitMany uses select(), and macOS returns EINVAL for timeouts above 10^8 seconds. Five years is about 1.58 * 10^8 s. The same happens with any explicit value above 10^8, e.g. -rpcclienttimeout=110000000. On Linux, poll() takes the timeout as an int, so the millisecond count gets narrowed. For five years this happens to wrap to a negative value, which means infinite, so it works there by accident.

   This caps the timeout in Sock::WaitMany at INT_MAX milliseconds (about 24.8 days). That fits poll() and is below the 31 days POSIX requires select() to support. Doing it in WaitMany covers every caller, not only bitcoin-cli.

   The new sock_tests/wait_large_timeout test waits with a 5 year timeout on a socket that already has data. It fails on master on macOS and passes with this change. I also checked by hand on macOS that -rpcclienttimeout=0, 157784760 and 2147483647 work against a running node, and interface_bitcoin_cli.py still passes.

   This doesn't overlap with #36299, which changes how the timeout is counted but still passes the 5 year value down.

2. DrahtBot added the label Utils/log/libs on Sep 26, 2026
3. 
   DrahtBot commented at 9:40 AM on September 26, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36340.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   Approach ACK	winterrdog

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #36299 (cli: Improve empty-response and fix -rpcclienttimeout regression by fjahr)

   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.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. util: cap Sock::WaitMany timeout

   bitcoin-cli turns -rpcclienttimeout=0 into a 5 year timeout and passes
   it down to Sock::Wait. On systems without poll() (e.g. macOS and the
   BSDs) the wait is done with select(), which fails with EINVAL for
   timeouts above 10^8 seconds on macOS. So since the libevent removal in
   #34342, `bitcoin-cli -rpcclienttimeout=0 ...` fails on macOS with
   "Could not connect to the server". With poll(), the millisecond count
   is narrowed to int, so large values can wrap as well.
   
   Cap the timeout at INT_MAX milliseconds (about 24.8 days), which fits
   poll() and stays below the 31 days that POSIX requires select() to
   accept.

   a9d2316b50
5. kriss39 force-pushed on Sep 26, 2026
6. DrahtBot added the label CI failed on Sep 26, 2026
7. DrahtBot removed the label CI failed on Sep 26, 2026
8. 
   winterrdog commented at 1:46 PM on September 26, 2026: contributor

   concept ACK

   ---

   could we handle the "no timeout" case using the OS primitives' native indefinite timeout, while keeping this cap for explicitly specified large finite timeouts ?

   since poll() supports a negative timeout and select() can take NULL, that would preserve the -rpcclienttimeout=0 semantics without removing the protection against very large finite values

   references:

   • Linux poll(2)
   • FreeBSD select(2)
   • OpenBSD select(2)
   • NetBSD select(2)
   • Windows select()

   [!NOTE] i do not have an Apple machine to check the macOS local manpages. so macOS users can just check the authoritative local versions with man 2 select and man 2 poll

9. 
   kriss39 commented at 2:47 PM on September 26, 2026: contributor

   thanks! good call, pushed a second commit that does this.

   you're right that the cap alone isn't really "no timeout". HTTPClient::Recv treats any wait that comes back empty as a timeout, so -rpcclienttimeout=0 would have given up after ~24.8 days of silence. I doubt anyone is going to keep a waitfornewblock open for three and a half weeks, but "no timeout" should probably mean no timeout.

   fwiw the 5 years was never really "no timeout" either. it's a leftover from libevent, which just couldn't wait indefinitely (b3b26e149c).

   I do have a Mac on my desk, so here's what the man pages say: select(2) "blocks indefinitely" with a NULL timeout and returns EINVAL if the timeout is "negative or too large". poll(2) blocks indefinitely with -1. same as the BSDs.

   what the new commit does:

   • adds Sock::NO_TIMEOUT (milliseconds::max()), which WaitMany passes as -1 to poll() and nullptr to select()
   • bitcoin-cli uses it for -rpcclienttimeout=0 instead of 5 years
   • the cap from the first commit stays for large finite values like -rpcclienttimeout=110000000

   added sock_tests/wait_no_timeout. I also checked by hand on macOS that -rpcclienttimeout=0 waitfornewblock waits until a block comes in, and that -rpcclienttimeout=1 still times out.

   this will conflict with #36299 since both touch the deadline code in cli. whichever goes in second gets a small rebase.

10. maflcko added this to the milestone 32.0 on Sep 28, 2026
11. maflcko added the label Needs Backport (32.x) on Sep 28, 2026
12. maflcko added the label macOS on Sep 28, 2026
13. 
    maflcko commented at 7:32 AM on September 28, 2026: member

    I wonder how much we want to backport here. I doubt there is a use case to have a timeout of more than a few hours, so capping to 1 day, or even 14 days, or the first commit should be fine for a backport?

14. kriss39 force-pushed on Sep 28, 2026
15. 
    winterrdog commented at 7:56 AM on September 28, 2026: contributor

    capping to 1 day

    I agree. The second commit might've gone overboard

16. 
    kriss39 commented at 7:56 AM on September 28, 2026: contributor

    makes sense, dropped the second commit so this is just the cap in Sock::WaitMany and should be easy to backport. that fixes both -rpcclienttimeout=0 and large explicit values; 0 now effectively means ~24.8 days, which as you say is way beyond any real use. @winterrdog the native infinite wait (poll -1 / select NULL) can go in a follow-up if people still want it.

17. 
    winterrdog commented at 8:06 AM on September 28, 2026: contributor

    the native infinite wait (poll -1 / select NULL) can go in a follow-up if people still want it.

    A protracted wait timeout is simple and does the job fine. So, no need to get dogmatic about native infinite wait.

    Approach ACK

Milestone
32.0