From 02e1e4eff6cda0bfc24b455a7c1583394cbff6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 20 Nov 2018 17:52:15 +0000 Subject: rpc: Add wait argument to stop --- src/rpc/client.cpp | 1 + src/rpc/server.cpp | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 2b99808c07..6f1bfb03d1 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -162,6 +162,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "rescanblockchain", 1, "stop_height"}, { "createwallet", 1, "disable_private_keys"}, { "getnodeaddresses", 0, "count"}, + { "stop", 0, "wait" }, }; // clang-format on diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index c565094a10..865d343eb2 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -227,6 +227,9 @@ UniValue help(const JSONRPCRequest& jsonRequest) UniValue stop(const JSONRPCRequest& jsonRequest) { // Accept the deprecated and ignored 'detach' boolean argument + // Also accept the hidden 'wait' integer argument (milliseconds) + // For instance, 'stop 1000' makes the call wait 1 second before returning + // to the client (intended for testing) if (jsonRequest.fHelp || jsonRequest.params.size() > 1) throw std::runtime_error( RPCHelpMan{"stop", @@ -235,6 +238,9 @@ UniValue stop(const JSONRPCRequest& jsonRequest) // Event loop will exit after current HTTP requests have been handled, so // this reply will get back to the client. StartShutdown(); + if (jsonRequest.params[0].isNum()) { + MilliSleep(jsonRequest.params[0].get_int()); + } return "Bitcoin server stopping"; } @@ -264,7 +270,7 @@ static const CRPCCommand vRPCCommands[] = // --------------------- ------------------------ ----------------------- ---------- /* Overall control/query calls */ { "control", "help", &help, {"command"} }, - { "control", "stop", &stop, {} }, + { "control", "stop", &stop, {"wait"} }, { "control", "uptime", &uptime, {} }, }; // clang-format on -- cgit v1.2.3 From 18e968581697078c36a3c3818f8906cf134ccadd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 7 Nov 2018 09:08:56 +0000 Subject: http: Send "Connection: close" header if shutdown is requested Sending the header "Connection: close" makes libevent close persistent connections (implicit with HTTP 1.1) which cleans the event base when shutdown is requested. --- src/httpserver.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 00434169cd..342956f7a3 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -10,6 +10,7 @@ #include #include #include // For HTTP status codes +#include #include #include @@ -583,6 +584,9 @@ 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()) { + WriteHeader("Connection", "close"); + } // Send event to main http thread to send reply message struct evbuffer* evb = evhttp_request_get_output_buffer(req); assert(evb); -- cgit v1.2.3 From 6b13580f4e3842c11abd9b8bee7255fb2472b6fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 7 Nov 2018 09:10:07 +0000 Subject: http: Unlisten sockets after all workers quit This (almost) move only ensures the event base loop doesn't exit before HTTP worker threads exit. This way events registered by HTTP workers are processed and not discarded. --- src/httpserver.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 342956f7a3..0ab9d966dc 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -443,10 +443,6 @@ void InterruptHTTPServer() { LogPrint(BCLog::HTTP, "Interrupting HTTP server\n"); if (eventHTTP) { - // Unlisten sockets - for (evhttp_bound_socket *socket : boundSockets) { - evhttp_del_accept_socket(eventHTTP, socket); - } // Reject requests on current connections evhttp_set_gencb(eventHTTP, http_reject_request_cb, nullptr); } @@ -466,6 +462,12 @@ void StopHTTPServer() delete workQueue; workQueue = nullptr; } + // Unlisten sockets, these are what make the event loop running, which means + // that after this and all connections are closed the event loop will quit. + for (evhttp_bound_socket *socket : boundSockets) { + evhttp_del_accept_socket(eventHTTP, socket); + } + boundSockets.clear(); if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); // Exit the event loop as soon as there are no active events. -- cgit v1.2.3 From e98a9eede2fb48ff33a020acc888cbcd83e24bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 7 Nov 2018 09:11:16 +0000 Subject: http: Remove unnecessary event_base_loopexit call Let event base loop exit cleanly by processing all active and pending events. The call is no longer necessary because closing persistent connections is now properly handled. --- src/httpserver.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 0ab9d966dc..2cc83eecdc 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -470,8 +470,6 @@ void StopHTTPServer() boundSockets.clear(); if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); - // Exit the event loop as soon as there are no active events. - event_base_loopexit(eventBase, nullptr); // Give event loop a few seconds to exit (to send back last RPC responses), then break it // Before this was solved with event_base_loopexit, but that didn't work as expected in // at least libevent 2.0.21 and always introduced a delay. In libevent -- cgit v1.2.3 From 8d3f46ec3938e2ba17654fecacd1d2629f9915fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 7 Nov 2018 09:13:36 +0000 Subject: http: Remove timeout to exit event loop Let HTTP connections to timeout due to inactivity. Let all remaning connections finish sending the response and close. --- src/httpserver.cpp | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 2cc83eecdc..cb8578927a 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include @@ -422,7 +421,6 @@ bool UpdateHTTPServerLogging(bool enable) { } std::thread threadHTTP; -std::future threadResult; static std::vector g_thread_http_workers; void StartHTTPServer() @@ -430,9 +428,7 @@ void StartHTTPServer() LogPrint(BCLog::HTTP, "Starting HTTP server\n"); int rpcThreads = std::max((long)gArgs.GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L); LogPrintf("HTTP: starting %d worker threads\n", rpcThreads); - std::packaged_task task(ThreadHTTP); - threadResult = task.get_future(); - threadHTTP = std::thread(std::move(task), eventBase); + threadHTTP = std::thread(ThreadHTTP, eventBase); for (int i = 0; i < rpcThreads; i++) { g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue); @@ -470,16 +466,6 @@ void StopHTTPServer() boundSockets.clear(); if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); - // Give event loop a few seconds to exit (to send back last RPC responses), then break it - // Before this was solved with event_base_loopexit, but that didn't work as expected in - // at least libevent 2.0.21 and always introduced a delay. In libevent - // master that appears to be solved, so in the future that solution - // could be used again (if desirable). - // (see discussion in https://github.com/bitcoin/bitcoin/pull/6990) - if (threadResult.valid() && threadResult.wait_for(std::chrono::milliseconds(2000)) == std::future_status::timeout) { - LogPrintf("HTTP event loop did not exit within allotted time, sending loopbreak\n"); - event_base_loopbreak(eventBase); - } threadHTTP.join(); } if (eventHTTP) { -- cgit v1.2.3