From 6d9b61d4f8dc39081dc4e0acde36f54cb340807c Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:39:56 +0200 Subject: [PATCH] refactor: Extract HTTPRemoteClient::MaybeDisconnect() from HTTPServer::DisconnectClients() --- src/httpserver.cpp | 97 +++++++++++++++++++++++++--------------------- src/httpserver.h | 2 + 2 files changed, 54 insertions(+), 45 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index c94863cae33..24b89ce38f2 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -1080,57 +1080,64 @@ void HTTPServer::DisconnectClients() const auto now{Now()}; size_t erased = std::erase_if(m_connected, [&](auto& client) { - // First check for idle timeout. We reset the timer when we send and receive data, - // but if the server is busy handling a request we should ignore the timeout until - // the reply is sent. If we did erase the shared_ptr reference in m_connected - // while the server is busy with a request, it might be prematurely dropped before - // the response has been sent, or if the HTTPRequest was holding a temporary shared_ptr - // client on a worker thread - it would keep the socket open even after "disconnecting". - const bool is_idle{m_rpcservertimeout.count() > 0 && - now - client->m_idle_since.load() > m_rpcservertimeout && - !client->m_req_busy}; - - // Disconnect this client due to error, end of communication, or idle timeout. - // May drop unsent data if we are closing due to error. - if (client->m_disconnect || is_idle) { - if (is_idle) { - LogDebug(BCLog::HTTP, - "HTTP client idle timeout %s (id=%llu)", - client->m_origin, - client->m_id); - } - } else { - // Disconnect this client because the server is shutting - // down and we need to disconnect all clients... - if (m_disconnect_all_clients) { - // ...unless we still have data for this client. - if (client->m_connection_busy) { - // There is still data for this healthy-connected client. - // Continue the I/O loop until all data is sent or an error is encountered. - return false; - } else { - // This is a healthy persistent connection (e.g. keep-alive) - // but it's time to say goodbye. - ; - } - } else { - // No reason to disconnect. - return false; - } - } - // No reason NOT to disconnect, log and remove. - LogDebug(BCLog::HTTP, - "Disconnecting HTTP client %s (id=%llu)", - client->m_origin, - client->m_id); - return true; - }); + return client->MaybeDisconnect(now, + m_rpcservertimeout, + /*disconnect_all=*/m_disconnect_all_clients); + }); if (erased > 0) { // Report back to the main thread m_connected_size.fetch_sub(erased, std::memory_order_relaxed); } } +bool HTTPRemoteClient::MaybeDisconnect(std::chrono::time_point now, std::chrono::seconds rpcservertimeout, bool disconnect_all) +{ + // First check for idle timeout. We reset the timer when we send and receive data, + // but if the server is busy handling a request we should ignore the timeout until + // the reply is sent. If we did erase the shared_ptr reference in m_connected + // while the server is busy with a request, it might be prematurely dropped before + // the response has been sent, or if the HTTPRequest was holding a temporary shared_ptr + // client on a worker thread - it would keep the socket open even after "disconnecting". + const bool is_idle{rpcservertimeout.count() > 0 && + now - m_idle_since.load() > rpcservertimeout && + !m_req_busy}; + + // Disconnect this client due to error, end of communication, or idle timeout. + // May drop unsent data if we are closing due to error. + if (m_disconnect || is_idle) { + if (is_idle) { + LogDebug(BCLog::HTTP, + "HTTP client idle timeout %s (id=%llu)", + m_origin, + m_id); + } + } else { + // Disconnect this client because the server is shutting + // down and we need to disconnect all clients... + if (disconnect_all) { + // ...unless we still have data for this client. + if (m_connection_busy) { + // There is still data for this healthy-connected client. + // Continue the I/O loop until all data is sent or an error is encountered. + return false; + } else { + // This is a healthy persistent connection (e.g. keep-alive) + // but it's time to say goodbye. + ; + } + } else { + // No reason to disconnect. + return false; + } + } + // No reason NOT to disconnect, log and remove. + LogDebug(BCLog::HTTP, + "Disconnecting HTTP client %s (id=%llu)", + m_origin, + m_id); + return true; +} + void HTTPServer::ClearConnectedClients() { Assume(!m_thread_socket_handler.joinable()); // must be called after JoinSocketsThreads() diff --git a/src/httpserver.h b/src/httpserver.h index ce2bd4eaac2..af4039cf55e 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -580,6 +580,8 @@ public: HTTPRemoteClient(const HTTPRemoteClient&) = delete; HTTPRemoteClient& operator=(const HTTPRemoteClient&) = delete; + bool MaybeDisconnect(std::chrono::time_point now, std::chrono::seconds rpcservertimeout, bool disconnect_all); + /** * Try to read an HTTP request from the receive buffer. * Updates HTTPRequest.m_state and drains buffer on error.