test: ensure HTTPServer race condition is fixed
What the developer wrote
Authored by Matthew Zipkin
test: ensure HTTPServer race condition is fixed
The result of WriteReply() losing the race condition would prevent any
new requests being read from the socket. The socket error test
sent 3 requests all at once after connecting, so in this commit
we separate the the third request to make the losing race
condition more likely, and make its effect more obvious.
What changed, and why it matters
This commit only changes a test file. It strengthens an existing test for the Bitcoin Core HTTP server so it can better catch a race condition where the server might stop reading new requests from a connection. The commit itself does not change the actual server code, so it is not a fix—it is a test that checks whether a previously suspected race condition is truly fixed.
Treat this as a test-hardening commit. If the race condition it targets has not already been fixed in the HTTP server implementation, a separate production-code fix should be identified and applied. Reviewers should confirm the related WriteReply()/SocketHandlerConnected() race was resolved before this test was added, and consider running the updated test under thread sanitizers or stress runs.
Security signals we found
Race condition in HTTP server request handling
Potential denial-of-service symptom: server stops reading from a socket
Test-only change, not a production fix
References prior race between WriteReply() and SocketHandlerConnected()
Evidence from the diff
The diff modifies src/test/httpserver_tests.cpp in the http_socket_error_tests test case. It splits the third HTTP keep-alive request out from the initial burst of three requests, waits for the first two replies, pauses one second to let worker and I/O threads quiesce, then sends the third request. The test now waits up to one minute (6000 attempts × 10 ms) for replies. The commit message says the goal is to make a race between WriteReply() setting m_send_ready=true and SocketHandlerConnected() flushing the send buffer more likely to be detected: if the race is lost, the socket gets stuck in write mode, the server stops reading, and the third request times out. No production code is changed.
Changed components
src/test/httpserver_tests.cppHTTP server test suiteInspect captured patch +29 / −2
diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp
index ff1eb22d..aae89920 100644
--- a/src/test/httpserver_tests.cpp
+++ b/src/test/httpserver_tests.cpp
@@ -697,7 +697,7 @@ BOOST_AUTO_TEST_CASE(http_socket_error_tests)
server.StartSocketsThreads();
// Prepare initial requests
- int num_requests = 3;
+ int num_requests = 2;
// Use keep-alive so the server holds the connection open for all requests.
std::string keepalive_request{full_request};
keepalive_request.replace(keepalive_request.find("Connection: close"), 17, "Connection: keep-alive");
@@ -730,7 +730,34 @@ BOOST_AUTO_TEST_CASE(http_socket_error_tests)
// Wait up to one minute for the last reply from the server
std::string actual;
char buf[0x10000] = {};
- int attempts = 1000;
+ int attempts = 6000;
+ while (attempts > 0)
+ {
+ ssize_t bytes_read = mock_client_socket_pipes->send.GetBytes(buf, sizeof(buf), 0);
+ if (bytes_read > 0) {
+ actual.append(buf, bytes_read);
+ if (actual.find(strprintf("height: %d", num_requests - 1)) != std::string::npos) {
+ break;
+ }
+ }
+ std::this_thread::sleep_for(10ms);
+ --attempts;
+ }
+
+ // Send the third request.
+ // If there was a race between WriteReply() in the worker thread setting m_send_ready=true
+ // and SocketHandlerConnected() in the I/O thread flushing the send buffer,
+ // then the socket would be stuck in write mode with nothing to write,
+ // the server would never read from the socket, and this request would time out.
+ // Wait a second to ensure both the worker thread and I/O thread are idle.
+ // If we send the next request too soon it might get accepted by the server before
+ // it gets wedged shut.
+ std::this_thread::sleep_for(1000ms);
+ mock_client_socket_pipes->recv.PushBytes(keepalive_request.data(), keepalive_request.size());
+ num_requests++;
+
+ // Wait up to one minute for reply
+ attempts = 6000;
while (attempts > 0)
{
ssize_t bytes_read = mock_client_socket_pipes->send.GetBytes(buf, sizeof(buf), 0);
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.