diff --git a/src/httpserver.cpp b/src/httpserver.cpp --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -11,6 +11,7 @@ #include #include #include // For HTTP status codes +#include #include #include #include @@ -39,7 +40,6 @@ #include #include #include -#include #include /** Maximum size of http request (request line + headers) */ @@ -451,7 +451,6 @@ } std::thread threadHTTP; -std::future threadResult; static std::vector g_thread_http_workers; void StartHTTPServer() { @@ -459,9 +458,7 @@ 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); @@ -471,10 +468,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); } @@ -494,24 +487,14 @@ 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. - 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 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) { @@ -617,6 +600,9 @@ */ 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); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -158,6 +158,7 @@ {"rescanblockchain", 1, "stop_height"}, {"createwallet", 1, "disable_private_keys"}, {"createwallet", 2, "blank"}, + {"stop", 0, "wait"}, }; class CRPCConvertTable { diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -286,6 +286,9 @@ static UniValue stop(const Config &config, 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("stop\n" "\nStop Bitcoin server."); @@ -294,6 +297,9 @@ // 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"; } @@ -322,7 +328,7 @@ // ------------------- ------------------------ ---------------------- ---------- /* Overall control/query calls */ { "control", "help", help, {"command"} }, - { "control", "stop", stop, {} }, + { "control", "stop", stop, {"wait"} }, { "control", "uptime", uptime, {} }, }; // clang-format on diff --git a/test/functional/feature_shutdown.py b/test/functional/feature_shutdown.py new file mode 100755 --- /dev/null +++ b/test/functional/feature_shutdown.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test bitcoind shutdown.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, get_rpc_proxy +from threading import Thread + + +def test_long_call(node): + block = node.waitfornewblock() + assert_equal(block['height'], 0) + + +class ShutdownTest(BitcoinTestFramework): + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def run_test(self): + node = get_rpc_proxy( + self.nodes[0].url, 1, timeout=600, coveragedir=self.nodes[0].coverage_dir) + Thread(target=test_long_call, args=(node,)).start() + # wait 1 second to ensure event loop waits for current connections to close + self.stop_node(0, wait=1000) + + +if __name__ == '__main__': + ShutdownTest().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -330,16 +330,16 @@ coverage.write_all_rpc_commands( self.options.coveragedir, node.rpc) - def stop_node(self, i, expected_stderr=''): + def stop_node(self, i, expected_stderr='', wait=0): """Stop a bitcoind test node""" - self.nodes[i].stop_node(expected_stderr) + self.nodes[i].stop_node(expected_stderr, wait=wait) self.nodes[i].wait_until_stopped() - def stop_nodes(self): + def stop_nodes(self, wait=0): """Stop multiple bitcoind test nodes""" for node in self.nodes: # Issue RPC to stop nodes - node.stop_node() + node.stop_node(wait=wait) for node in self.nodes: # Wait for nodes to stop diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -248,13 +248,13 @@ wallet_path = "wallet/{}".format(wallet_name) return self.rpc / wallet_path - def stop_node(self, expected_stderr=''): + def stop_node(self, expected_stderr='', wait=0): """Stop the node.""" if not self.running: return self.log.debug("Stopping node") try: - self.stop() + self.stop(wait=wait) except http.client.CannotSendRequest: self.log.exception("Unable to stop node.")