From a00dfd2046a9edd8fc4063971c2a5f9a444117e7 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Fri, 21 Feb 2025 14:58:29 +0100 Subject: [PATCH] http: Abort with clear error instead of hanging forever Could happen when connection doesn't complete for some reason. Output early warning to give clue about what's up. Prior check+log message before even starting to wait was a bit too eager to hint at something being wrong. --- src/httpserver.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index a3a6a8bea62..288d7c08fd6 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -197,10 +197,10 @@ public: return WITH_LOCK(m_mutex, return m_tracker.size()); } //! Wait until there are no more connections with active requests in the tracker - void WaitUntilEmpty() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + void WaitUntilEmpty(std::chrono::seconds timeout) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { WAIT_LOCK(m_mutex, lock); - m_cv.wait(lock, [this]() EXCLUSIVE_LOCKS_REQUIRED(m_mutex) { return m_tracker.empty(); }); + m_cv.wait_for(lock, timeout, [this]() EXCLUSIVE_LOCKS_REQUIRED(m_mutex) { return m_tracker.empty(); }); } }; //! Track active requests @@ -536,11 +536,19 @@ void StopHTTPServer() evhttp_del_accept_socket(eventHTTP, socket); } boundSockets.clear(); - { - if (const auto n_connections{g_requests.CountActiveConnections()}; n_connections != 0) { - LogDebug(BCLog::HTTP, "Waiting for %d connections to stop HTTP server\n", n_connections); + // Give clients time to close down TCP connections from their side before we + // free eventHTTP, this avoids issues like RemoteDisconnected exceptions in + // the test framework. + g_requests.WaitUntilEmpty(/*timeout=*/30s); + if (auto connections{g_requests.CountActiveConnections()}) { + LogWarning("%d remaining HTTP connection(s) after 30s timeout, waiting for longer.", connections); + g_requests.WaitUntilEmpty(/*timeout=*/10min); + if ((connections = g_requests.CountActiveConnections())) { + LogError("%d remaining HTTP connection(s) after long timeout. " + "Aborting to avoid potential use-after-frees from " + "connections still running after freeing eventHTTP.", connections); + std::abort(); } - g_requests.WaitUntilEmpty(); } if (eventHTTP) { // Schedule a callback to call evhttp_free in the event base thread, as