From b1c2370dde9ade180c638e5d9a4797f085322b5b Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Tue, 6 Feb 2018 19:27:13 +0100
Subject: [PATCH 1/3] http: Join worker threads before deleting work queue

This prevents a potential race condition if control flow ends up in
`ShutdownHTTPServer` before the thread gets to `queue->Run()`,
deleting the work queue while workers are still going to use it.

Meant to fix #12362.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
---
 src/httpserver.cpp | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index a022d220e0..a554dcb097 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -449,6 +449,7 @@ bool UpdateHTTPServerLogging(bool enable) {
 
 std::thread threadHTTP;
 std::future<bool> threadResult;
+static std::vector<std::thread> g_thread_http_workers;
 
 bool StartHTTPServer()
 {
@@ -460,8 +461,7 @@ bool StartHTTPServer()
     threadHTTP = std::thread(std::move(task), eventBase, eventHTTP);
 
     for (int i = 0; i < rpcThreads; i++) {
-        std::thread rpc_worker(HTTPWorkQueueRun, workQueue);
-        rpc_worker.detach();
+        g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue);
     }
     return true;
 }
@@ -487,6 +487,10 @@ void StopHTTPServer()
     if (workQueue) {
         LogPrint(BCLog::HTTP, "Waiting for HTTP worker threads to exit\n");
         workQueue->WaitExit();
+        for (auto& thread: g_thread_http_workers) {
+            thread.join();
+        }
+        g_thread_http_workers.clear();
         delete workQueue;
         workQueue = nullptr;
     }

From f94665466ed50e868c98b1a1c708ad5767727bb6 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Tue, 6 Feb 2018 20:32:33 +0100
Subject: [PATCH 2/3] http: Remove WaitExit from WorkQueue

This function, which waits for all threads to exit, is no longer needed
now that threads are joined instead.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
---
 src/httpserver.cpp | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index a554dcb097..5000b0e245 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -99,8 +99,7 @@ public:
                                  numThreads(0)
     {
     }
-    /** Precondition: worker threads have all stopped
-     * (call WaitExit)
+    /** Precondition: worker threads have all stopped (they have been joined).
      */
     ~WorkQueue()
     {
@@ -141,13 +140,6 @@ public:
         running = false;
         cond.notify_all();
     }
-    /** Wait for worker threads to exit */
-    void WaitExit()
-    {
-        std::unique_lock<std::mutex> lock(cs);
-        while (numThreads > 0)
-            cond.wait(lock);
-    }
 };
 
 struct HTTPPathHandler
@@ -486,7 +478,6 @@ void StopHTTPServer()
     LogPrint(BCLog::HTTP, "Stopping HTTP server\n");
     if (workQueue) {
         LogPrint(BCLog::HTTP, "Waiting for HTTP worker threads to exit\n");
-        workQueue->WaitExit();
         for (auto& thread: g_thread_http_workers) {
             thread.join();
         }

From 11e01515fe0fbc7823d4111ad6e016a02c485a78 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 7 Feb 2018 09:53:46 +0100
Subject: [PATCH 3/3] http: Remove numThreads and ThreadCounter

The HTTP worker thread counter, as well as the RAII object that was used
to maintain it, is unused now, so can be removed.

Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
---
 src/httpserver.cpp | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 5000b0e245..f78ce13737 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -73,30 +73,10 @@ private:
     std::deque<std::unique_ptr<WorkItem>> queue;
     bool running;
     size_t maxDepth;
-    int numThreads;
-
-    /** RAII object to keep track of number of running worker threads */
-    class ThreadCounter
-    {
-    public:
-        WorkQueue &wq;
-        explicit ThreadCounter(WorkQueue &w): wq(w)
-        {
-            std::lock_guard<std::mutex> lock(wq.cs);
-            wq.numThreads += 1;
-        }
-        ~ThreadCounter()
-        {
-            std::lock_guard<std::mutex> lock(wq.cs);
-            wq.numThreads -= 1;
-            wq.cond.notify_all();
-        }
-    };
 
 public:
     explicit WorkQueue(size_t _maxDepth) : running(true),
-                                 maxDepth(_maxDepth),
-                                 numThreads(0)
+                                 maxDepth(_maxDepth)
     {
     }
     /** Precondition: worker threads have all stopped (they have been joined).
@@ -118,7 +98,6 @@ public:
     /** Thread function */
     void Run()
     {
-        ThreadCounter count(*this);
         while (true) {
             std::unique_ptr<WorkItem> i;
             {