cli: respect -rpcclienttimeout even when sending #36676

pull darosior wants to merge 1 commits into bitcoin:master from darosior:2610_cli_socket_error_fix changing 1 files +2 −1

darosior commented at 4:38 PM on October 27, 2026: member

The move away from libevent introduced a small regression in this regard. In #36299, we switched the timeout to apply per socket wait, but continued on to another socket wait in case of timeout, instead of returning. See commit message for details.

In general, we also shouldn't ignore Sock::Wait's errors and timeouts by silently retrying forever.

Fix these issues by returning instead when Sock::Wait times out or returns an error. ~~Tentative bot functional test for this in second commit that appears to be robust enough.~~

cli: fail to send a request when the socket does not become writable

HTTPClient::SendRequest() waits for the socket to become writable and
went back to waiting whenever Sock::Wait() returned without SendEvent.
Before 1805716354, the `continue` was the route to the deadline check. That
commit removed the deadline, so a Sock::Wait() timeout silently started
waiting again, forever. This means -rpcclienttimeout is never respected
when sending: bitcoin-cli will hang for as long as the server keeps the
connection open without reading the request.

Fix this by returning a failure whenever `Sock::Wait()` timeouts or errors.

e5d9a7e04b

DrahtBot added the label Scripts and tools on Oct 27, 2026
DrahtBot commented at 4:38 PM on October 27, 2026: contributor


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



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36676
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36676.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36676#author-ralph


Reviews

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

Type Reviewers

ACK jpk68, l0rinc, pinheadmz

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
darosior force-pushed on Oct 27, 2026
DrahtBot added the label CI failed on Oct 27, 2026
darosior commented at 5:18 PM on October 27, 2026: member

Tentative bot functional test for this in second commit that appears to be robust enough.

Well, scratch that. It's flaky on Windows. I'm dropping it from this PR: reviewers can easily bot-implement one locally to sanity check the behaviour. The only robust alternative would be a unit test, but would require some refactoring. That's overkill for this PR's trivial fix to the CLI.
DrahtBot removed the label CI failed on Oct 27, 2026
jpk68 commented at 8:36 PM on October 27, 2026: contributor

ACK e5d9a7e04b50c723aeb65984e5048ffd098f6737
l0rinc commented at 9:15 PM on October 27, 2026: contributor

tested ACK e5d9a7e04b50c723aeb65984e5048ffd098f6737

I also had it in my backlog (came up a few weeks ago when I was reviewing a different change - cc: @maflcko). Tested locally that a stalled upload reports a send failure and that the provided fix is correct.
pinheadmz approved
pinheadmz commented at 3:12 PM on October 28, 2026: member
ACK e5d9a7e04b50c723aeb65984e5048ffd098f6737

Built and tested on macos/arm64.

The undesired behavior is not just an ordinary timeout while waiting for a server reply, but a timeout while waiting for the socket to be writable on the client end. Even if no server exists at all, the client's kernel's TCP stack may still happily receive bytes with no where to send them.

To reproduce I ran a trivial accept-and-do-not-read server and made bitcoin-cli connections to it. The only way to garuntee the "wait forever" behavior is to send an excessive amount of data with bitcoin-cli and trigger TCP backpressure so the socket actually becomes not writable.

On master, in that case, we continue to wait forever. With this patch, we quit with (Failed to send HTTP request).

One thing thats not exactly clear in the PR description or commit message is that a timeout is NOT treated as an error. select() and poll() return 0 on timeout, and so Wait() and WaitMany() still return true. On master, we already quit on error. This PR also quits on timeout.

<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK e5d9a7e04b50c723aeb65984e5048ffd098f6737
-----BEGIN PGP SIGNATURE-----

iQJPBAEBCAA5FiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmriD40bFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMCwzAAoJEOfimEtiick67R8P/2nB0EV+mZEP0XTYb0VK
6IyxFDmrv1/NS37TNO84KjEM2L3Ff60+8pQZ/rCZ/ZMIS7BOZIb9dSEJb2OH2kvW
W8I8anYFjsKYy++ZN8fn4iQMlkYp1MjhGt2RzVPx4tYJpcTjCJmM+tstb2WHikvl
W/vnzmHfJpl3dX29A5q9d+cf6Ks4CN1hWGQfAJtiBIfD+snYzaeiRV5aI75fPb6N
g38jlkiBulIOPuVd1LbdxgyY0oT4L9///1qCFHMyHJhva9NAfWFaIzvnVD1dCZPw
ocSyhiBvITVLMXzxm86eKL1CX3vfg+z6nB1Q5jMNhmTOs13bTWrJGI40WcFo1G5C
HVeqciuDIByDSAIr78hrLpTpH5+itgZQRWM9tTgcI6o/OTodF8ejsexynOGedCtS
aYfnt6KjBk2T+x5BN5u0jbsswqoEuLfiLwEKwQc0DCLDncQAHOfUn90FiAjcEP86
go9oCA92i9TBaLHruKXdqthLyIRTEHacG117rcRTShd9tysyeci3I2LeoNzwTwNx
XOjD6ctuB0xOs2rOLTVQnKuF3D+9heCkuSu+Vh2zT8A3LRC53oF5+5PDcZW+fELN
VX+dqBKWMUaBSaJD1e84i94I9GdJ2mfeDYkR5j1GZ+r7imkQAZP9C+SjzgojaXBv
95ycWHMvmPgo+jEZPFTI/nQX
=ll+o
-----END PGP SIGNATURE-----
```
pinheadmz's public key is on openpgp.org

</details>
fanquake merged this on Oct 28, 2026
fanquake closed this on Oct 28, 2026
darosior deleted the branch on Oct 28, 2026

Type	Reviewers
ACK	jpk68, l0rinc, pinheadmz

Contributors

Labels

Linked (view graph)

#36299 cli: Improve empty-response and fix -rpcclienttimeout regression

cli: respect -rpcclienttimeout even when sending #36676

External sites

Reviews