From 42e5829d9710ebebda5de356fab01dd7c149d5fa Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 7 Jul 2023 17:32:54 -0400 Subject: [PATCH] refactor: Remove call to ShutdownRequested from HTTPRequest Pass HTTP server an interrupt object instead of having it depend on shutdown.h and global shutdown state. There is no change in behavior in this commit. --- src/httpserver.cpp | 13 +++++++------ src/httpserver.h | 9 +++++++-- src/init.cpp | 3 ++- src/test/fuzz/http_request.cpp | 4 +++- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 4c14c5b939c..e26ea6a596f 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -15,9 +15,9 @@ #include #include #include // For HTTP status codes -#include #include #include +#include #include #include #include @@ -284,7 +284,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg) } } } - std::unique_ptr hreq(new HTTPRequest(req)); + auto hreq{std::make_unique(req, *static_cast(arg))}; // Early address-based allow check if (!ClientAllowed(hreq->GetPeer())) { @@ -425,7 +425,7 @@ static void libevent_log_cb(int severity, const char *msg) LogPrintLevel(BCLog::LIBEVENT, level, "%s\n", msg); } -bool InitHTTPServer() +bool InitHTTPServer(const util::SignalInterrupt& interrupt) { if (!InitHTTPAllowList()) return false; @@ -454,7 +454,7 @@ bool InitHTTPServer() evhttp_set_timeout(http, gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)); evhttp_set_max_headers_size(http, MAX_HEADERS_SIZE); evhttp_set_max_body_size(http, MAX_SIZE); - evhttp_set_gencb(http, http_request_cb, nullptr); + evhttp_set_gencb(http, http_request_cb, (void*)&interrupt); if (!HTTPBindAddresses(http)) { LogPrintf("Unable to bind any endpoint for RPC server\n"); @@ -579,7 +579,8 @@ void HTTPEvent::trigger(struct timeval* tv) else evtimer_add(ev, tv); // trigger after timeval passed } -HTTPRequest::HTTPRequest(struct evhttp_request* _req, bool _replySent) : req(_req), replySent(_replySent) +HTTPRequest::HTTPRequest(struct evhttp_request* _req, const util::SignalInterrupt& interrupt, bool _replySent) + : req(_req), m_interrupt(interrupt), replySent(_replySent) { } @@ -639,7 +640,7 @@ void HTTPRequest::WriteHeader(const std::string& hdr, const std::string& value) void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) { assert(!replySent && req); - if (ShutdownRequested()) { + if (m_interrupt) { WriteHeader("Connection", "close"); } // Send event to main http thread to send reply message diff --git a/src/httpserver.h b/src/httpserver.h index 036a39a0232..9a49877f09d 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -9,6 +9,10 @@ #include #include +namespace util { +class SignalInterrupt; +} // namespace util + static const int DEFAULT_HTTP_THREADS=4; static const int DEFAULT_HTTP_WORKQUEUE=16; static const int DEFAULT_HTTP_SERVER_TIMEOUT=30; @@ -21,7 +25,7 @@ class HTTPRequest; /** Initialize HTTP server. * Call this before RegisterHTTPHandler or EventBase(). */ -bool InitHTTPServer(); +bool InitHTTPServer(const util::SignalInterrupt& interrupt); /** Start HTTP server. * This is separate from InitHTTPServer to give users race-condition-free time * to register their handlers between InitHTTPServer and StartHTTPServer. @@ -57,10 +61,11 @@ class HTTPRequest { private: struct evhttp_request* req; + const util::SignalInterrupt& m_interrupt; bool replySent; public: - explicit HTTPRequest(struct evhttp_request* req, bool replySent = false); + explicit HTTPRequest(struct evhttp_request* req, const util::SignalInterrupt& interrupt, bool replySent = false); ~HTTPRequest(); enum RequestMethod { diff --git a/src/init.cpp b/src/init.cpp index dfc05ebb675..1c4c79aa861 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -690,8 +690,9 @@ static bool AppInitServers(NodeContext& node) const ArgsManager& args = *Assert(node.args); RPCServer::OnStarted(&OnRPCStarted); RPCServer::OnStopped(&OnRPCStopped); - if (!InitHTTPServer()) + if (!InitHTTPServer(*Assert(node.shutdown))) { return false; + } StartRPC(); node.rpc_interruption_point = RpcInterruptionPoint; if (!StartHTTPRPC(&node)) diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index 9928c4a1ab7..f13f1c72a51 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -47,7 +48,8 @@ FUZZ_TARGET(http_request) return; } - HTTPRequest http_request{evreq, true}; + util::SignalInterrupt interrupt; + HTTPRequest http_request{evreq, interrupt, true}; const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod(); (void)RequestMethodString(request_method); (void)http_request.GetURI();