diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 3f271da389e..dd17754bfac 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -996,13 +995,13 @@ void HTTPServer::ThreadSocketHandler() void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr& client) const { - // Try reading (potentially multiple) HTTP requests from the buffer - while (!client->m_recv_buffer.empty()) { + // Try reading the next HTTP request from the buffer + if (!client->m_req) { // Create a new request object and try to fill it with data from the receive buffer auto req = std::make_unique(client); try { // Stop reading if we need more data from the client to parse a complete request - if (!client->ReadRequest(*req)) break; + if (!client->ReadRequest(*req)) return; } catch (const ContentTooLargeError& e) { LogDebug( BCLog::HTTP, @@ -1037,8 +1036,8 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptrm_origin, client->m_id); - // add request to client queue - client->m_req_queue.push_back(std::move(req)); + // Move request to client + client->m_req = std::move(req); } // If we are already handling a request from @@ -1046,12 +1045,11 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptrm_req_busy) return; - // Otherwise, if there is a pending request in the queue, handle it. - if (!client->m_req_queue.empty()) { + // Otherwise, if there is a request ready to go, handle it. + if (client->m_req) { LOCK(m_request_dispatcher_mutex); client->m_req_busy = true; - m_request_dispatcher(std::move(client->m_req_queue.front())); - client->m_req_queue.pop_front(); + m_request_dispatcher(std::move(client->m_req)); } } @@ -1102,6 +1100,7 @@ void HTTPServer::DisconnectClients() "Disconnecting HTTP client %s (id=%llu)", client->m_origin, client->m_id); + client->ReleaseRequest(); return true; }); if (erased > 0) { @@ -1116,6 +1115,9 @@ void HTTPServer::ClearConnectedClients() if (m_connected.empty()) return; LogWarning("Force-disconnecting %d HTTP client(s) that did not disconnect gracefully", m_connected.size()); m_connected_size.fetch_sub(m_connected.size(), std::memory_order_relaxed); + for (auto& client : m_connected) { + client->ReleaseRequest(); + } m_connected.clear(); } diff --git a/src/httpserver.h b/src/httpserver.h index 943dc12def4..1ae4a5e5ee5 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -6,7 +6,6 @@ #define BITCOIN_HTTPSERVER_H #include -#include #include #include #include @@ -479,10 +478,9 @@ public: std::string 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> m_req_queue; + //! they were received, blocking on a per-client basis. We read + //! one request at a time from the socket buffer then pass it to a worker. + std::unique_ptr m_req; //! 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. @@ -555,6 +553,12 @@ public: HTTPRemoteClient(const HTTPRemoteClient&) = delete; HTTPRemoteClient& operator=(const HTTPRemoteClient&) = delete; + //! Release any in-progress request. HTTPRequest holds a shared_ptr back to its + //! HTTPRemoteClient to keep the client alive from a worker thread. If a request + //! hasn't been moved to a worker yet it will prevent the client from destructing + //! and never close the socket. Therefore this must be called when disconnecting. + void ReleaseRequest() { m_req.reset(); } + /** * Try to read an HTTP request from the receive buffer. * @param[in] req A HTTPRequest to read into