From 8c6d007c80dc3fec5ce6c0196381444a5ed7e424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 30 Jun 2020 00:05:47 +0100 Subject: [PATCH 1/3] http: Track active requests and wait for last to finish --- src/httpserver.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 6f84d5c83bd..f49f2b3f37b 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -21,12 +21,14 @@ #include #include +#include #include #include #include #include #include #include +#include #include #include @@ -34,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -146,6 +149,10 @@ static GlobalMutex g_httppathhandlers_mutex; static std::vector pathHandlers GUARDED_BY(g_httppathhandlers_mutex); //! Bound listening sockets static std::vector boundSockets; +//! Track active requests +static GlobalMutex g_requests_mutex; +static std::condition_variable g_requests_cv; +static std::unordered_set g_requests GUARDED_BY(g_requests_mutex); /** Check if a network address is allowed to access the HTTP server */ static bool ClientAllowed(const CNetAddr& netaddr) @@ -207,6 +214,17 @@ std::string RequestMethodString(HTTPRequest::RequestMethod m) /** HTTP request callback */ static void http_request_cb(struct evhttp_request* req, void* arg) { + // Track requests and notify when a request is completed. + { + WITH_LOCK(g_requests_mutex, g_requests.insert(req)); + g_requests_cv.notify_all(); + evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) { + auto n{WITH_LOCK(g_requests_mutex, return g_requests.erase(req))}; + assert(n == 1); + g_requests_cv.notify_all(); + }, nullptr); + } + // Disable reading to work around a libevent bug, fixed in 2.2.0. if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02020001) { evhttp_connection* conn = evhttp_request_get_connection(req); @@ -459,6 +477,15 @@ void StopHTTPServer() evhttp_del_accept_socket(eventHTTP, socket); } boundSockets.clear(); + { + WAIT_LOCK(g_requests_mutex, lock); + if (!g_requests.empty()) { + LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", g_requests.size()); + } + g_requests_cv.wait(lock, []() EXCLUSIVE_LOCKS_REQUIRED(g_requests_mutex) { + return g_requests.empty(); + }); + } if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); if (g_thread_http.joinable()) g_thread_http.join(); From 660bdbf785a32024f0694915fa043968a0afb573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Sat, 3 Aug 2019 02:52:53 +0100 Subject: [PATCH 2/3] http: Release server before waiting for event base loop exit --- src/httpserver.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index f49f2b3f37b..5c1bbc87fcf 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -486,15 +486,18 @@ void StopHTTPServer() return g_requests.empty(); }); } + if (eventHTTP) { + // Schedule a callback to call evhttp_free in the event base thread, so + // that evhttp_free does not need to be called again after the handling + // of unfinished request connections that follows. + event_base_once(eventBase, -1, EV_TIMEOUT, [](evutil_socket_t, short, void*) { + evhttp_free(eventHTTP); + eventHTTP = nullptr; + }, nullptr, nullptr); + } if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); if (g_thread_http.joinable()) g_thread_http.join(); - } - if (eventHTTP) { - evhttp_free(eventHTTP); - eventHTTP = nullptr; - } - if (eventBase) { event_base_free(eventBase); eventBase = nullptr; } From 60978c8080ec13ff4571c8a89e742517b2aca692 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 5 Jan 2023 12:56:19 +0100 Subject: [PATCH 3/3] test: Reduce extended timeout on abortnode test This was made obsolete by tracking the active requests and explicitly waiting for them to finish before shutdown. --- test/functional/feature_abortnode.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py index 32cf4a47f48..fa1bb6506ab 100755 --- a/test/functional/feature_abortnode.py +++ b/test/functional/feature_abortnode.py @@ -19,7 +19,6 @@ class AbortNodeTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - self.rpc_timeout = 240 def setup_network(self): self.setup_nodes() @@ -41,7 +40,7 @@ class AbortNodeTest(BitcoinTestFramework): # Check that node0 aborted self.log.info("Waiting for crash") - self.nodes[0].wait_until_stopped(timeout=200) + self.nodes[0].wait_until_stopped(timeout=5) self.log.info("Node crashed - now verifying restart fails") self.nodes[0].assert_start_raises_init_error()