http: Join worker threads before deleting work queue

laanwj · furszy · commit b8f7364917d4 · 2021-02-18T10:03:26.000-03:00
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 bitcoin#12362. Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
@@ -466,6 +466,7 @@ bool UpdateHTTPServerLogging(bool enable) {
 
 std::thread threadHTTP;
 std::future<bool> threadResult;
+static std::vector<std::thread> g_thread_http_workers;
 
 bool StartHTTPServer()
 {
@@ -477,8 +478,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;
 }
@@ -502,6 +502,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;
     }
     MilliSleep(500); // Avoid race condition while the last HTTP-thread is exiting

Original file line number	Diff line number	Diff line change
`@@ -466,6 +466,7 @@ bool UpdateHTTPServerLogging(bool enable) {`
`466`	`466`
`467`	`467`	`std::thread threadHTTP;`
`468`	`468`	`std::future<bool> threadResult;`
	`469`	`+static std::vector<std::thread> g_thread_http_workers;`
`469`	`470`
`470`	`471`	`bool StartHTTPServer()`
`471`	`472`	`{`
`@@ -477,8 +478,7 @@ bool StartHTTPServer()`
`477`	`478`	`threadHTTP = std::thread(std::move(task), eventBase, eventHTTP);`
`478`	`479`
`479`	`480`	`for (int i = 0; i < rpcThreads; i++) {`
`480`		`- std::thread rpc_worker(HTTPWorkQueueRun, workQueue);`
`481`		`- rpc_worker.detach();`
	`481`	`+ g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue);`
`482`	`482`	`}`
`483`	`483`	`return true;`
`484`	`484`	`}`
`@@ -502,6 +502,10 @@ void StopHTTPServer()`
`502`	`502`	`if (workQueue) {`
`503`	`503`	`LogPrint(BCLog::HTTP, "Waiting for HTTP worker threads to exit\n");`
`504`	`504`	`workQueue->WaitExit();`
	`505`	`+ for (auto& thread: g_thread_http_workers) {`
	`506`	`+ thread.join();`
	`507`	`+ }`
	`508`	`+ g_thread_http_workers.clear();`
`505`	`509`	`delete workQueue;`
`506`	`510`	`}`
`507`	`511`	`MilliSleep(500); // Avoid race condition while the last HTTP-thread is exiting`