ThreadPool follow-ups, proactive shutdown and HasReason dependency cleanup

furszy commented at 6:00 pm on February 11, 2026: member

A few follow-ups to #33689, includes:

ThreadPool active-wait during shutdown: Instead of just waiting for workers to finish processing tasks, Stop() now helps them actively. This speeds up the JSON-RPC and REST server shutdown, resulting in a faster node shutdown when many requests remain unhandled. This wasn’t included in the original PR due to the behavior change this introduces.
Decouple HasReason from the unit test framework machinery This avoids providing the entire unit test framework dependency to low-level tests that only require access to the HasReason utility class. Examples are: reverselock_tests.cpp, sync_tests.cpp, util_check_tests.cpp, util_string_tests.cpp, script_parse_tests.cpp and threadpool_tests.cpp. These tests no longer gain access to unnecessary components like the chainstate, node context, caches, etc. It includes l0rinc’s threadpool_tests.cpp HasReason changes.
Include response body in non-JSON HTTP error messages Straight from pinheadmz comment, it makes debugging CI issues easier.

DrahtBot commented at 6:00 pm on February 11, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hodlinator, maflcko, achow101

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34576 (threadpool: add ranged Submit overload by andrewtoth)
#34411 ([POC] Full Libevent removal by fanquake)
#33421 (node: add BlockTemplateCache by ismaelsadeeq)
#32061 (Replace libevent with our own HTTP and socket-handling implementation by pinheadmz)
#31682 ([IBD] specialize CheckBlock’s input & coinbase checks by l0rinc)
#31132 (validation: fetch block inputs on parallel threads 3x faster IBD by andrewtoth)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

drain -> drains [In the comment “Delay release so Stop() drain all tasks from the calling thread”, the verb should agree with the singular subject “Stop()”, so use “drains”.]

No other typos were found.

2026-02-26 14:30:33

fanquake added this to the milestone 31.0 on Feb 11, 2026

in src/util/threadpool.h:146 in ddf227371e

hodlinator commented at 8:00 pm on February 12, 2026:

There’s a race here already on master where another thread may run Start() after we release our lock on m_mutex above. Start() will both set m_interrupt = false, and later, the m_work_queue could also be added to from this new “session”, while the moved-out worker-threads that we join() on here are still reading the very same data.

The HTTP logic doesn’t call Start() a second time AFAIK so it’s not an issue in practice, but would still be nice to fix.

  0diff --git a/src/util/threadpool.h b/src/util/threadpool.h
  1index b75a94157e..9cafbd87d8 100644
  2--- a/src/util/threadpool.h
  3+++ b/src/util/threadpool.h
  4@@ -12,8 +12,8 @@
  5 
  6 #include <algorithm>
  7 #include <condition_variable>
  8-#include <functional>
  9 #include <future>
 10+#include <memory>
 11 #include <queue>
 12 #include <stdexcept>
 13 #include <thread>
 14@@ -46,39 +46,41 @@ class ThreadPool
 15 {
 16 private:
 17     std::string m_name;
 18-    Mutex m_mutex;
 19-    std::queue<std::packaged_task<void()>> m_work_queue GUARDED_BY(m_mutex);
 20-    std::condition_variable m_cv;
 21-    // Note: m_interrupt must be guarded by m_mutex, and cannot be replaced by an unguarded atomic bool.
 22-    // This ensures threads blocked on m_cv reliably observe the change and proceed correctly without missing signals.
 23-    // Ref: https://en.cppreference.com/w/cpp/thread/condition_variable
 24-    bool m_interrupt GUARDED_BY(m_mutex){false};
 25-    std::vector<std::thread> m_workers GUARDED_BY(m_mutex);
 26-
 27-    void WorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
 28+    struct Batch {
 29+        Mutex m_mutex;
 30+        std::queue<std::packaged_task<void()>> m_work_queue GUARDED_BY(m_mutex);
 31+        std::condition_variable m_cv;
 32+        // Note: m_interrupt must be guarded by m_mutex, and cannot be replaced by an unguarded atomic bool.
 33+        // This ensures threads blocked on m_cv reliably observe the change and proceed correctly without missing signals.
 34+        // Ref: https://en.cppreference.com/w/cpp/thread/condition_variable
 35+        bool m_interrupt GUARDED_BY(m_mutex){false};
 36+        std::vector<std::thread> m_workers GUARDED_BY(m_mutex);
 37+    };
 38+    Mutex m_batch_mutex;
 39+    std::unique_ptr<Batch> m_batch GUARDED_BY(m_batch_mutex);
 40+
 41+    // The function is executed within the context of a Batch which may no
 42+    // longer be stored in m_batch while stopping.
 43+    void WorkerThread(Batch& batch) EXCLUSIVE_LOCKS_REQUIRED(!batch.m_mutex)
 44     {
 45-        WAIT_LOCK(m_mutex, wait_lock);
 46+        WAIT_LOCK(batch.m_mutex, wait_lock);
 47         for (;;) {
 48-            std::packaged_task<void()> task;
 49-            {
 50-                // Wait only if needed; avoid sleeping when a new task was submitted while we were processing another one.
 51-                if (!m_interrupt && m_work_queue.empty()) {
 52-                    // Block until the pool is interrupted or a task is available.
 53-                    m_cv.wait(wait_lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(m_mutex) { return m_interrupt || !m_work_queue.empty(); });
 54-                }
 55-
 56-                // If stopped and no work left, exit worker
 57-                if (m_interrupt && m_work_queue.empty()) {
 58-                    return;
 59-                }
 60-
 61-                task = std::move(m_work_queue.front());
 62-                m_work_queue.pop();
 63+            // Wait only if needed; avoid sleeping when a new task was submitted while we were processing another one.
 64+            if (!batch.m_interrupt && batch.m_work_queue.empty()) {
 65+                // Block until the pool is interrupted or a task is available.
 66+                batch.m_cv.wait(wait_lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(batch.m_mutex) { return batch.m_interrupt || !batch.m_work_queue.empty(); });
 67+            }
 68+
 69+            // If stopped and no work left, exit worker
 70+            if (batch.m_interrupt && batch.m_work_queue.empty()) {
 71+                return;
 72             }
 73 
 74+            std::packaged_task<void()> task{std::move(batch.m_work_queue.front())};
 75+            batch.m_work_queue.pop();
 76             {
 77                 // Execute the task without the lock
 78-                REVERSE_LOCK(wait_lock, m_mutex);
 79+                REVERSE_LOCK(wait_lock, batch.m_mutex);
 80                 task();
 81             }
 82         }
 83@@ -100,17 +102,20 @@ public:
 84      *
 85      * Must be called from a controller (non-worker) thread.
 86      */
 87-    void Start(int num_workers) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
 88+    void Start(int num_workers) EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
 89     {
 90         assert(num_workers > 0);
 91-        LOCK(m_mutex);
 92-        if (!m_workers.empty()) throw std::runtime_error("Thread pool already started");
 93-        m_interrupt = false; // Reset
 94+        LOCK(m_batch_mutex);
 95+        if (m_batch) throw std::runtime_error("Thread pool already started");
 96+
 97+        m_batch = std::make_unique<Batch>();
 98+        LOCK(m_batch->m_mutex);
 99+        m_batch->m_interrupt = false; // Reset
100 
101         // Create workers
102-        m_workers.reserve(num_workers);
103+        m_batch->m_workers.reserve(num_workers);
104         for (int i = 0; i < num_workers; i++) {
105-            m_workers.emplace_back(&util::TraceThread, strprintf("%s_pool_%d", m_name, i), [this] { WorkerThread(); });
106+            m_batch->m_workers.emplace_back(&util::TraceThread, strprintf("%s_pool_%d", m_name, i), [this, batch = m_batch.get()] { this->WorkerThread(*batch); });
107         }
108     }
109 
110@@ -122,24 +127,30 @@ public:
111      *
112      * Must be called from a controller (non-worker) thread.
113      */
114-    void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
115+    void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
116     {
117         // Notify workers and join them
118-        std::vector<std::thread> threads_to_join;
119+        std::unique_ptr<Batch> local_batch;
120         {
121-            LOCK(m_mutex);
122+            LOCK(m_batch_mutex);
123+            if (!m_batch) return;
124+
125+            LOCK(m_batch->m_mutex);
126             // Ensure Stop() is not called from a worker thread while workers are still registered,
127             // otherwise a self-join deadlock would occur.
128-            auto id = std::this_thread::get_id();
129-            for (const auto& worker : m_workers) assert(worker.get_id() != id);
130+            const auto id = std::this_thread::get_id();
131+            for (const auto& worker : m_batch->m_workers) assert(worker.get_id() != id);
132             // Early shutdown to return right away on any concurrent Submit() call
133-            m_interrupt = true;
134-            threads_to_join.swap(m_workers);
135+            m_batch->m_interrupt = true;
136+            local_batch.swap(m_batch);
137         }
138-        m_cv.notify_all();
139+        local_batch->m_cv.notify_all();
140+        std::vector<std::thread> threads_to_join;
141+        WITH_LOCK(local_batch->m_mutex, threads_to_join.swap(local_batch->m_workers));
142         for (auto& worker : threads_to_join) worker.join();
143         // Since we currently wait for tasks completion, sanity-check empty queue
144-        WITH_LOCK(m_mutex, Assume(m_work_queue.empty()));
145+        //WITH_LOCK(m_mutex, Assume(m_work_queue.empty())); // Racy! Another thread might have Start() + Submit() after the last join!
146+        WITH_LOCK(local_batch->m_mutex, Assume(local_batch->m_work_queue.empty()));
147         // Note: m_interrupt is left true until next Start()
148     }
149 
150@@ -151,19 +162,22 @@ public:
151      * Note: Ignoring the returned future requires guarding the task against
152      * uncaught exceptions, as they would otherwise be silently discarded.
153      */
154-    template <class F> [[nodiscard]] EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
155-    auto Submit(F&& fn)
156+    template <class F>
157+    [[nodiscard]] auto Submit(F&& fn) EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
158     {
159         std::packaged_task task{std::forward<F>(fn)};
160         auto future{task.get_future()};
161+        LOCK(m_batch_mutex);
162+        if (!m_batch) throw std::runtime_error("No active workers; cannot accept new tasks");
163         {
164-            LOCK(m_mutex);
165-            if (m_interrupt || m_workers.empty()) {
166+            LOCK(m_batch->m_mutex);
167+            assert(!m_batch->m_workers.empty());
168+            if (m_batch->m_interrupt) {
169                 throw std::runtime_error("No active workers; cannot accept new tasks");
170             }
171-            m_work_queue.emplace(std::move(task));
172+            m_batch->m_work_queue.emplace(std::move(task));
173         }
174-        m_cv.notify_one();
175+        m_batch->m_cv.notify_one();
176         return future;
177     }
178 
179@@ -171,16 +185,18 @@ public:
180      * [@brief](/bitcoin-bitcoin/contributor/brief/) Execute a single queued task synchronously.
181      * Removes one task from the queue and executes it on the calling thread.
182      */
183-    void ProcessTask() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
184+    void ProcessTask() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
185     {
186         std::packaged_task<void()> task;
187         {
188-            LOCK(m_mutex);
189-            if (m_work_queue.empty()) return;
190+            LOCK(m_batch_mutex);
191+            if (!m_batch) return;
192+            LOCK(m_batch->m_mutex);
193+            if (m_batch->m_work_queue.empty()) return;
194 
195             // Pop the task
196-            task = std::move(m_work_queue.front());
197-            m_work_queue.pop();
198+            task = std::move(m_batch->m_work_queue.front());
199+            m_batch->m_work_queue.pop();
200         }
201         task();
202     }
203@@ -191,20 +207,31 @@ public:
204      * Wakes all worker threads so they can drain the queue and exit.
205      * Unlike Stop(), this function does not wait for threads to finish.
206      */
207-    void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
208+    void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
209     {
210-        WITH_LOCK(m_mutex, m_interrupt = true);
211-        m_cv.notify_all();
212+        LOCK(m_batch_mutex);
213+        if (!m_batch) return;
214+        {
215+            LOCK(m_batch->m_mutex);
216+            m_batch->m_interrupt = true;
217+        }
218+        m_batch->m_cv.notify_all();
219     }
220 
221-    size_t WorkQueueSize() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
222+    size_t WorkQueueSize() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
223     {
224-        return WITH_LOCK(m_mutex, return m_work_queue.size());
225+        LOCK(m_batch_mutex);
226+        if (!m_batch) return 0;
227+        LOCK(m_batch->m_mutex);
228+        return m_batch->m_work_queue.size();
229     }
230 
231-    size_t WorkersCount() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
232+    size_t WorkersCount() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
233     {
234-        return WITH_LOCK(m_mutex, return m_workers.size());
235+        LOCK(m_batch_mutex);
236+        if (!m_batch) return 0;
237+        LOCK(m_batch->m_mutex);
238+        return m_batch->m_workers.size();
239     }
240 };
241

maflcko commented at 9:45 am on February 13, 2026:

style nit, could use a C++11 for loop here:

 0diff --git a/src/test/threadpool_tests.cpp b/src/test/threadpool_tests.cpp
 1index 46acf1d67d..41882345a2 100644
 2--- a/src/test/threadpool_tests.cpp
 3+++ b/src/test/threadpool_tests.cpp
 4@@ -56,8 +56,7 @@ std::vector<std::future<void>> BlockWorkers(ThreadPool& threadPool, const std::s
 5 
 6     // Fill all workers with blocking tasks
 7     std::vector<std::future<void>> blocking_tasks;
 8-    for (int i = 0; i < num_of_threads_to_block; i++) {
 9-        std::promise<void>& ready = ready_promises[i];
10+    for (auto& ready : ready_promises) {
11         blocking_tasks.emplace_back(threadPool.Submit([blocker_future, &ready]() {
12             ready.set_value();
13             blocker_future.wait();

maflcko commented at 10:25 am on February 13, 2026:

Also, there seems missing unit test coverage for what happens with queued tasks after an interrupt (they should be checked to still execute and not be dropped)?

furszy commented at 8:41 pm on February 17, 2026:

Nice catch. The shared session restructure seems too much for this, why not just adding a lifecycle mutex, so Start() and Stop() are synchronized? We just need to ensure no overlaps between these two methods. These two shouldn’t occur too often as threads generation and release are expensive.

Also, this pushes us more towards a pool RAII.

furszy commented at 10:00 pm on February 17, 2026:

Sure. Coverage added and reworked BlockWorkers.

furszy commented at 11:03 pm on February 19, 2026:

Done. Check it now please. Added a m_running flag to prevent concurrent Start() happening while Stop() is in progress. Plus test coverage for it.

hodlinator commented at 12:07 pm on February 24, 2026:

PR description and c9e6f2725b6efc4b41dd0e2851781a521684c172 commit message:

0-reverlock_tests.cpp
1+reverselock_tests.cpp

hodlinator commented at 1:20 pm on February 24, 2026:

Given that you want to avoid RAII for now, let’s follow that approach. Seems like a bit too much state though, how about merging m_running behavior into m_interrupted?

My diff also disallows much progress in Start() between the two locks of m_mutex in Stop() through throwing if m_interrupted has been set.

 0--- a/src/util/threadpool.h
 1+++ b/src/util/threadpool.h
 2@@ -54,8 +54,6 @@ private:
 3     // This ensures threads blocked on m_cv reliably observe the change and proceed correctly without missing signals.
 4     // Ref: https://en.cppreference.com/w/cpp/thread/condition_variable
 5     bool m_interrupt GUARDED_BY(m_mutex){false};
 6-    // Toggled throughout Stop() to prevent Start() from being called while workers are being joined.
 7-    bool m_stopping GUARDED_BY(m_mutex){false};
 8     std::vector<std::thread> m_workers GUARDED_BY(m_mutex);
 9 
10     void WorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
11@@ -107,9 +105,8 @@ public:
12     {
13         assert(num_workers > 0);
14         LOCK(m_mutex);
15-        if (m_stopping) throw std::runtime_error("Thread pool is stopping");
16+        if (m_interrupt) throw std::runtime_error("Thread pool has been interrupted or is stopping");
17         if (!m_workers.empty()) throw std::runtime_error("Thread pool already started");
18-        m_interrupt = false; // Reset
19 
20         // Create workers
21         m_workers.reserve(num_workers);
22@@ -137,7 +134,6 @@ public:
23             // otherwise a self-join deadlock would occur.
24             auto id = std::this_thread::get_id();
25             for (const auto& worker : m_workers) assert(worker.get_id() != id);
26-            m_stopping = true;
27             // Early shutdown to return right away on any concurrent Submit() call
28             m_interrupt = true;
29             threads_to_join.swap(m_workers);
30@@ -152,8 +148,7 @@ public:
31         LOCK(m_mutex);
32         Assume(m_work_queue.empty());
33         // Re-allow Start() now that all workers have exited
34-        m_stopping = false;
35-        // Note: m_interrupt is left true until next Start()
36+        m_interrupt = false;
37     }
38 
39     enum class SubmitError {

furszy commented at 4:23 pm on February 24, 2026:

not sure I understand the comment, reverselock_tests.cpp is being modified.

furszy commented at 10:11 pm on February 24, 2026:

Given that you want to avoid RAII for now, let’s follow that approach. Seems like a bit too much state though, how about merging m_running behavior into m_interrupted?

Sounds great. Taken, thanks.

hodlinator commented at 12:00 pm on February 25, 2026:

Sorry for the brevity. They are typos in the designated locations. (Edited original post for syntax highlighting).

hodlinator commented at 12:06 pm on February 25, 2026:

I tried dropping the fix commit (8cd4a4363fb85f5487a19ace82aa0d12d5fab450 “threadpool: guard against Start-Stop race”), and was unable to reproduce any failures by running the unit tests. The following test reliably deadlocks without the fix, and succeeds with the fix. Maybe you could replace test 11 with it, or add it as an extra test if you are able to reproduce failures with the current test 11?

 0// Test 11, Start() must not cause a deadlock when called during Stop()
 1BOOST_AUTO_TEST_CASE(start_mid_stop_does_not_deadlock)
 2{
 3    ThreadPool pool(POOL_NAME);
 4    pool.Start(NUM_WORKERS_DEFAULT);
 5
 6    std::latch stop_called{1};
 7    std::latch second_start_called{1};
 8    auto future = Submit(pool, [&stop_called, &second_start_called]{
 9        stop_called.wait();
10        second_start_called.wait();
11    });
12
13    std::latch stopper_running{1};
14    std::thread stopper([&stopper_running, &pool] {
15        stopper_running.count_down();
16        pool.Stop();
17    });
18    stopper_running.wait();
19    // Wait a bit in case the stopper thread was context switched before the Stop()-call.
20    UninterruptibleSleep(500ms);
21    stop_called.count_down();
22
23    try {
24        pool.Start(NUM_WORKERS_DEFAULT);
25    } catch (const std::runtime_error&) {
26        // Expected: Start() was correctly rejected while Stop() was in progress
27    }
28    second_start_called.count_down();
29    // Without the fix, calling Start() right above would set
30    // ThreadPool::m_interrupt = false, causing the worker threads from the
31    // initial Start() to never complete the join at the end of Stop(), causing
32    // the test to hang here:
33    stopper.join();
34
35    pool.Stop();
36}

furszy commented at 5:56 pm on February 25, 2026:

It was working for me before.. but now is not that consistent anymore. I like your test, we can go even simpler:

 0// Test 11, Start() must not cause a deadlock when called during Stop()
 1BOOST_AUTO_TEST_CASE(start_mid_stop_does_not_deadlock)
 2{
 3    ThreadPool threadPool(POOL_NAME);
 4    threadPool.Start(NUM_WORKERS_DEFAULT);
 5
 6    // Keep all workers busy so Stop() gets stuck waiting for them to finish during join()
 7    std::counting_semaphore<> workers_blocker(0);
 8    const auto blocking_tasks = BlockWorkers(threadPool, workers_blocker, NUM_WORKERS_DEFAULT);
 9
10    std::thread stopper_thread([&threadPool] { threadPool.Stop(); });
11
12    // Stop() takes ownership of the workers before joining them, so WorkersCount()
13    // hits 0 the moment Stop() is hanging waiting them to join. That is out signal
14    // to call Start() right into the middle of the joining phase.
15    while (threadPool.WorkersCount() != 0) {
16        UninterruptibleSleep(200ms); // let the OS breathe so it can switch context
17    }
18    // Now we know for sure the stopper thread is hanging while workers are still alive.
19    // Restart the pool and resume workers so the stopper thread can proceed.
20    // This will throw an exception only if the pool handles Start-Stop race properly,
21    // otherwise it will proceed and hang the stopper_thread.
22    try {
23        threadPool.Start(NUM_WORKERS_DEFAULT);
24    } catch (std::exception& e) {
25        BOOST_CHECK_EQUAL(e.what(), "Thread pool has been interrupted or is stopping");
26    }
27    workers_blocker.release(NUM_WORKERS_DEFAULT);
28    WAIT_FOR(blocking_tasks);
29
30    // If Stop() is stuck, joining the stopper thread will deadlock
31    stopper_thread.join();
32}

Without the comments, this is quite short and hangs 100% of the time for me. Thanks for the push.

hodlinator commented at 6:05 pm on February 25, 2026:

Neat simplification!

Could use std::this_thread::yield() instead of UninterruptibleSleep in that case, would probably make the test even quicker: https://en.cppreference.com/w/cpp/thread/yield.html

furszy commented at 6:12 pm on February 25, 2026:

Done.

hodlinator commented at 6:29 pm on February 25, 2026:

Benchmark of PR version with UninterruptibleSleep:

0₿ hyperfine "./build/bin/test_bitcoin -t threadpool_tests/start_mid_stop_does_not_deadlock"
1Benchmark 1: ./build/bin/test_bitcoin -t threadpool_tests/start_mid_stop_does_not_deadlock
2  Time (mean ± σ):     329.7 ms ±  14.5 ms    [User: 118.6 ms, System: 10.5 ms]
3  Range (min … max):   314.6 ms … 347.8 ms    10 runs

Recommendation of using yield():

0₿ hyperfine "./build/bin/test_bitcoin -t threadpool_tests/start_mid_stop_does_not_deadlock"
1Benchmark 1: ./build/bin/test_bitcoin -t threadpool_tests/start_mid_stop_does_not_deadlock
2  Time (mean ± σ):     111.0 ms ±   0.3 ms    [User: 107.3 ms, System: 4.1 ms]
3  Range (min … max):   110.3 ms … 111.8 ms    26 runs

~3x faster, not an order of magnitude, so no blocker.

furszy commented at 6:41 pm on February 25, 2026:

Sure, done.

in test/functional/test_framework/authproxy.py:198 in ddf227371e

194@@ -195,7 +195,7 @@ def _get_response(self):
195         content_type = http_response.getheader('Content-Type')
196         if content_type != 'application/json':
197             raise JSONRPCException(
198-                {'code': -342, 'message': 'non-JSON HTTP response with \'%i %s\' from server' % (http_response.status, http_response.reason)},
199+                {'code': -342, 'message': 'non-JSON HTTP response with \'%i %s\' from server: %s' % (http_response.status, http_response.reason, http_response.read().decode())},

maflcko commented at 8:12 pm on February 12, 2026:

nit: Would be nice to use format or an f-string instead. (It seems a bit odd to use Python to not type types, but then format strings that could throw TypeError)

furszy commented at 10:00 pm on February 17, 2026:

Done as suggested. Thanks

in src/test/util/common.h:6 in ddf227371e outdated

0@@ -0,0 +1,26 @@
1+// Copyright (c) The Bitcoin Core developers
2+// Distributed under the MIT software license, see the accompanying
3+// file COPYING or https://www.opensource.org/licenses/mit-license.php.
4+
5+#ifndef BITCOIN_TEST_UTIL_COMMON_H
6+#define BITCOIN_TEST_UTIL_COMMON_H

maflcko commented at 8:18 pm on February 12, 2026:

nit: Would be nice to call this checkers.h (or so). It is not really “common” if only a few tests use it.

furszy commented at 9:37 pm on February 17, 2026:

nit: Would be nice to call this checkers.h (or so). It is not really “common” if only a few tests use it.

I count 14 test files using it. Haven’t counted the number of tests but we can assume each file has at least 2 tests using it?

It seems like a common utility to me. But happy to change it if you a strong there.

furszy commented at 10:15 pm on February 24, 2026:

With the inclusion of 7c59f3499929e6c67176352cefbd775144348399, the file definitely includes shared components :). Closing this thread.

furszy force-pushed on Feb 17, 2026

furszy commented at 10:04 pm on February 17, 2026: member

Thanks for the feedback!

Have tackled everything except one point which will address it post-#34577. Have also rebased the PR on top of #34577 since both touch the pool code. Better to focus on that one first.

seduless commented at 1:25 am on February 18, 2026: contributor

Reviewed adaf57119b1ab567f736535762cf9fa5a83b113e

While testing, I noticed that commenting out while (ProcessTask()) {} in Stop() doesn’t cause any test failures, so the active-wait drain behavior isn’t currently covered by the test suite. Here’s a test that exercises it by verifying the calling thread processes queued tasks during Stop(), and provably fails if the active-wait is removed:

 0BOOST_AUTO_TEST_CASE(stop_active_wait_drains_queue)
 1{
 2    ThreadPool threadPool(POOL_NAME);
 3    threadPool.Start(NUM_WORKERS_DEFAULT);
 4
 5    std::counting_semaphore<> blocker(0);
 6    const auto blocking_tasks = BlockWorkers(threadPool, blocker, NUM_WORKERS_DEFAULT);
 7
 8    auto main_thread_id = std::this_thread::get_id();
 9    std::atomic<int> main_thread_tasks{0};
10    const int num_tasks = 20;
11    for (int i = 0; i < num_tasks; i++) {
12        (void)Submit(threadPool, [&main_thread_tasks, main_thread_id]() {
13            if (std::this_thread::get_id() == main_thread_id)
14                main_thread_tasks.fetch_add(1, std::memory_order_relaxed);
15        });
16    }
17    BOOST_CHECK_EQUAL(threadPool.WorkQueueSize(), static_cast<size_t>(num_tasks));
18
19    // Delay release so Stop() has a window to drain tasks from the calling thread
20    std::thread unblocker([&blocker]() {
21        UninterruptibleSleep(300ms);
22        blocker.release(NUM_WORKERS_DEFAULT);
23    });
24
25    threadPool.Stop();
26    unblocker.join();
27
28    BOOST_CHECK_GT(main_thread_tasks.load(), 0);
29    WAIT_FOR(blocking_tasks);
30}

gniumg-source referenced this in commit a93cf6c8d2 on Feb 18, 2026

gniumg-source referenced this in commit e73f089772 on Feb 18, 2026

DrahtBot added the label Needs rebase on Feb 18, 2026

furszy force-pushed on Feb 19, 2026

DrahtBot removed the label Needs rebase on Feb 19, 2026

DrahtBot added the label CI failed on Feb 19, 2026

furszy commented at 11:20 pm on February 19, 2026: member

@seduless, just pushed the test with a few small modifications. Since you haven’t contributed to any repository yet, I couldn’t find your email to add you as the commit author. You can send it to me via IRC or leaving a comment here, and I’ll give you authorship.

furszy force-pushed on Feb 19, 2026

seduless commented at 0:53 am on February 20, 2026: contributor

@furszy Thanks! Appreciate it: seduless@proton.me

furszy force-pushed on Feb 20, 2026

furszy commented at 1:51 am on February 20, 2026: member

@seduless done. Feel very welcome to keep reviewing!

DrahtBot removed the label CI failed on Feb 20, 2026

in src/test/util_string_tests.cpp:219 in 5561d0cc4c

215@@ -215,23 +216,23 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
216 
217         LineReader reader1(input, /*max_line_length=*/22);
218         // First line is exactly the length of max_line_length
219-        BOOST_CHECK_EQUAL(reader1.ReadLine(), "once upon a time there");
220+        BOOST_CHECK_EQUAL(*reader1.ReadLine(), "once upon a time there");

hodlinator commented at 12:06 pm on February 24, 2026:

In c9e6f2725b6efc4b41dd0e2851781a521684c172 “test: refactor, decouple HasReason from test framework machinery”:

All of these changes to prevent https://en.cppreference.com/w/cpp/utility/optional/operator_cmp.html (21), comparing optional with value, from being used should be dropped IMO.

0        BOOST_CHECK_EQUAL(reader1.ReadLine(), "once upon a time there");

Doing that requires keeping the #include of setup_common.h, or moving the ostream operator<< implementations from setup_common.h/cpp to common.h, which may be a more suitable place anyway.

  0diff --git a/src/test/arith_uint256_tests.cpp b/src/test/arith_uint256_tests.cpp
  1index 0a15458041..016b465871 100644
  2--- a/src/test/arith_uint256_tests.cpp
  3+++ b/src/test/arith_uint256_tests.cpp
  4@@ -3,7 +3,7 @@
  5 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
  6 
  7 #include <arith_uint256.h>
  8-#include <test/util/setup_common.h>
  9+#include <test/util/common.h>
 10 #include <uint256.h>
 11 
 12 #include <boost/test/unit_test.hpp>
 13diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
 14index 0fbe0f3c13..e4200cace2 100644
 15--- a/src/test/blockencodings_tests.cpp
 16+++ b/src/test/blockencodings_tests.cpp
 17@@ -10,6 +10,7 @@
 18 #include <test/util/random.h>
 19 #include <test/util/txmempool.h>
 20 
 21+#include <test/util/common.h>
 22 #include <test/util/setup_common.h>
 23 
 24 #include <boost/test/unit_test.hpp>
 25diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp
 26index d7d10dfb1a..25762e070d 100644
 27--- a/src/test/blockfilter_index_tests.cpp
 28+++ b/src/test/blockfilter_index_tests.cpp
 29@@ -12,6 +12,7 @@
 30 #include <node/miner.h>
 31 #include <pow.h>
 32 #include <test/util/blockfilter.h>
 33+#include <test/util/common.h>
 34 #include <test/util/setup_common.h>
 35 #include <validation.h>
 36 
 37diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp
 38index c8334dabe1..e89909dfaa 100644
 39--- a/src/test/blockfilter_tests.cpp
 40+++ b/src/test/blockfilter_tests.cpp
 41@@ -3,6 +3,7 @@
 42 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 43 
 44 #include <test/data/blockfilters.json.h>
 45+#include <test/util/common.h>
 46 #include <test/util/setup_common.h>
 47 
 48 #include <blockfilter.h>
 49diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
 50index db59931ba7..28bb7b70a7 100644
 51--- a/src/test/blockmanager_tests.cpp
 52+++ b/src/test/blockmanager_tests.cpp
 53@@ -14,6 +14,7 @@
 54 #include <validation.h>
 55 
 56 #include <boost/test/unit_test.hpp>
 57+#include <test/util/common.h>
 58 #include <test/util/logging.h>
 59 #include <test/util/setup_common.h>
 60 
 61diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp
 62index 8e02cfd08c..ed333c34c0 100644
 63--- a/src/test/bloom_tests.cpp
 64+++ b/src/test/bloom_tests.cpp
 65@@ -13,6 +13,7 @@
 66 #include <random.h>
 67 #include <serialize.h>
 68 #include <streams.h>
 69+#include <test/util/common.h>
 70 #include <test/util/random.h>
 71 #include <test/util/setup_common.h>
 72 #include <uint256.h>
 73diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp
 74index 5588d4cdbc..b348793bfb 100644
 75--- a/src/test/crypto_tests.cpp
 76+++ b/src/test/crypto_tests.cpp
 77@@ -17,6 +17,7 @@
 78 #include <crypto/muhash.h>
 79 #include <random.h>
 80 #include <streams.h>
 81+#include <test/util/common.h>
 82 #include <test/util/random.h>
 83 #include <test/util/setup_common.h>
 84 #include <util/strencodings.h>
 85diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp
 86index d3a9e54348..3896ea64da 100644
 87--- a/src/test/dbwrapper_tests.cpp
 88+++ b/src/test/dbwrapper_tests.cpp
 89@@ -3,6 +3,7 @@
 90 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 91 
 92 #include <dbwrapper.h>
 93+#include <test/util/common.h>
 94 #include <test/util/random.h>
 95 #include <test/util/setup_common.h>
 96 #include <uint256.h>
 97diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
 98index ec17fe3997..d349ceea44 100644
 99--- a/src/test/getarg_tests.cpp
100+++ b/src/test/getarg_tests.cpp
101@@ -5,6 +5,7 @@
102 #include <common/args.h>
103 #include <common/settings.h>
104 #include <logging.h>
105+#include <test/util/common.h>
106 #include <test/util/setup_common.h>
107 #include <univalue.h>
108 #include <util/strencodings.h>
109diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp
110index f9426fa311..bba612f8b4 100644
111--- a/src/test/headers_sync_chainwork_tests.cpp
112+++ b/src/test/headers_sync_chainwork_tests.cpp
113@@ -8,6 +8,7 @@
114 #include <headerssync.h>
115 #include <net_processing.h>
116 #include <pow.h>
117+#include <test/util/common.h>
118 #include <test/util/setup_common.h>
119 #include <validation.h>
120 
121diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp
122index da0f5eecce..1a98256ce2 100644
123--- a/src/test/interfaces_tests.cpp
124+++ b/src/test/interfaces_tests.cpp
125@@ -5,6 +5,7 @@
126 #include <chainparams.h>
127 #include <consensus/validation.h>
128 #include <interfaces/chain.h>
129+#include <test/util/common.h>
130 #include <test/util/setup_common.h>
131 #include <script/solver.h>
132 #include <validation.h>
133diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp
134index acced716f0..b0c052af0f 100644
135--- a/src/test/key_tests.cpp
136+++ b/src/test/key_tests.cpp
137@@ -9,6 +9,7 @@
138 #include <span.h>
139 #include <streams.h>
140 #include <secp256k1_extrakeys.h>
141+#include <test/util/common.h>
142 #include <test/util/random.h>
143 #include <test/util/setup_common.h>
144 #include <uint256.h>
145diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
146index 8ead946f7f..f232bb4181 100644
147--- a/src/test/logging_tests.cpp
148+++ b/src/test/logging_tests.cpp
149@@ -6,6 +6,7 @@
150 #include <logging.h>
151 #include <logging/timer.h>
152 #include <scheduler.h>
153+#include <test/util/common.h>
154 #include <test/util/logging.h>
155 #include <test/util/setup_common.h>
156 #include <tinyformat.h>
157diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp
158index dc4791419e..a7212147b6 100644
159--- a/src/test/merkle_tests.cpp
160+++ b/src/test/merkle_tests.cpp
161@@ -3,6 +3,7 @@
162 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
163 
164 #include <consensus/merkle.h>
165+#include <test/util/common.h>
166 #include <test/util/random.h>
167 #include <test/util/setup_common.h>
168 
169diff --git a/src/test/merkleblock_tests.cpp b/src/test/merkleblock_tests.cpp
170index 00a1f0b92d..a461e04300 100644
171--- a/src/test/merkleblock_tests.cpp
172+++ b/src/test/merkleblock_tests.cpp
173@@ -3,6 +3,7 @@
174 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
175 
176 #include <merkleblock.h>
177+#include <test/util/common.h>
178 #include <test/util/setup_common.h>
179 #include <uint256.h>
180 
181diff --git a/src/test/minisketch_tests.cpp b/src/test/minisketch_tests.cpp
182index 1d6f2a12ef..ba895478de 100644
183--- a/src/test/minisketch_tests.cpp
184+++ b/src/test/minisketch_tests.cpp
185@@ -5,6 +5,7 @@
186 #include <minisketch.h>
187 #include <node/minisketchwrapper.h>
188 #include <random.h>
189+#include <test/util/common.h>
190 #include <test/util/random.h>
191 #include <test/util/setup_common.h>
192 
193diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp
194index 932b83fee4..0b1cc98086 100644
195--- a/src/test/multisig_tests.cpp
196+++ b/src/test/multisig_tests.cpp
197@@ -9,6 +9,7 @@
198 #include <script/script_error.h>
199 #include <script/sign.h>
200 #include <script/signingprovider.h>
201+#include <test/util/common.h>
202 #include <test/util/setup_common.h>
203 #include <test/util/transaction_utils.h>
204 #include <tinyformat.h>
205diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
206index 4770cabf7a..167b88f5f3 100644
207--- a/src/test/netbase_tests.cpp
208+++ b/src/test/netbase_tests.cpp
209@@ -10,7 +10,7 @@
210 #include <protocol.h>
211 #include <serialize.h>
212 #include <streams.h>
213-#include <test/util/setup_common.h>
214+#include <test/util/common.h>
215 #include <util/strencodings.h>
216 #include <util/translation.h>
217 
218@@ -22,7 +22,7 @@
219 using namespace std::literals;
220 using namespace util::hex_literals;
221 
222-BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)
223+BOOST_AUTO_TEST_SUITE(netbase_tests)
224 
225 static CNetAddr ResolveIP(const std::string& ip)
226 {
227diff --git a/src/test/node_init_tests.cpp b/src/test/node_init_tests.cpp
228index a4292e66ca..44635dd4aa 100644
229--- a/src/test/node_init_tests.cpp
230+++ b/src/test/node_init_tests.cpp
231@@ -7,6 +7,7 @@
232 #include <rpc/server.h>
233 
234 #include <boost/test/unit_test.hpp>
235+#include <test/util/common.h>
236 #include <test/util/setup_common.h>
237 
238 using node::NodeContext;
239diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
240index 933736dcc1..6ebfab34c5 100644
241--- a/src/test/orphanage_tests.cpp
242+++ b/src/test/orphanage_tests.cpp
243@@ -10,6 +10,7 @@
244 #include <pubkey.h>
245 #include <script/sign.h>
246 #include <script/signingprovider.h>
247+#include <test/util/common.h>
248 #include <test/util/random.h>
249 #include <test/util/setup_common.h>
250 #include <test/util/transaction_utils.h>
251diff --git a/src/test/pcp_tests.cpp b/src/test/pcp_tests.cpp
252index c71c9de160..51b004a5d5 100644
253--- a/src/test/pcp_tests.cpp
254+++ b/src/test/pcp_tests.cpp
255@@ -5,6 +5,7 @@
256 #include <common/pcp.h>
257 #include <netbase.h>
258 #include <test/util/logging.h>
259+#include <test/util/common.h>
260 #include <test/util/setup_common.h>
261 #include <util/time.h>
262 
263diff --git a/src/test/pow_tests.cpp b/src/test/pow_tests.cpp
264index 8de76ff21f..f6123401ad 100644
265--- a/src/test/pow_tests.cpp
266+++ b/src/test/pow_tests.cpp
267@@ -6,6 +6,7 @@
268 #include <chainparams.h>
269 #include <pow.h>
270 #include <test/util/random.h>
271+#include <test/util/common.h>
272 #include <test/util/setup_common.h>
273 #include <util/chaintype.h>
274 
275diff --git a/src/test/rest_tests.cpp b/src/test/rest_tests.cpp
276index 5dc122507f..fefc41bd73 100644
277--- a/src/test/rest_tests.cpp
278+++ b/src/test/rest_tests.cpp
279@@ -3,6 +3,7 @@
280 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
281 
282 #include <rest.h>
283+#include <test/util/common.h>
284 #include <test/util/setup_common.h>
285 
286 #include <boost/test/unit_test.hpp>
287diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
288index f8d75a8022..b435fb8992 100644
289--- a/src/test/rpc_tests.cpp
290+++ b/src/test/rpc_tests.cpp
291@@ -9,6 +9,7 @@
292 #include <rpc/client.h>
293 #include <rpc/server.h>
294 #include <rpc/util.h>
295+#include <test/util/common.h>
296 #include <test/util/setup_common.h>
297 #include <univalue.h>
298 #include <util/time.h>
299diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
300index c6d3b125eb..63215895ff 100644
301--- a/src/test/script_standard_tests.cpp
302+++ b/src/test/script_standard_tests.cpp
303@@ -10,16 +10,24 @@
304 #include <script/script.h>
305 #include <script/signingprovider.h>
306 #include <script/solver.h>
307-#include <test/util/setup_common.h>
308+#include <test/util/common.h>
309 #include <util/strencodings.h>
310 
311 #include <boost/test/unit_test.hpp>
312+#include <boost/test/unit_test_suite.hpp>
313 
314 #include <univalue.h>
315 
316 using namespace util::hex_literals;
317 
318-BOOST_FIXTURE_TEST_SUITE(script_standard_tests, BasicTestingSetup)
319+namespace {
320+struct ScriptStandardTest {
321+    ECC_Context ecc;
322+    ScriptStandardTest() { SelectParams(ChainType::MAIN); }
323+};
324+} // namespace
325+
326+BOOST_FIXTURE_TEST_SUITE(script_standard_tests, ScriptStandardTest)
327 
328 BOOST_AUTO_TEST_CASE(dest_default_is_no_dest)
329 {
330diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
331index 2134bcc179..1478cf9125 100644
332--- a/src/test/script_tests.cpp
333+++ b/src/test/script_tests.cpp
334@@ -19,6 +19,7 @@
335 #include <streams.h>
336 #include <test/util/json.h>
337 #include <test/util/random.h>
338+#include <test/util/common.h>
339 #include <test/util/setup_common.h>
340 #include <test/util/transaction_utils.h>
341 #include <util/fs.h>
342@@ -26,7 +27,6 @@
343 #include <util/string.h>
344 
345 #include <cstdint>
346-#include <fstream>
347 #include <string>
348 #include <vector>
349 
350diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp
351index 33a9133d2e..f646ba5faf 100644
352--- a/src/test/serialize_tests.cpp
353+++ b/src/test/serialize_tests.cpp
354@@ -5,6 +5,7 @@
355 #include <hash.h>
356 #include <serialize.h>
357 #include <streams.h>
358+#include <test/util/common.h>
359 #include <test/util/setup_common.h>
360 #include <util/strencodings.h>
361 
362diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp
363index 5d65c35a8c..5afdea43d8 100644
364--- a/src/test/sighash_tests.cpp
365+++ b/src/test/sighash_tests.cpp
366@@ -11,6 +11,7 @@
367 #include <serialize.h>
368 #include <streams.h>
369 #include <test/data/sighash.json.h>
370+#include <test/util/common.h>
371 #include <test/util/json.h>
372 #include <test/util/random.h>
373 #include <test/util/setup_common.h>
374diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp
375index e9c6160ebf..d7ddef905b 100644
376--- a/src/test/txdownload_tests.cpp
377+++ b/src/test/txdownload_tests.cpp
378@@ -8,6 +8,7 @@
379 #include <node/txdownloadman_impl.h>
380 #include <primitives/transaction.h>
381 #include <script/script.h>
382+#include <test/util/common.h>
383 #include <test/util/random.h>
384 #include <test/util/setup_common.h>
385 #include <validation.h>
386diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
387index 24760664c1..06c5816b72 100644
388--- a/src/test/txpackage_tests.cpp
389+++ b/src/test/txpackage_tests.cpp
390@@ -11,6 +11,7 @@
391 #include <script/script.h>
392 #include <serialize.h>
393 #include <streams.h>
394+#include <test/util/common.h>
395 #include <test/util/random.h>
396 #include <test/util/script.h>
397 #include <test/util/setup_common.h>
398diff --git a/src/test/txreconciliation_tests.cpp b/src/test/txreconciliation_tests.cpp
399index d4ed3511f4..8d1a47b6e3 100644
400--- a/src/test/txreconciliation_tests.cpp
401+++ b/src/test/txreconciliation_tests.cpp
402@@ -4,11 +4,11 @@
403 
404 #include <node/txreconciliation.h>
405 
406-#include <test/util/setup_common.h>
407+#include <test/util/common.h>
408 
409 #include <boost/test/unit_test.hpp>
410 
411-BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup)
412+BOOST_AUTO_TEST_SUITE(txreconciliation_tests)
413 
414 BOOST_AUTO_TEST_CASE(RegisterPeerTest)
415 {
416diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp
417index b16b60474a..2a4f213c4b 100644
418--- a/src/test/txvalidation_tests.cpp
419+++ b/src/test/txvalidation_tests.cpp
420@@ -11,6 +11,7 @@
421 #include <primitives/transaction.h>
422 #include <random.h>
423 #include <script/script.h>
424+#include <test/util/common.h>
425 #include <test/util/setup_common.h>
426 #include <test/util/txmempool.h>
427 #include <validation.h>
428diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp
429index a4ebc049fb..0de391d8cc 100644
430--- a/src/test/uint256_tests.cpp
431+++ b/src/test/uint256_tests.cpp
432@@ -4,7 +4,7 @@
433 
434 #include <primitives/transaction_identifier.h>
435 #include <streams.h>
436-#include <test/util/setup_common.h>
437+#include <test/util/common.h>
438 #include <uint256.h>
439 #include <util/strencodings.h>
440 
441diff --git a/src/test/util/common.h b/src/test/util/common.h
442index 788df0d3fe..9d5b70d9c8 100644
443--- a/src/test/util/common.h
444+++ b/src/test/util/common.h
445@@ -5,6 +5,8 @@
446 #ifndef BITCOIN_TEST_UTIL_COMMON_H
447 #define BITCOIN_TEST_UTIL_COMMON_H
448 
449+#include <primitives/transaction_identifier.h>
450+#include <arith_uint256.h>
451 #include <string>
452 
453 /**
454@@ -23,4 +25,47 @@ private:
455     const std::string m_reason;
456 };
457 
458+// Make types usable in BOOST_CHECK_* @{
459+namespace std {
460+template <typename T> requires std::is_enum_v<T>
461+inline std::ostream& operator<<(std::ostream& os, const T& e)
462+{
463+    return os << static_cast<std::underlying_type_t<T>>(e);
464+}
465+
466+template <typename T>
467+inline std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
468+{
469+    return v ? os << *v
470+             : os << "std::nullopt";
471+}
472+} // namespace std
473+
474+inline std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
475+{
476+    return os << num.ToString();
477+}
478+
479+inline std::ostream& operator<<(std::ostream& os, const uint160& num)
480+{
481+    return os << num.ToString();
482+}
483+
484+inline std::ostream& operator<<(std::ostream& os, const uint256& num)
485+{
486+    return os << num.ToString();
487+}
488+
489+inline std::ostream& operator<<(std::ostream& os, const Txid& txid)
490+{
491+    return os << txid.ToString();
492+}
493+
494+inline std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid)
495+{
496+    return os << wtxid.ToString();
497+}
498+
499+// @}
500+
501 #endif // BITCOIN_TEST_UTIL_COMMON_H
502diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
503index 995f532fd7..6fdc9e1882 100644
504--- a/src/test/util/setup_common.cpp
505+++ b/src/test/util/setup_common.cpp
506@@ -620,26 +620,3 @@ CBlock getBlock13b8a()
507     stream >> TX_WITH_WITNESS(block);
508     return block;
509 }
510-
511-std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
512-{
513-    return os << num.ToString();
514-}
515-
516-std::ostream& operator<<(std::ostream& os, const uint160& num)
517-{
518-    return os << num.ToString();
519-}
520-
521-std::ostream& operator<<(std::ostream& os, const uint256& num)
522-{
523-    return os << num.ToString();
524-}
525-
526-std::ostream& operator<<(std::ostream& os, const Txid& txid) {
527-    return os << txid.ToString();
528-}
529-
530-std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid) {
531-    return os << wtxid.ToString();
532-}
533diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
534index 1f71b21a25..c02a4fd331 100644
535--- a/src/test/util/setup_common.h
536+++ b/src/test/util/setup_common.h
537@@ -261,27 +261,4 @@ std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::
538 
539 CBlock getBlock13b8a();
540 
541-// Make types usable in BOOST_CHECK_* @{
542-namespace std {
543-template <typename T> requires std::is_enum_v<T>
544-inline std::ostream& operator<<(std::ostream& os, const T& e)
545-{
546-    return os << static_cast<std::underlying_type_t<T>>(e);
547-}
548-
549-template <typename T>
550-inline std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
551-{
552-    return v ? os << *v
553-             : os << "std::nullopt";
554-}
555-} // namespace std
556-
557-std::ostream& operator<<(std::ostream& os, const arith_uint256& num);
558-std::ostream& operator<<(std::ostream& os, const uint160& num);
559-std::ostream& operator<<(std::ostream& os, const uint256& num);
560-std::ostream& operator<<(std::ostream& os, const Txid& txid);
561-std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid);
562-// @}
563-
564 #endif // BITCOIN_TEST_UTIL_SETUP_COMMON_H
565diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
566index 69a6c9c110..7757695085 100644
567--- a/src/test/util_string_tests.cpp
568+++ b/src/test/util_string_tests.cpp
569@@ -216,15 +216,15 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
570 
571         LineReader reader1(input, /*max_line_length=*/22);
572         // First line is exactly the length of max_line_length
573-        BOOST_CHECK_EQUAL(*reader1.ReadLine(), "once upon a time there");
574+        BOOST_CHECK_EQUAL(reader1.ReadLine(), "once upon a time there");
575         // Second line is +1 character too long
576         BOOST_CHECK_EXCEPTION(reader1.ReadLine(), std::runtime_error, HasReason{"max_line_length exceeded by LineReader"});
577 
578         // Increase max_line_length by 1
579         LineReader reader2(input, /*max_line_length=*/23);
580         // Both lines fit within limit
581-        BOOST_CHECK_EQUAL(*reader2.ReadLine(), "once upon a time there");
582-        BOOST_CHECK_EQUAL(*reader2.ReadLine(), "was a dog who liked tea");
583+        BOOST_CHECK_EQUAL(reader2.ReadLine(), "once upon a time there");
584+        BOOST_CHECK_EQUAL(reader2.ReadLine(), "was a dog who liked tea");
585         // End of buffer reached
586         BOOST_CHECK(!reader2.ReadLine());
587     }
588@@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
589         // Empty lines are empty
590         const std::vector<std::byte> input{StringToBuffer("\n")};
591         LineReader reader(input, /*max_line_length=*/1024);
592-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "");
593+        BOOST_CHECK_EQUAL(reader.ReadLine(), "");
594         BOOST_CHECK(!reader.ReadLine());
595     }
596     {
597@@ -251,15 +251,15 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
598     {
599         const std::vector<std::byte> input{StringToBuffer("a\nb\n")};
600         LineReader reader(input, /*max_line_length=*/1);
601-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "a");
602-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "b");
603+        BOOST_CHECK_EQUAL(reader.ReadLine(), "a");
604+        BOOST_CHECK_EQUAL(reader.ReadLine(), "b");
605         BOOST_CHECK(!reader.ReadLine());
606     }
607     {
608         // If ReadLine fails, the iterator is reset and we can ReadLength instead
609         const std::vector<std::byte> input{StringToBuffer("a\nbaboon\n")};
610         LineReader reader(input, /*max_line_length=*/1);
611-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "a");
612+        BOOST_CHECK_EQUAL(reader.ReadLine(), "a");
613         // "baboon" is too long
614         BOOST_CHECK_EXCEPTION(reader.ReadLine(), std::runtime_error, HasReason{"max_line_length exceeded by LineReader"});
615         BOOST_CHECK_EQUAL(reader.ReadLength(1), "b");
616@@ -268,7 +268,7 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
617         // "on" is too long
618         BOOST_CHECK_EXCEPTION(reader.ReadLine(), std::runtime_error, HasReason{"max_line_length exceeded by LineReader"});
619         BOOST_CHECK_EQUAL(reader.ReadLength(1), "o");
620-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "n"); // now the remainder of the buffer fits in one line
621+        BOOST_CHECK_EQUAL(reader.ReadLine(), "n"); // now the remainder of the buffer fits in one line
622         BOOST_CHECK(!reader.ReadLine());
623     }
624     {
625diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
626index f91b30a307..083a32da96 100644
627--- a/src/test/validation_block_tests.cpp
628+++ b/src/test/validation_block_tests.cpp
629@@ -10,6 +10,7 @@
630 #include <node/miner.h>
631 #include <pow.h>
632 #include <random.h>
633+#include <test/util/common.h>
634 #include <test/util/random.h>
635 #include <test/util/script.h>
636 #include <test/util/setup_common.h>
637diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
638index 9e2c710977..df5e522edd 100644
639--- a/src/test/validation_chainstate_tests.cpp
640+++ b/src/test/validation_chainstate_tests.cpp
641@@ -9,6 +9,7 @@
642 #include <rpc/blockchain.h>
643 #include <sync.h>
644 #include <test/util/chainstate.h>
645+#include <test/util/common.h>
646 #include <test/util/coins.h>
647 #include <test/util/random.h>
648 #include <test/util/setup_common.h>
649diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
650index 40f99690ce..06de793d25 100644
651--- a/src/test/validation_chainstatemanager_tests.cpp
652+++ b/src/test/validation_chainstatemanager_tests.cpp
653@@ -12,6 +12,7 @@
654 #include <rpc/blockchain.h>
655 #include <sync.h>
656 #include <test/util/chainstate.h>
657+#include <test/util/common.h>
658 #include <test/util/logging.h>
659 #include <test/util/random.h>
660 #include <test/util/setup_common.h>
661diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
662index 66c284b979..c35cef7f45 100644
663--- a/src/test/validation_flush_tests.cpp
664+++ b/src/test/validation_flush_tests.cpp
665@@ -5,6 +5,7 @@
666 #include <sync.h>
667 #include <test/util/coins.h>
668 #include <test/util/random.h>
669+#include <test/util/common.h>
670 #include <test/util/setup_common.h>
671 #include <validation.h>
672 
673diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp
674index ea1044023e..ad79205311 100644
675--- a/src/test/versionbits_tests.cpp
676+++ b/src/test/versionbits_tests.cpp
677@@ -6,6 +6,7 @@
678 #include <chainparams.h>
679 #include <consensus/params.h>
680 #include <test/util/random.h>
681+#include <test/util/common.h>
682 #include <test/util/setup_common.h>
683 #include <util/chaintype.h>
684 #include <versionbits.h>
685diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
686index 7f55d76585..92a35b08af 100644
687--- a/src/wallet/test/coinselector_tests.cpp
688+++ b/src/wallet/test/coinselector_tests.cpp
689@@ -7,6 +7,7 @@
690 #include <policy/policy.h>
691 #include <primitives/transaction.h>
692 #include <random.h>
693+#include <test/util/common.h>
694 #include <test/util/setup_common.h>
695 #include <util/translation.h>
696 #include <wallet/coincontrol.h>
697diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp
698index 8b71634ed8..532bdce384 100644
699--- a/src/wallet/test/db_tests.cpp
700+++ b/src/wallet/test/db_tests.cpp
701@@ -4,6 +4,7 @@
702 
703 #include <boost/test/unit_test.hpp>
704 
705+#include <test/util/common.h>
706 #include <test/util/setup_common.h>
707 #include <util/check.h>
708 #include <util/fs.h>
709@@ -14,7 +15,6 @@
710 #include <wallet/walletutil.h>
711 
712 #include <cstddef>
713-#include <fstream>
714 #include <memory>
715 #include <span>
716 #include <string>
717diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
718index 36782cd187..54f453623e 100644
719--- a/src/wallet/test/wallet_tests.cpp
720+++ b/src/wallet/test/wallet_tests.cpp
721@@ -17,6 +17,7 @@
722 #include <policy/policy.h>
723 #include <rpc/server.h>
724 #include <script/solver.h>
725+#include <test/util/common.h>
726 #include <test/util/logging.h>
727 #include <test/util/random.h>
728 #include <test/util/setup_common.h>
729diff --git a/src/wallet/test/wallet_transaction_tests.cpp b/src/wallet/test/wallet_transaction_tests.cpp
730index e49439065a..38be52f41f 100644
731--- a/src/wallet/test/wallet_transaction_tests.cpp
732+++ b/src/wallet/test/wallet_transaction_tests.cpp
733@@ -4,6 +4,7 @@
734 
735 #include <wallet/transaction.h>
736 
737+#include <test/util/common.h>
738 #include <wallet/test/wallet_test_fixture.h>
739 
740 #include <boost/test/unit_test.hpp>
741diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
742index bc994ba606..1463ee98b3 100644
743--- a/src/wallet/test/walletload_tests.cpp
744+++ b/src/wallet/test/walletload_tests.cpp
745@@ -4,6 +4,7 @@
746 
747 #include <wallet/test/util.h>
748 #include <wallet/wallet.h>
749+#include <test/util/common.h>
750 #include <test/util/logging.h>
751 #include <test/util/setup_common.h>
752

furszy commented at 3:54 pm on February 24, 2026:

Have you tried defining it for all types at once? It would be nice to avoid adding the two extra dependencies added to common.h. Something like:

0template <typename T>
1concept HasToString = requires(const T& t) { t.ToString(); };
2
3template <typename T> requires HasToString<T>
4inline std::ostream& operator<<(std::ostream& os, const T& obj)
5{
6    return os << obj.ToString();
7}

sipa commented at 4:05 pm on February 24, 2026:

(total flyby comment)

template <typename T> requires HasToString<T> is equivalent to template<HasToString T>.

hodlinator commented at 4:10 pm on February 24, 2026:

Have you tried defining it for all types at once?

No, but it was very tempting. 🙂

furszy commented at 4:19 pm on February 24, 2026:

(total flyby comment)

template <typename T> requires HasToString<T> is equivalent to template<HasToString T>.

I guess we could also fully inline it duplicating the requires? The first one should be the clause and the second one the expression?

0template <typename T> requires requires(const T& t) { t.ToString(); };

Probably better to keep yours even if this works, the double requires meaning hurt my eyes.

furszy commented at 10:21 pm on February 24, 2026:

Pushed 8048bff0eb544da01291166fb207c0e52aeaed2e with your changes + this new template. Gave you authorship for it @hodlinator. I merely contributed with the final idea. Thanks!

hodlinator commented at 12:55 pm on February 25, 2026:

Thanks for taking and improving my suggestion! Maybe could add a note about the large number of touched files in the PR description to re-assure potential reviewers. If the large number of touched files is a hindrance for review, maybe one could split the stream helpers and HasReason in common.h into different files and smuggle in an include for the stream-helpers into setup_common.h for this PR, but let’s see how it goes.

in src/test/threadpool_tests.cpp:403 in 5561d0cc4c

401+        (void)Submit(threadPool, [&main_thread_tasks, main_thread_id]() {
402+            if (std::this_thread::get_id() == main_thread_id)
403+                main_thread_tasks.fetch_add(1, std::memory_order_relaxed);
404+        });
405+    }
406+    BOOST_CHECK_EQUAL(threadPool.WorkQueueSize(), static_cast<size_t>(num_tasks));

hodlinator commented at 12:20 pm on February 24, 2026:

nit: Could avoid cast:

0    const size_t num_tasks = 20;
1    for (size_t i = 0; i < num_tasks; i++) {
2        (void)Submit(threadPool, [&main_thread_tasks, main_thread_id]() {
3            if (std::this_thread::get_id() == main_thread_id)
4                main_thread_tasks.fetch_add(1, std::memory_order_relaxed);
5        });
6    }
7    BOOST_CHECK_EQUAL(threadPool.WorkQueueSize(), num_tasks);

furszy commented at 10:19 pm on February 24, 2026:

Done as suggested.

hodlinator commented at 1:37 pm on February 24, 2026: contributor

Reviewed 5561d0cc4c53c588ff299fb7ad5d43d8de67fe15

furszy force-pushed on Feb 24, 2026

DrahtBot added the label CI failed on Feb 24, 2026

test: cleanup, block threads via semaphore instead of shared_future

No-behavior change.

9ff1e82e7d

threadpool: guard against Start-Stop race

Stop() has two windows where Start() could cause troubles:

1) m_workers is temporarily empty while workers are being joined,
this creates a window where Start() could slip through and reset
m_interrupt to false, preventing the old workers from exiting and
causing a deadlock.

2) Start() could be called after workers are joined but before the
empty() sanity check on m_work_queue, causing a crash.

Fix both races by keeping m_interrupt set for the entire duration
of Stop(), so any concurrent Start() call is rejected until all
workers have exited.

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

8cd4a4363f

furszy force-pushed on Feb 24, 2026

furszy commented at 10:34 pm on February 24, 2026: member

Updated per feedback. Thanks @hodlinator. The main change is using the interrupted flag, instead of introducing a new one, to fix the concurrent Start-Stop edge case race. Beyond that, extended test coverage for all submission errors and expanded the test/util/common.h file with a generic template for the BOOST_CHECK* helpers, which simplifies the HasReason refactoring commit and cleans up a good number of setup_common.h unneeded dependencies (see #34562 (review) for background).

Also, rebased on master to fix silent conflicts with #34653.

This PR now looks long but all commits should be straightforward to review.

furszy force-pushed on Feb 24, 2026

DrahtBot removed the label CI failed on Feb 24, 2026

in src/test/util/common.h:47 in 64c48098b7

42+} // namespace std
43+
44+template <typename T>
45+concept HasToString = requires(const T& t) { t.ToString(); };
46+
47+template<HasToString T>

hodlinator commented at 12:44 pm on February 25, 2026:

nit: Pretty sure clang-format would prefer:

0template <HasToString T>

furszy commented at 6:07 pm on February 25, 2026:

Upps, my bad. Fixed.

hodlinator commented at 12:56 pm on February 25, 2026: contributor

Reviewed 64c48098b745bea3afdfa522b199a585c091dc36

furszy force-pushed on Feb 25, 2026

furszy commented at 6:11 pm on February 25, 2026: member

Updated per feedback, thanks @hodlinator.

If the large number of touched files is a hindrance for review, maybe one could split the stream helpers and HasReason in common.h into different files and smuggle in an include for the stream-helpers into setup_common.h for this PR, but let’s see how it goes.

I don’t think they matter much, all the modified files are unit tests that rarely change, and even more rarely introduce a new dependency, which would be the main source of conflicts for us. I would say we are good here, but happy to read more thoughts.

hodlinator approved

hodlinator commented at 6:37 pm on February 25, 2026: contributor

ACK e771c5cdd7eba3cfcf66d9809152a5c1855d73e7

furszy force-pushed on Feb 25, 2026

DrahtBot added the label CI failed on Feb 25, 2026

furszy commented at 6:42 pm on February 25, 2026: member

Updated per feedback, thanks. Replaced UninterruptibleSleep() for yield() call in the test.

hodlinator approved

hodlinator commented at 6:42 pm on February 25, 2026: contributor

re-ACK 566f7f81de967065afebc153ec41dc7b8d8300d0

DrahtBot removed the label CI failed on Feb 25, 2026

achow101 commented at 8:45 pm on February 25, 2026: member

ACK 566f7f81de967065afebc153ec41dc7b8d8300d0

in src/test/threadpool_tests.cpp:333 in e9cab3ec6d outdated

328+    const auto blocking_tasks = BlockWorkers(threadPool, workers_blocker, NUM_WORKERS_DEFAULT);
329+
330+    std::thread stopper_thread([&threadPool] { threadPool.Stop(); });
331+
332+    // Stop() takes ownership of the workers before joining them, so WorkersCount()
333+    // hits 0 the moment Stop() is hanging waiting them to join. That is out signal

maflcko commented at 10:50 am on February 26, 2026:

nit in e9cab3ec6d0d84e2e6a43d00097390764df9364c: out signal -> the signal

maflcko commented at 10:51 am on February 26, 2026:

nit in e9cab3ec6d0d84e2e6a43d00097390764df9364c: hanging waiting them -> waiting for them

furszy commented at 2:18 pm on February 26, 2026:

Fixed, thanks.

in src/test/threadpool_tests.cpp:407 in d502ced600 outdated

402+    BOOST_CHECK_EQUAL(threadPool.WorkQueueSize(), num_tasks);
403+
404+    // Delay release so Stop() drain all tasks from the calling thread
405+    std::thread unblocker([&blocker, &threadPool]() {
406+        while (threadPool.WorkQueueSize() > 0) {
407+            UninterruptibleSleep(300ms);

maflcko commented at 11:06 am on February 26, 2026:

nit in d502ced60004c93dead3a513cda2f8caf2308380: Could use std::this_thread::yield(); here as well?

furszy commented at 2:18 pm on February 26, 2026:

Done as suggested.

in src/test/txreconciliation_tests.cpp:11 in 1814fb911d outdated

 8+#include <test/util/common.h>
 9 
10 #include <boost/test/unit_test.hpp>
11 
12-BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup)
13+BOOST_AUTO_TEST_SUITE(txreconciliation_tests)

maflcko commented at 11:14 am on February 26, 2026:

in 1814fb911d5d633d2925795144e94e69234836e7: Seems a bit odd to hide those changes without any mention in a commit called “test: move and simplify BOOST_CHECK ostream helpers”.

This will break DEBUG_LOG_OUT for all of those tests, which makes failures harder to debug.

I think this should be reverted, or at a minimum put into a different commit, to explain that this is a breaking change.

furszy commented at 2:19 pm on February 26, 2026:

sure, we can talk about this in a follow-up. Reverted.

maflcko commented at 2:21 pm on February 26, 2026:

Well, I just left the comment on one line, but it applies to all those changes

hodlinator commented at 2:26 pm on February 26, 2026:

TIL about DEBUG_LOG_OUT.

Without BasicTestingSetup:

0₿ cmake --build build -t test_bitcoin && hyperfine "./build/bin/test_bitcoin -t txreconciliation_tests"
1...
2Benchmark 1: ./build/bin/test_bitcoin -t txreconciliation_tests
3  Time (mean ± σ):     111.0 ms ±   0.3 ms    [User: 106.8 ms, System: 3.8 ms]
4  Range (min … max):   110.5 ms … 111.9 ms    26 runs

With BasicTestingSetup:

0₿ cmake --build build -t test_bitcoin && hyperfine "./build/bin/test_bitcoin -t txreconciliation_tests"
1...
2Benchmark 1: ./build/bin/test_bitcoin -t txreconciliation_tests
3  Time (mean ± σ):     113.5 ms ±   0.3 ms    [User: 108.7 ms, System: 4.4 ms]
4  Range (min … max):   112.9 ms … 114.1 ms    26 runs

So only ~3% overhead.

furszy commented at 2:32 pm on February 26, 2026:

ups, sorry. Done as suggested. All of them gone for a follow-up.

furszy commented at 2:52 pm on February 26, 2026:

@hodlinator, the changes make more sense from a dependency cleanup perspective. setup_common.h brings in a lot of unneeded dependencies.

maflcko commented at 5:08 pm on February 26, 2026:

So only ~3% overhead.

If the overhead is an issue, maybe the logging to storage can be globally disabled for unit tests with a slim NoLogFileSetup, which is a base to BasicTestingSetup?

in src/test/threadpool_tests.cpp:90 in 44607f3856 outdated

87+    // Interrupted (workers still alive): Interrupted, and Start() must be rejected too
88+    std::counting_semaphore<> blocker(0);
89+    threadPool.Start(NUM_WORKERS_DEFAULT);
90+    const auto blocking_tasks = BlockWorkers(threadPool, blocker, NUM_WORKERS_DEFAULT);
91+    threadPool.Interrupt();
92+    res = threadPool.Submit([]{ return false; });

maflcko commented at 11:20 am on February 26, 2026:

nit in 44607f3856f2f3853ef53f1e0463c6c072d7ce02: I think this can be void?

0    res = threadPool.Submit([]{  });

maflcko commented at 11:20 am on February 26, 2026:

(same below)

furszy commented at 2:19 pm on February 26, 2026:

Done as suggested

maflcko commented at 11:22 am on February 26, 2026: member

lgtm, but I think the DEBUG_LOG_OUT regression should not be hidden, and probably reverted.

review ACK 566f7f81de967065afebc153ec41dc7b8d8300d0 🔭

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 566f7f81de967065afebc153ec41dc7b8d8300d0 🔭
3djAJqobfVijAJhOWxsBW8FcWoW6+27+15S5YpVNGfATR3dMyeVI0VKItWZUCTQjN3RbjDvRiymA04OyDpnk4Dg==

test: add threadpool Start-Stop race coverage

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

e88d274430

threadpool: active-wait during shutdown

Instead of waiting for the workers to finish processing tasks, help
them actively inside Stop().

This speeds up the JSON-RPC and REST server shutdown procedure,
and results in a faster node shutdown when many requests remain unhandled

bf2c607aaa

test: coverage for queued tasks completion after interrupt ca101a2315

test: ensure Stop() thread helps drain the queue

Exercise the case where tasks remain pending and verify that the
thread calling Stop() participates in draining the queue

3b7cbcafcb

furszy force-pushed on Feb 26, 2026

furszy commented at 2:23 pm on February 26, 2026: member

Updated per feedback, thanks @maflcko.

maflcko commented at 2:23 pm on February 26, 2026: member

#34562 (review)

775d6d6d8f8796eacf690875fe2a78f397aab320 🕹

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: 775d6d6d8f8796eacf690875fe2a78f397aab320 🕹
3QwvFHlDaFCAgy1I+7OiI8E8aDUrgH/gePWpp2F1sMNRNah9d/I4//SDl6l8pL1i8ZU9mFExzfDFxmA/obzihBA==

test: move and simplify BOOST_CHECK ostream helpers

Move the operator<< overloads used by BOOST_CHECK_* out of the
unit test machinery test/setup_common, into test/util/common.h.

And replace the individual per-type ToString() overloads with
a single concept-constrained template that covers any type
exposing a ToString() method. This is important to not add
uint256.h and transaction_identifier.h dependencies to the
shared test/util/common.h file.

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>

dbbb780af0

test: refactor, decouple HasReason from test framework machinery

Avoid providing the entire unit test framework dependency to tests that only
require access to the HasReason utility class.

E.g. reverselock_tests.cpp, sync_tests.cpp, util_check_tests.cpp, util_string_tests.cpp,
and script_parse_tests.cpp only require access to HasReason and nothing else.

d9c6769d03

test: cleanup, use HasReason in threadpool_tests.cpp ce2a984ee3

test: threadpool, add coverage for all Submit() errors

Submit tasks to a non-started, interrupted, or stopped
pool and verify the proper error is always returned.

9dc653b3b4

test: include response body in non-JSON HTTP error msg

Useful for debugging issues.

Co-authored-by: Matias Furszyfer <matiasfurszyfer@protonmail.com>

408d5b12e8

furszy force-pushed on Feb 26, 2026

DrahtBot added the label CI failed on Feb 26, 2026

hodlinator approved

hodlinator commented at 2:32 pm on February 26, 2026: contributor

re-ACK 408d5b12e80151feded31c2a5509e2dc5f15edf3

DrahtBot requested review from maflcko on Feb 26, 2026

DrahtBot requested review from achow101 on Feb 26, 2026

maflcko commented at 5:12 pm on February 26, 2026: member

review ACK 408d5b12e80151feded31c2a5509e2dc5f15edf3 🕗

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 408d5b12e80151feded31c2a5509e2dc5f15edf3 🕗
3JJta6Ss0/OBWFyZepOHB0Iyzl9klXuQr0EmD24LbgoVXIFsDBSWAgN/CYlsllIFYf4meXWND7PgQfKLKTR67AQ==

DrahtBot removed the label CI failed on Feb 26, 2026

achow101 commented at 8:23 pm on February 26, 2026: member

ACK 408d5b12e80151feded31c2a5509e2dc5f15edf3

achow101 merged this on Feb 26, 2026

achow101 closed this on Feb 26, 2026

furszy deleted the branch on Feb 26, 2026

ThreadPool follow-ups, proactive shutdown and HasReason dependency cleanup #34562

Reviews

Conflicts

LLM Linter (✨ experimental)