From 3a174cd400c6c239539d4c0c10b557c3e0615212 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 28 Aug 2015 16:55:16 +0200 Subject: [PATCH] Fix race condition between starting HTTP server thread and setting EventBase() Split StartHTTPServer into InitHTTPServer and StartHTTPServer to give clients a window to register their handlers without race conditions. Thanks @ajweiss for figuring this out. --- src/httpserver.cpp | 24 +++++++++++++++--------- src/httpserver.h | 9 ++++++++- src/init.cpp | 4 +++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 813764f22..7e599b1d7 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -320,7 +320,7 @@ static void HTTPWorkQueueRun(WorkQueue* queue) queue->Run(); } -bool StartHTTPServer(boost::thread_group& threadGroup) +bool InitHTTPServer() { struct evhttp* http = 0; struct event_base* base = 0; @@ -366,19 +366,25 @@ bool StartHTTPServer(boost::thread_group& threadGroup) return false; } - LogPrint("http", "Starting HTTP server\n"); + LogPrint("http", "Initialized HTTP server\n"); int workQueueDepth = std::max((long)GetArg("-rpcworkqueue", DEFAULT_HTTP_WORKQUEUE), 1L); - int rpcThreads = std::max((long)GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L); - LogPrintf("HTTP: creating work queue of depth %d and %d worker threads\n", workQueueDepth, rpcThreads); - workQueue = new WorkQueue(workQueueDepth); + LogPrintf("HTTP: creating work queue of depth %d\n", workQueueDepth); - threadGroup.create_thread(boost::bind(&ThreadHTTP, base, http)); + workQueue = new WorkQueue(workQueueDepth); + eventBase = base; + eventHTTP = http; + return true; +} + +bool StartHTTPServer(boost::thread_group& threadGroup) +{ + 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); + threadGroup.create_thread(boost::bind(&ThreadHTTP, eventBase, eventHTTP)); for (int i = 0; i < rpcThreads; i++) threadGroup.create_thread(boost::bind(&HTTPWorkQueueRun, workQueue)); - - eventBase = base; - eventHTTP = http; return true; } diff --git a/src/httpserver.h b/src/httpserver.h index 1b0d77ad4..459c60c04 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -20,7 +20,14 @@ struct event_base; class CService; class HTTPRequest; -/** Start HTTP server */ +/** Initialize HTTP server. + * Call this before RegisterHTTPHandler or EventBase(). + */ +bool InitHTTPServer(); +/** Start HTTP server. + * This is separate from InitHTTPServer to give users race-condition-free time + * to register their handlers between InitHTTPServer and StartHTTPServer. + */ bool StartHTTPServer(boost::thread_group& threadGroup); /** Interrupt HTTP server threads */ void InterruptHTTPServer(); diff --git a/src/init.cpp b/src/init.cpp index bbf73dc8f..4aaeee257 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -618,7 +618,7 @@ bool AppInitServers(boost::thread_group& threadGroup) { RPCServer::OnStopped(&OnRPCStopped); RPCServer::OnPreCommand(&OnRPCPreCommand); - if (!StartHTTPServer(threadGroup)) + if (!InitHTTPServer()) return false; if (!StartRPC()) return false; @@ -626,6 +626,8 @@ bool AppInitServers(boost::thread_group& threadGroup) return false; if (GetBoolArg("-rest", false) && !StartREST()) return false; + if (!StartHTTPServer(threadGroup)) + return false; return true; }