diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 5b062d65d97c..fc0132133ff3 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -14,6 +14,7 @@ #include // For HTTP status codes #include #include +#include #include #include #include @@ -23,12 +24,14 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -162,6 +165,61 @@ static std::vector pathHandlers GUARDED_BY(g_httppathhandlers_m //! Bound listening sockets static std::vector boundSockets; +/** + * @brief Helps keep track of open `evhttp_connection`s with active `evhttp_requests` + * + */ +class HTTPRequestTracker +{ +private: + mutable Mutex m_mutex; + mutable std::condition_variable m_cv; + //! For each connection, keep a counter of how many requests are open + std::unordered_map m_tracker GUARDED_BY(m_mutex); + + void RemoveConnectionInternal(const decltype(m_tracker)::iterator it) EXCLUSIVE_LOCKS_REQUIRED(m_mutex) + { + m_tracker.erase(it); + if (m_tracker.empty()) m_cv.notify_all(); + } +public: + //! Increase request counter for the associated connection by 1 + void AddRequest(evhttp_request* req) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + const evhttp_connection* conn{Assert(evhttp_request_get_connection(Assert(req)))}; + WITH_LOCK(m_mutex, ++m_tracker[conn]); + } + //! Decrease request counter for the associated connection by 1, remove connection if counter is 0 + void RemoveRequest(evhttp_request* req) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + const evhttp_connection* conn{Assert(evhttp_request_get_connection(Assert(req)))}; + LOCK(m_mutex); + auto it{m_tracker.find(conn)}; + if (it != m_tracker.end() && it->second > 0) { + if (--(it->second) == 0) RemoveConnectionInternal(it); + } + } + //! Remove a connection entirely + void RemoveConnection(const evhttp_connection* conn) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + LOCK(m_mutex); + auto it{m_tracker.find(Assert(conn))}; + if (it != m_tracker.end()) RemoveConnectionInternal(it); + } + size_t CountActiveConnections() const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + 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) + { + WAIT_LOCK(m_mutex, lock); + m_cv.wait(lock, [this]() EXCLUSIVE_LOCKS_REQUIRED(m_mutex) { return m_tracker.empty(); }); + } +}; +//! Track active requests +static HTTPRequestTracker g_requests; + /** Check if a network address is allowed to access the HTTP server */ static bool ClientAllowed(const CNetAddr& netaddr) { @@ -217,11 +275,22 @@ std::string RequestMethodString(HTTPRequest::RequestMethod m) /** HTTP request callback */ static void http_request_cb(struct evhttp_request* req, void* arg) { + evhttp_connection* conn{evhttp_request_get_connection(req)}; + // Track active requests + { + g_requests.AddRequest(req); + evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) { + g_requests.RemoveRequest(req); + }, nullptr); + evhttp_connection_set_closecb(conn, [](evhttp_connection* conn, void* arg) { + g_requests.RemoveConnection(conn); + }, nullptr); + } + // Disable reading to work around a libevent bug, fixed in 2.1.9 // See https://github.com/libevent/libevent/commit/5ff8eb26371c4dc56f384b2de35bea2d87814779 // and https://github.com/bitcoin/bitcoin/pull/11593. if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02010900) { - evhttp_connection* conn = evhttp_request_get_connection(req); if (conn) { bufferevent* bev = evhttp_connection_get_bufferevent(conn); if (bev) { @@ -496,15 +565,24 @@ void StopHTTPServer() evhttp_del_accept_socket(eventHTTP, socket); } boundSockets.clear(); - if (eventBase) { - LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); - if (g_thread_http.joinable()) g_thread_http.join(); + { + if (const auto n_connections{g_requests.CountActiveConnections()}; n_connections != 0) { + LogPrint(BCLog::HTTP, "Waiting for %d connections to stop HTTP server\n", n_connections); + } + g_requests.WaitUntilEmpty(); } if (eventHTTP) { - evhttp_free(eventHTTP); - eventHTTP = nullptr; + // 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(); event_base_free(eventBase); eventBase = nullptr; } diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py index 19431d2baaf8..3c4e03bb05ee 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 @@ def run_test(self): # 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()