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.
This commit is contained in:
Ryan Ofsky 2023-07-07 17:32:54 -04:00
parent 73133c36aa
commit 42e5829d97
4 changed files with 19 additions and 10 deletions

View file

@ -15,9 +15,9 @@
#include <netbase.h> #include <netbase.h>
#include <node/interface_ui.h> #include <node/interface_ui.h>
#include <rpc/protocol.h> // For HTTP status codes #include <rpc/protocol.h> // For HTTP status codes
#include <shutdown.h>
#include <sync.h> #include <sync.h>
#include <util/check.h> #include <util/check.h>
#include <util/signalinterrupt.h>
#include <util/strencodings.h> #include <util/strencodings.h>
#include <util/threadnames.h> #include <util/threadnames.h>
#include <util/translation.h> #include <util/translation.h>
@ -284,7 +284,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
} }
} }
} }
std::unique_ptr<HTTPRequest> hreq(new HTTPRequest(req)); auto hreq{std::make_unique<HTTPRequest>(req, *static_cast<const util::SignalInterrupt*>(arg))};
// Early address-based allow check // Early address-based allow check
if (!ClientAllowed(hreq->GetPeer())) { 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); LogPrintLevel(BCLog::LIBEVENT, level, "%s\n", msg);
} }
bool InitHTTPServer() bool InitHTTPServer(const util::SignalInterrupt& interrupt)
{ {
if (!InitHTTPAllowList()) if (!InitHTTPAllowList())
return false; return false;
@ -454,7 +454,7 @@ bool InitHTTPServer()
evhttp_set_timeout(http, gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)); evhttp_set_timeout(http, gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT));
evhttp_set_max_headers_size(http, MAX_HEADERS_SIZE); evhttp_set_max_headers_size(http, MAX_HEADERS_SIZE);
evhttp_set_max_body_size(http, MAX_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)) { if (!HTTPBindAddresses(http)) {
LogPrintf("Unable to bind any endpoint for RPC server\n"); LogPrintf("Unable to bind any endpoint for RPC server\n");
@ -579,7 +579,8 @@ void HTTPEvent::trigger(struct timeval* tv)
else else
evtimer_add(ev, tv); // trigger after timeval passed 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) void HTTPRequest::WriteReply(int nStatus, const std::string& strReply)
{ {
assert(!replySent && req); assert(!replySent && req);
if (ShutdownRequested()) { if (m_interrupt) {
WriteHeader("Connection", "close"); WriteHeader("Connection", "close");
} }
// Send event to main http thread to send reply message // Send event to main http thread to send reply message

View file

@ -9,6 +9,10 @@
#include <optional> #include <optional>
#include <string> #include <string>
namespace util {
class SignalInterrupt;
} // namespace util
static const int DEFAULT_HTTP_THREADS=4; static const int DEFAULT_HTTP_THREADS=4;
static const int DEFAULT_HTTP_WORKQUEUE=16; static const int DEFAULT_HTTP_WORKQUEUE=16;
static const int DEFAULT_HTTP_SERVER_TIMEOUT=30; static const int DEFAULT_HTTP_SERVER_TIMEOUT=30;
@ -21,7 +25,7 @@ class HTTPRequest;
/** Initialize HTTP server. /** Initialize HTTP server.
* Call this before RegisterHTTPHandler or EventBase(). * Call this before RegisterHTTPHandler or EventBase().
*/ */
bool InitHTTPServer(); bool InitHTTPServer(const util::SignalInterrupt& interrupt);
/** Start HTTP server. /** Start HTTP server.
* This is separate from InitHTTPServer to give users race-condition-free time * This is separate from InitHTTPServer to give users race-condition-free time
* to register their handlers between InitHTTPServer and StartHTTPServer. * to register their handlers between InitHTTPServer and StartHTTPServer.
@ -57,10 +61,11 @@ class HTTPRequest
{ {
private: private:
struct evhttp_request* req; struct evhttp_request* req;
const util::SignalInterrupt& m_interrupt;
bool replySent; bool replySent;
public: public:
explicit HTTPRequest(struct evhttp_request* req, bool replySent = false); explicit HTTPRequest(struct evhttp_request* req, const util::SignalInterrupt& interrupt, bool replySent = false);
~HTTPRequest(); ~HTTPRequest();
enum RequestMethod { enum RequestMethod {

View file

@ -690,8 +690,9 @@ static bool AppInitServers(NodeContext& node)
const ArgsManager& args = *Assert(node.args); const ArgsManager& args = *Assert(node.args);
RPCServer::OnStarted(&OnRPCStarted); RPCServer::OnStarted(&OnRPCStarted);
RPCServer::OnStopped(&OnRPCStopped); RPCServer::OnStopped(&OnRPCStopped);
if (!InitHTTPServer()) if (!InitHTTPServer(*Assert(node.shutdown))) {
return false; return false;
}
StartRPC(); StartRPC();
node.rpc_interruption_point = RpcInterruptionPoint; node.rpc_interruption_point = RpcInterruptionPoint;
if (!StartHTTPRPC(&node)) if (!StartHTTPRPC(&node))

View file

@ -7,6 +7,7 @@
#include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h> #include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h> #include <test/fuzz/util.h>
#include <util/signalinterrupt.h>
#include <util/strencodings.h> #include <util/strencodings.h>
#include <event2/buffer.h> #include <event2/buffer.h>
@ -47,7 +48,8 @@ FUZZ_TARGET(http_request)
return; return;
} }
HTTPRequest http_request{evreq, true}; util::SignalInterrupt interrupt;
HTTPRequest http_request{evreq, interrupt, true};
const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod(); const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod();
(void)RequestMethodString(request_method); (void)RequestMethodString(request_method);
(void)http_request.GetURI(); (void)http_request.GetURI();