From 755aa05174e06effd758eeb78c5af9fb465e9611 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 28 Jul 2016 17:52:51 -0400 Subject: httpserver: use a future rather than relying on boost's try_join_for --- src/httpserver.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 812940eaf9..7150f96ed7 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -302,13 +303,14 @@ static void http_reject_request_cb(struct evhttp_request* req, void*) } /** Event dispatcher thread */ -static void ThreadHTTP(struct event_base* base, struct evhttp* http) +static bool ThreadHTTP(struct event_base* base, struct evhttp* http) { RenameThread("bitcoin-http"); LogPrint("http", "Entering http event loop\n"); event_base_dispatch(base); // Event loop will be interrupted by InterruptHTTPServer() LogPrint("http", "Exited http event loop\n"); + return event_base_got_break(base) == 0; } /** Bind HTTP server to specified addresses */ @@ -438,13 +440,16 @@ bool InitHTTPServer() } boost::thread threadHTTP; +std::future threadResult; bool StartHTTPServer() { LogPrint("http", "Starting HTTP server\n"); int rpcThreads = std::max((long)GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L); LogPrintf("HTTP: starting %d worker threads\n", rpcThreads); - threadHTTP = boost::thread(boost::bind(&ThreadHTTP, eventBase, eventHTTP)); + std::packaged_task task(ThreadHTTP); + threadResult = task.get_future(); + threadHTTP = boost::thread(std::bind(std::move(task), eventBase, eventHTTP)); for (int i = 0; i < rpcThreads; i++) boost::thread(boost::bind(&HTTPWorkQueueRun, workQueue)); @@ -482,15 +487,11 @@ void StopHTTPServer() // 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 BOOST_VERSION >= 105000 - if (!threadHTTP.try_join_for(boost::chrono::milliseconds(2000))) { -#else - if (!threadHTTP.timed_join(boost::posix_time::milliseconds(2000))) { -#endif + 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(); } + threadHTTP.join(); } if (eventHTTP) { evhttp_free(eventHTTP); -- cgit v1.2.3 From d3773ca9aeb0d2f12dc0c5a0726778050c8cb455 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 28 Jul 2016 18:21:00 -0400 Subject: httpserver: explicitly detach worker threads When using std::thread in place of boost::thread, letting the threads destruct results in a std::terminate. According to the docs, the same thing should be be happening in later boost versions: http://www.boost.org/doc/libs/1_55_0/doc/html/thread/thread_management.html#thread.thread_management.thread.destructor I'm unsure why this hasn't blown up already, but explicitly detaching can't hurt. --- src/httpserver.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 7150f96ed7..8d0d3c1584 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -451,8 +451,10 @@ bool StartHTTPServer() threadResult = task.get_future(); threadHTTP = boost::thread(std::bind(std::move(task), eventBase, eventHTTP)); - for (int i = 0; i < rpcThreads; i++) - boost::thread(boost::bind(&HTTPWorkQueueRun, workQueue)); + for (int i = 0; i < rpcThreads; i++) { + boost::thread rpc_worker(HTTPWorkQueueRun, workQueue); + rpc_worker.detach(); + } return true; } -- cgit v1.2.3 From 7e87033447149e54d9b5ab2f90ad3a7ed094d784 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 28 Jul 2016 18:31:25 -0400 Subject: httpserver: replace boost threads with std along with mutex/condvar/bind/etc. httpserver handles its own interruption, so there's no reason not to use std threading. While we're at it, may as well kill the BOOST_FOREACH's as well. --- src/httpserver.cpp | 39 ++++++++++++++++++--------------------- src/httpserver.h | 10 ++++------ 2 files changed, 22 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 8d0d3c1584..be7a6a1dde 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -35,9 +35,6 @@ #endif #endif -#include // for to_lower() -#include - /** Maximum size of http request (request line + headers) */ static const size_t MAX_HEADERS_SIZE = 8192; @@ -69,8 +66,8 @@ class WorkQueue { private: /** Mutex protects entire object */ - CWaitableCriticalSection cs; - CConditionVariable cond; + std::mutex cs; + std::condition_variable cond; std::deque> queue; bool running; size_t maxDepth; @@ -83,12 +80,12 @@ private: WorkQueue &wq; ThreadCounter(WorkQueue &w): wq(w) { - boost::lock_guard lock(wq.cs); + std::lock_guard lock(wq.cs); wq.numThreads += 1; } ~ThreadCounter() { - boost::lock_guard lock(wq.cs); + std::lock_guard lock(wq.cs); wq.numThreads -= 1; wq.cond.notify_all(); } @@ -109,7 +106,7 @@ public: /** Enqueue a work item */ bool Enqueue(WorkItem* item) { - boost::unique_lock lock(cs); + std::unique_lock lock(cs); if (queue.size() >= maxDepth) { return false; } @@ -124,7 +121,7 @@ public: while (running) { std::unique_ptr i; { - boost::unique_lock lock(cs); + std::unique_lock lock(cs); while (running && queue.empty()) cond.wait(lock); if (!running) @@ -138,14 +135,14 @@ public: /** Interrupt and exit loops */ void Interrupt() { - boost::unique_lock lock(cs); + std::unique_lock lock(cs); running = false; cond.notify_all(); } /** Wait for worker threads to exit */ void WaitExit() { - boost::unique_lock lock(cs); + std::unique_lock lock(cs); while (numThreads > 0) cond.wait(lock); } @@ -153,7 +150,7 @@ public: /** Return current depth of queue */ size_t Depth() { - boost::unique_lock lock(cs); + std::unique_lock lock(cs); return queue.size(); } }; @@ -190,7 +187,7 @@ static bool ClientAllowed(const CNetAddr& netaddr) { if (!netaddr.IsValid()) return false; - BOOST_FOREACH (const CSubNet& subnet, rpc_allow_subnets) + for(const CSubNet& subnet : rpc_allow_subnets) if (subnet.Match(netaddr)) return true; return false; @@ -204,7 +201,7 @@ static bool InitHTTPAllowList() rpc_allow_subnets.push_back(CSubNet("::1")); // always allow IPv6 localhost if (mapMultiArgs.count("-rpcallowip")) { const std::vector& vAllow = mapMultiArgs["-rpcallowip"]; - BOOST_FOREACH (std::string strAllow, vAllow) { + for (std::string strAllow : vAllow) { CSubNet subnet(strAllow); if (!subnet.IsValid()) { uiInterface.ThreadSafeMessageBox( @@ -216,7 +213,7 @@ static bool InitHTTPAllowList() } } std::string strAllowed; - BOOST_FOREACH (const CSubNet& subnet, rpc_allow_subnets) + for (const CSubNet& subnet : rpc_allow_subnets) strAllowed += subnet.ToString() + " "; LogPrint("http", "Allowing HTTP connections from: %s\n", strAllowed); return true; @@ -439,7 +436,7 @@ bool InitHTTPServer() return true; } -boost::thread threadHTTP; +std::thread threadHTTP; std::future threadResult; bool StartHTTPServer() @@ -449,10 +446,10 @@ bool StartHTTPServer() LogPrintf("HTTP: starting %d worker threads\n", rpcThreads); std::packaged_task task(ThreadHTTP); threadResult = task.get_future(); - threadHTTP = boost::thread(std::bind(std::move(task), eventBase, eventHTTP)); + threadHTTP = std::thread(std::move(task), eventBase, eventHTTP); for (int i = 0; i < rpcThreads; i++) { - boost::thread rpc_worker(HTTPWorkQueueRun, workQueue); + std::thread rpc_worker(HTTPWorkQueueRun, workQueue); rpc_worker.detach(); } return true; @@ -463,7 +460,7 @@ void InterruptHTTPServer() LogPrint("http", "Interrupting HTTP server\n"); if (eventHTTP) { // Unlisten sockets - BOOST_FOREACH (evhttp_bound_socket *socket, boundSockets) { + for (evhttp_bound_socket *socket : boundSockets) { evhttp_del_accept_socket(eventHTTP, socket); } // Reject requests on current connections @@ -520,7 +517,7 @@ static void httpevent_callback_fn(evutil_socket_t, short, void* data) delete self; } -HTTPEvent::HTTPEvent(struct event_base* base, bool deleteWhenTriggered, const boost::function& handler): +HTTPEvent::HTTPEvent(struct event_base* base, bool deleteWhenTriggered, const std::function& handler): deleteWhenTriggered(deleteWhenTriggered), handler(handler) { ev = event_new(base, -1, 0, httpevent_callback_fn, this); @@ -602,7 +599,7 @@ void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) assert(evb); evbuffer_add(evb, strReply.data(), strReply.size()); HTTPEvent* ev = new HTTPEvent(eventBase, true, - boost::bind(evhttp_send_reply, req, nStatus, (const char*)NULL, (struct evbuffer *)NULL)); + std::bind(evhttp_send_reply, req, nStatus, (const char*)NULL, (struct evbuffer *)NULL)); ev->trigger(0); replySent = true; req = 0; // transferred back to main thread diff --git a/src/httpserver.h b/src/httpserver.h index 20a119cc5c..0e30e666a6 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -7,9 +7,7 @@ #include #include -#include -#include -#include +#include static const int DEFAULT_HTTP_THREADS=4; static const int DEFAULT_HTTP_WORKQUEUE=16; @@ -35,7 +33,7 @@ void InterruptHTTPServer(); void StopHTTPServer(); /** Handler for requests to a certain HTTP path */ -typedef boost::function HTTPRequestHandler; +typedef std::function HTTPRequestHandler; /** Register handler for prefix. * If multiple handlers match a prefix, the first-registered one will * be invoked. @@ -132,7 +130,7 @@ public: * deleteWhenTriggered deletes this event object after the event is triggered (and the handler called) * handler is the handler to call when the event is triggered. */ - HTTPEvent(struct event_base* base, bool deleteWhenTriggered, const boost::function& handler); + HTTPEvent(struct event_base* base, bool deleteWhenTriggered, const std::function& handler); ~HTTPEvent(); /** Trigger the event. If tv is 0, trigger it immediately. Otherwise trigger it after @@ -141,7 +139,7 @@ public: void trigger(struct timeval* tv); bool deleteWhenTriggered; - boost::function handler; + std::function handler; private: struct event* ev; }; -- cgit v1.2.3