HTTPServer: use a queue to pipeline requests from each connected client

See https://www.rfc-editor.org/rfc/rfc7230#section-6.3.2

> A server MAY process a sequence of pipelined requests in
  parallel if they all have safe methods (Section 4.2.1 of [RFC7231]),
  but it MUST send the corresponding responses in the same order that
  the requests were received.

We choose NOT to process requests in parallel. They are executed in
the order recevied as well as responded to in the order received.
This prevents race conditions where old state may get sent in response
to requests that are very quick to process but were requested later on
in the queue.
This commit is contained in:
Matthew Zipkin
2025-04-03 12:28:42 -04:00
parent 5ef1b80a09
commit 7ee7df988e
2 changed files with 33 additions and 4 deletions

View File

@@ -1072,6 +1072,9 @@ void HTTPRequest::WriteReply(HTTPStatusCode status, std::span<const std::byte> r
// in the next loop iteration.
m_client->m_send_ready = true;
}
// Signal to the I/O loop that we are ready to handle the next request.
m_client->m_req_busy = false;
}
util::Expected<void, std::string> HTTPServer::BindAndStartListening(const CService& to)
@@ -1293,10 +1296,13 @@ void HTTPServer::SocketHandlerConnected(const IOReadiness& io_readiness) const
client->m_recv_buffer.end(),
buf,
buf + nrecv);
// Process as much received data as we can
MaybeDispatchRequestsFromClient(client);
}
}
// Process as much received data as we can.
// This executes for every client whether or not reading or writing
// took place because it also (might) parse a request we have already
// received and pass it to a worker thread.
MaybeDispatchRequestsFromClient(client);
}
}
@@ -1410,8 +1416,20 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr<HTTPRemot
client->m_origin,
client->m_id);
// handle request
m_request_dispatcher(std::move(req));
// add request to client queue
client->m_req_queue.push_back(std::move(req));
}
// If we are already handling a request from
// this client, do nothing. We'll check again on the next I/O
// loop iteration.
if (client->m_req_busy) return;
// Otherwise, if there is a pending request in the queue, handle it.
if (!client->m_req_queue.empty()) {
client->m_req_busy = true;
m_request_dispatcher(std::move(client->m_req_queue.front()));
client->m_req_queue.pop_front();
}
}

View File

@@ -6,6 +6,7 @@
#define BITCOIN_HTTPSERVER_H
#include <atomic>
#include <deque>
#include <functional>
#include <memory>
#include <optional>
@@ -538,6 +539,16 @@ public:
*/
std::vector<std::byte> m_recv_buffer{};
//! Requests from a client must be processed in the order in which
//! they were received, blocking on a per-client basis. We won't
//! process the next request in the queue if we are currently busy
//! handling a previous request.
std::deque<std::unique_ptr<HTTPRequest>> m_req_queue;
//! Set to true by the I/O thread when a request is popped off
//! and passed to a worker thread, reset to false by the worker thread.
std::atomic_bool m_req_busy{false};
/**
* Response data destined for this client.
* Written to by http worker threads, read and erased by HTTPServer I/O thread