Skip to content

Commit a264c32

Browse files
committed
http: speed up shutdown
This continues/fixes #6719. `event_base_loopbreak` was not doing what I expected it to, at least in libevent 2.0.21. What I expected was that it sets a timeout, given that no other pending events it would exit in N seconds. However, what it does was delay the event loop exit with 10 seconds, even if nothing is pending. Solve it in a different way: give the event loop thread time to exit out of itself, and if it doesn't, send loopbreak. This speeds up the RPC tests a lot, each exit incurred a 10 second overhead, with this change there should be no shutdown overhead in the common case and up to two seconds if the event loop is blocking. As a bonus this breaks dependency on boost::thread_group, as the HTTP server minds its own offspring.
1 parent 3ac7060 commit a264c32

File tree

3 files changed

+22
-12
lines changed

3 files changed

+22
-12
lines changed

src/httpserver.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -438,15 +438,17 @@ bool InitHTTPServer()
438438
return true;
439439
}
440440

441-
bool StartHTTPServer(boost::thread_group& threadGroup)
441+
boost::thread threadHTTP;
442+
443+
bool StartHTTPServer()
442444
{
443445
LogPrint("http", "Starting HTTP server\n");
444446
int rpcThreads = std::max((long)GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L);
445447
LogPrintf("HTTP: starting %d worker threads\n", rpcThreads);
446-
threadGroup.create_thread(boost::bind(&ThreadHTTP, eventBase, eventHTTP));
448+
threadHTTP = boost::thread(boost::bind(&ThreadHTTP, eventBase, eventHTTP));
447449

448450
for (int i = 0; i < rpcThreads; i++)
449-
threadGroup.create_thread(boost::bind(&HTTPWorkQueueRun, workQueue));
451+
boost::thread(boost::bind(&HTTPWorkQueueRun, workQueue));
450452
return true;
451453
}
452454

@@ -461,13 +463,6 @@ void InterruptHTTPServer()
461463
// Reject requests on current connections
462464
evhttp_set_gencb(eventHTTP, http_reject_request_cb, NULL);
463465
}
464-
if (eventBase) {
465-
// Force-exit event loop after predefined time
466-
struct timeval tv;
467-
tv.tv_sec = 10;
468-
tv.tv_usec = 0;
469-
event_base_loopexit(eventBase, &tv);
470-
}
471466
if (workQueue)
472467
workQueue->Interrupt();
473468
}
@@ -480,6 +475,20 @@ void StopHTTPServer()
480475
workQueue->WaitExit();
481476
delete workQueue;
482477
}
478+
if (eventBase) {
479+
LogPrint("http", "Waiting for HTTP event thread to exit\n");
480+
// Give event loop a few seconds to exit (to send back last RPC responses), then break it
481+
// Before this was solved with event_base_loopexit, but that didn't work as expected in
482+
// at least libevent 2.0.21 and always introduced a delay. In libevent
483+
// master that appears to be solved, so in the future that solution
484+
// could be used again (if desirable).
485+
// (see discussion in https://github.com/bitcoin/bitcoin/pull/6990)
486+
if (!threadHTTP.try_join_for(boost::chrono::milliseconds(2000))) {
487+
LogPrintf("HTTP event loop did not exit within allotted time, sending loopbreak\n");
488+
event_base_loopbreak(eventBase);
489+
threadHTTP.join();
490+
}
491+
}
483492
if (eventHTTP) {
484493
evhttp_free(eventHTTP);
485494
eventHTTP = 0;
@@ -488,6 +497,7 @@ void StopHTTPServer()
488497
event_base_free(eventBase);
489498
eventBase = 0;
490499
}
500+
LogPrint("http", "Stopped HTTP server\n");
491501
}
492502

493503
struct event_base* EventBase()

src/httpserver.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ bool InitHTTPServer();
2828
* This is separate from InitHTTPServer to give users race-condition-free time
2929
* to register their handlers between InitHTTPServer and StartHTTPServer.
3030
*/
31-
bool StartHTTPServer(boost::thread_group& threadGroup);
31+
bool StartHTTPServer();
3232
/** Interrupt HTTP server threads */
3333
void InterruptHTTPServer();
3434
/** Stop HTTP server */

src/init.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ bool AppInitServers(boost::thread_group& threadGroup)
661661
return false;
662662
if (GetBoolArg("-rest", false) && !StartREST())
663663
return false;
664-
if (!StartHTTPServer(threadGroup))
664+
if (!StartHTTPServer())
665665
return false;
666666
return true;
667667
}

0 commit comments

Comments
 (0)