From a264c32e3321ae909ca59cb8ce8bf5d812dbc4e1 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 11 Nov 2015 17:34:10 +0100
Subject: [PATCH] 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.
---
 src/httpserver.cpp | 30 ++++++++++++++++++++----------
 src/httpserver.h   |  2 +-
 src/init.cpp       |  2 +-
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 8698abb90..424ef015c 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -438,15 +438,17 @@ bool InitHTTPServer()
     return true;
 }
 
-bool StartHTTPServer(boost::thread_group& threadGroup)
+boost::thread threadHTTP;
+
+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);
-    threadGroup.create_thread(boost::bind(&ThreadHTTP, eventBase, eventHTTP));
+    threadHTTP = boost::thread(boost::bind(&ThreadHTTP, eventBase, eventHTTP));
 
     for (int i = 0; i < rpcThreads; i++)
-        threadGroup.create_thread(boost::bind(&HTTPWorkQueueRun, workQueue));
+        boost::thread(boost::bind(&HTTPWorkQueueRun, workQueue));
     return true;
 }
 
@@ -461,13 +463,6 @@ void InterruptHTTPServer()
         // Reject requests on current connections
         evhttp_set_gencb(eventHTTP, http_reject_request_cb, NULL);
     }
-    if (eventBase) {
-        // Force-exit event loop after predefined time
-        struct timeval tv;
-        tv.tv_sec = 10;
-        tv.tv_usec = 0;
-        event_base_loopexit(eventBase, &tv);
-    }
     if (workQueue)
         workQueue->Interrupt();
 }
@@ -480,6 +475,20 @@ void StopHTTPServer()
         workQueue->WaitExit();
         delete workQueue;
     }
+    if (eventBase) {
+        LogPrint("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 (!threadHTTP.try_join_for(boost::chrono::milliseconds(2000))) {
+            LogPrintf("HTTP event loop did not exit within allotted time, sending loopbreak\n");
+            event_base_loopbreak(eventBase);
+            threadHTTP.join();
+        }
+    }
     if (eventHTTP) {
         evhttp_free(eventHTTP);
         eventHTTP = 0;
@@ -488,6 +497,7 @@ void StopHTTPServer()
         event_base_free(eventBase);
         eventBase = 0;
     }
+    LogPrint("http", "Stopped HTTP server\n");
 }
 
 struct event_base* EventBase()
diff --git a/src/httpserver.h b/src/httpserver.h
index b377dc19f..20a119cc5 100644
--- a/src/httpserver.h
+++ b/src/httpserver.h
@@ -28,7 +28,7 @@ bool InitHTTPServer();
  * 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);
+bool StartHTTPServer();
 /** Interrupt HTTP server threads */
 void InterruptHTTPServer();
 /** Stop HTTP server */
diff --git a/src/init.cpp b/src/init.cpp
index 5f2dc8bf2..b58c47a94 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -661,7 +661,7 @@ bool AppInitServers(boost::thread_group& threadGroup)
         return false;
     if (GetBoolArg("-rest", false) && !StartREST())
         return false;
-    if (!StartHTTPServer(threadGroup))
+    if (!StartHTTPServer())
         return false;
     return true;
 }