From d3f9a66580613c931f6e9b272fbf823b0e7e2a35 Mon Sep 17 00:00:00 2001
From: "W. J. van der Laan" <laanwj@protonmail.com>
Date: Mon, 21 Jun 2021 11:15:11 +0200
Subject: [PATCH] Merge bitcoin/bitcoin#19033: http: Release work queue after
 event base finish
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

4e353cb618745cdb5d98e58e7dcd400ded01299a http: Release work queue after event base finish (João Barbosa)

Pull request description:

  This fixes a race between `http_request_cb` and `StopHTTPServer` where
  the work queue is used after release.

  Fixes #18856.

ACKs for top commit:
  fjahr:
    Code review ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
  achow101:
    ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
  LarryRuane:
    ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a
  hebasto:
    ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a, tested (rebased on top of master 9313c4e6aa4b707c06a86b33d5d2753cd8383340) on Linux Mint 20.1 (x86_64) using MarcoFalke's [patch](https://github.com/bitcoin/bitcoin/pull/19033#issuecomment-640106647), including different `-rpcthreads`/`-rpcworkqueue` cases. The bug is fixed. The code is correct.

Tree-SHA512: 185d2a9744d0d5134d782bf321ac9958ba17b11a5b3d70b4897c8243e6b146dfd3f23c57aef8e10ae9484374120b64389c1949a9cf0a21dccc47ffc934c20930
---
 src/httpserver.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 2cf97280d6..60864e06dc 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -94,7 +94,7 @@ public:
     bool Enqueue(WorkItem* item)
     {
         LOCK(cs);
-        if (queue.size() >= maxDepth) {
+        if (!running || queue.size() >= maxDepth) {
             return false;
         }
         queue.emplace_back(std::unique_ptr<WorkItem>(item));
@@ -110,7 +110,7 @@ public:
                 WAIT_LOCK(cs, lock);
                 while (running && queue.empty())
                     cond.wait(lock);
-                if (!running)
+                if (!running && queue.empty())
                     break;
                 i = std::move(queue.front());
                 queue.pop_front();
@@ -468,8 +468,6 @@ void StopHTTPServer()
             thread.join();
         }
         g_thread_http_workers.clear();
-        delete workQueue;
-        workQueue = nullptr;
     }
     // Unlisten sockets, these are what make the event loop running, which means
     // that after this and all connections are closed the event loop will quit.
@@ -489,6 +487,10 @@ void StopHTTPServer()
         event_base_free(eventBase);
         eventBase = nullptr;
     }
+    if (workQueue) {
+        delete workQueue;
+        workQueue = nullptr;
+    }
     LogPrint(BCLog::HTTP, "Stopped HTTP server\n");
 }