#36276 http: don't register RecvEvent while a request is in-flight
https://github.com/bitcoin/bitcoin/pull/36276 · · +40/-17 in 3 files, 2 commits · labels: RPC/REST/ZMQ, CI failed · draft
Goal
- Prevent the HTTP server from reading new data on a connection while an earlier request is in flight
- Stop socket buffer draining that triggers delayed TCP window probes and test timeouts on macOS
This PR updates the HTTP server event loop logic so that `RecvEvent` is not registered on a connection when a request from that connection is already being handled by a worker thread (`m_req_busy`). It also modifies `interface_http.py` to use `select()` before writing so tests do not hang on macOS when the kernel socket buffer fills up.
Problem: Under the existing read throttle implemented in #36123, an empty receive buffer would still allow the server to read up to 65kB from the socket even while an earlier request was in-flight. This drained the OS socket buffer, reopened the TCP window, and caused flaky timeout failures in `interface_http.py` on macOS due to delayed zero-window probing.
Category: RPC / REST / ZMQ (#18 of 52)
P3 · bug fix
- P3 because it fixes a minor edge case in HTTP socket read throttling
- Resolves an intermittent test timeout on macOS rather than an acute security or denial of service risk
- The worst-case socket buffer drain before this fix is limited to 65kB
Fixes a minor edge-case in HTTP socket read throttling and resolves an intermittent test failure in interface_http.py (#36216). As noted by the author, the worst-case buffer drain is only 65kB so this is not an acute OOM/DoS vulnerability, making P3 appropriate.
Membership: Modifies src/httpserver.cpp, src/httpserver.h, and the HTTP interface test interface_http.py.
Factors: security/stability 1, bug 1, performance 1, user value 0, leverage 1
Reviewability: Paused: Author reworking
- Author converted PR to draft to investigate a potential 50ms latency added to RPC calls
- Wait for author investigation and for failing CI to be resolved
The author converted the pull request to draft after noting that 50ms might be added to every RPC call, and CI is currently failing.
Author status: reworking (converted to draft to investigate latency impact)
Open concerns:
- The author converted the PR to draft after noticing that 50ms might be added to RPC calls.
Resolved concerns:
- davidgumberg verified the trace of the bug in `src/httpserver.cpp` and how the zero-window probe interaction manifests in the test.
Agreement: Positive
- Reviewers support the concept and diagnosis of the socket throttling gap
- Concept approval on the initial commit (hodlinator)
- Verified that the server drained kernel buffers instead of maintaining backpressure (davidgumberg)
Positive: hodlinator gave Concept ACK and davidgumberg confirmed the bug mechanism
Reviewers agree with the concept and the diagnosis of the throttling gap. No objections to the goal have been raised.
- hodlinator Concept ACKed the first commit.
- davidgumberg detailed the state transitions and confirmed that the server was draining the kernel buffer when it should have maintained backpressure.
Review verdicts (DrahtBot): 0
- Concept ACK: hodlinator
Files
37 lines under test/bench/ci.
- test/functional/interface_http.py +24/-13
- src/httpserver.cpp +13/-4
- src/httpserver.h +3/-0
Card
PR #36276 updates the HTTP server event loop to avoid registering RecvEvent while a request is in-flight on a worker thread, preventing an extra 65kB read that reopens the TCP window. This addresses intermittent failures in interface_http.py on macOS (#36216) following the throttling introduced in #36123. Reviewers agree with the diagnosis, but the author has moved the PR to draft to investigate an apparent 50ms latency regression in RPC calls.