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

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 <code></code> 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.

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.

<details><summary>Possible fix - moving the shared state belonging to one session/Batch into it's own memory (not just the workers)</summary>

diff --git a/src/util/threadpool.h b/src/util/threadpool.h
index b75a94157e..9cafbd87d8 100644
--- a/src/util/threadpool.h
+++ b/src/util/threadpool.h
@@ -12,8 +12,8 @@
 
 #include <algorithm>
 #include <condition_variable>
-#include <functional>
 #include <future>
+#include <memory>
 #include <queue>
 #include <stdexcept>
 #include <thread>
@@ -46,39 +46,41 @@ class ThreadPool
 {
 private:
     std::string m_name;
-    Mutex m_mutex;
-    std::queue<std::packaged_task<void()>> m_work_queue GUARDED_BY(m_mutex);
-    std::condition_variable m_cv;
-    // Note: m_interrupt must be guarded by m_mutex, and cannot be replaced by an unguarded atomic bool.
-    // This ensures threads blocked on m_cv reliably observe the change and proceed correctly without missing signals.
-    // Ref: https://en.cppreference.com/w/cpp/thread/condition_variable
-    bool m_interrupt GUARDED_BY(m_mutex){false};
-    std::vector<std::thread> m_workers GUARDED_BY(m_mutex);
-
-    void WorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    struct Batch {
+        Mutex m_mutex;
+        std::queue<std::packaged_task<void()>> m_work_queue GUARDED_BY(m_mutex);
+        std::condition_variable m_cv;
+        // Note: m_interrupt must be guarded by m_mutex, and cannot be replaced by an unguarded atomic bool.
+        // This ensures threads blocked on m_cv reliably observe the change and proceed correctly without missing signals.
+        // Ref: https://en.cppreference.com/w/cpp/thread/condition_variable
+        bool m_interrupt GUARDED_BY(m_mutex){false};
+        std::vector<std::thread> m_workers GUARDED_BY(m_mutex);
+    };
+    Mutex m_batch_mutex;
+    std::unique_ptr<Batch> m_batch GUARDED_BY(m_batch_mutex);
+
+    // The function is executed within the context of a Batch which may no
+    // longer be stored in m_batch while stopping.
+    void WorkerThread(Batch& batch) EXCLUSIVE_LOCKS_REQUIRED(!batch.m_mutex)
     {
-        WAIT_LOCK(m_mutex, wait_lock);
+        WAIT_LOCK(batch.m_mutex, wait_lock);
         for (;;) {
-            std::packaged_task<void()> task;
-            {
-                // Wait only if needed; avoid sleeping when a new task was submitted while we were processing another one.
-                if (!m_interrupt && m_work_queue.empty()) {
-                    // Block until the pool is interrupted or a task is available.
-                    m_cv.wait(wait_lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(m_mutex) { return m_interrupt || !m_work_queue.empty(); });
-                }
-
-                // If stopped and no work left, exit worker
-                if (m_interrupt && m_work_queue.empty()) {
-                    return;
-                }
-
-                task = std::move(m_work_queue.front());
-                m_work_queue.pop();
+            // Wait only if needed; avoid sleeping when a new task was submitted while we were processing another one.
+            if (!batch.m_interrupt && batch.m_work_queue.empty()) {
+                // Block until the pool is interrupted or a task is available.
+                batch.m_cv.wait(wait_lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(batch.m_mutex) { return batch.m_interrupt || !batch.m_work_queue.empty(); });
+            }
+
+            // If stopped and no work left, exit worker
+            if (batch.m_interrupt && batch.m_work_queue.empty()) {
+                return;
             }
 
+            std::packaged_task<void()> task{std::move(batch.m_work_queue.front())};
+            batch.m_work_queue.pop();
             {
                 // Execute the task without the lock
-                REVERSE_LOCK(wait_lock, m_mutex);
+                REVERSE_LOCK(wait_lock, batch.m_mutex);
                 task();
             }
         }
@@ -100,17 +102,20 @@ public:
      *
      * Must be called from a controller (non-worker) thread.
      */
-    void Start(int num_workers) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    void Start(int num_workers) EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
         assert(num_workers > 0);
-        LOCK(m_mutex);
-        if (!m_workers.empty()) throw std::runtime_error("Thread pool already started");
-        m_interrupt = false; // Reset
+        LOCK(m_batch_mutex);
+        if (m_batch) throw std::runtime_error("Thread pool already started");
+
+        m_batch = std::make_unique<Batch>();
+        LOCK(m_batch->m_mutex);
+        m_batch->m_interrupt = false; // Reset
 
         // Create workers
-        m_workers.reserve(num_workers);
+        m_batch->m_workers.reserve(num_workers);
         for (int i = 0; i < num_workers; i++) {
-            m_workers.emplace_back(&util::TraceThread, strprintf("%s_pool_%d", m_name, i), [this] { WorkerThread(); });
+            m_batch->m_workers.emplace_back(&util::TraceThread, strprintf("%s_pool_%d", m_name, i), [this, batch = m_batch.get()] { this->WorkerThread(*batch); });
         }
     }
 
@@ -122,24 +127,30 @@ public:
      *
      * Must be called from a controller (non-worker) thread.
      */
-    void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
         // Notify workers and join them
-        std::vector<std::thread> threads_to_join;
+        std::unique_ptr<Batch> local_batch;
         {
-            LOCK(m_mutex);
+            LOCK(m_batch_mutex);
+            if (!m_batch) return;
+
+            LOCK(m_batch->m_mutex);
             // Ensure Stop() is not called from a worker thread while workers are still registered,
             // otherwise a self-join deadlock would occur.
-            auto id = std::this_thread::get_id();
-            for (const auto& worker : m_workers) assert(worker.get_id() != id);
+            const auto id = std::this_thread::get_id();
+            for (const auto& worker : m_batch->m_workers) assert(worker.get_id() != id);
             // Early shutdown to return right away on any concurrent Submit() call
-            m_interrupt = true;
-            threads_to_join.swap(m_workers);
+            m_batch->m_interrupt = true;
+            local_batch.swap(m_batch);
         }
-        m_cv.notify_all();
+        local_batch->m_cv.notify_all();
+        std::vector<std::thread> threads_to_join;
+        WITH_LOCK(local_batch->m_mutex, threads_to_join.swap(local_batch->m_workers));
         for (auto& worker : threads_to_join) worker.join();
         // Since we currently wait for tasks completion, sanity-check empty queue
-        WITH_LOCK(m_mutex, Assume(m_work_queue.empty()));
+        //WITH_LOCK(m_mutex, Assume(m_work_queue.empty())); // Racy! Another thread might have Start() + Submit() after the last join!
+        WITH_LOCK(local_batch->m_mutex, Assume(local_batch->m_work_queue.empty()));
         // Note: m_interrupt is left true until next Start()
     }
 
@@ -151,19 +162,22 @@ public:
      * Note: Ignoring the returned future requires guarding the task against
      * uncaught exceptions, as they would otherwise be silently discarded.
      */
-    template <class F> [[nodiscard]] EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
-    auto Submit(F&& fn)
+    template <class F>
+    [[nodiscard]] auto Submit(F&& fn) EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
         std::packaged_task task{std::forward<F>(fn)};
         auto future{task.get_future()};
+        LOCK(m_batch_mutex);
+        if (!m_batch) throw std::runtime_error("No active workers; cannot accept new tasks");
         {
-            LOCK(m_mutex);
-            if (m_interrupt || m_workers.empty()) {
+            LOCK(m_batch->m_mutex);
+            assert(!m_batch->m_workers.empty());
+            if (m_batch->m_interrupt) {
                 throw std::runtime_error("No active workers; cannot accept new tasks");
             }
-            m_work_queue.emplace(std::move(task));
+            m_batch->m_work_queue.emplace(std::move(task));
         }
-        m_cv.notify_one();
+        m_batch->m_cv.notify_one();
         return future;
     }
 
@@ -171,16 +185,18 @@ public:
      * [@brief](/bitcoin-bitcoin/contributor/brief/) Execute a single queued task synchronously.
      * Removes one task from the queue and executes it on the calling thread.
      */
-    void ProcessTask() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    void ProcessTask() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
         std::packaged_task<void()> task;
         {
-            LOCK(m_mutex);
-            if (m_work_queue.empty()) return;
+            LOCK(m_batch_mutex);
+            if (!m_batch) return;
+            LOCK(m_batch->m_mutex);
+            if (m_batch->m_work_queue.empty()) return;
 
             // Pop the task
-            task = std::move(m_work_queue.front());
-            m_work_queue.pop();
+            task = std::move(m_batch->m_work_queue.front());
+            m_batch->m_work_queue.pop();
         }
         task();
     }
@@ -191,20 +207,31 @@ public:
      * Wakes all worker threads so they can drain the queue and exit.
      * Unlike Stop(), this function does not wait for threads to finish.
      */
-    void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
-        WITH_LOCK(m_mutex, m_interrupt = true);
-        m_cv.notify_all();
+        LOCK(m_batch_mutex);
+        if (!m_batch) return;
+        {
+            LOCK(m_batch->m_mutex);
+            m_batch->m_interrupt = true;
+        }
+        m_batch->m_cv.notify_all();
     }
 
-    size_t WorkQueueSize() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    size_t WorkQueueSize() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
-        return WITH_LOCK(m_mutex, return m_work_queue.size());
+        LOCK(m_batch_mutex);
+        if (!m_batch) return 0;
+        LOCK(m_batch->m_mutex);
+        return m_batch->m_work_queue.size();
     }
 
-    size_t WorkersCount() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    size_t WorkersCount() EXCLUSIVE_LOCKS_REQUIRED(!m_batch_mutex, !m_batch->m_mutex)
     {
-        return WITH_LOCK(m_mutex, return m_workers.size());
+        LOCK(m_batch_mutex);
+        if (!m_batch) return 0;
+        LOCK(m_batch->m_mutex);
+        return m_batch->m_workers.size();
     }
 };

</details>

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

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

diff --git a/src/test/threadpool_tests.cpp b/src/test/threadpool_tests.cpp
index 46acf1d67d..41882345a2 100644
--- a/src/test/threadpool_tests.cpp
+++ b/src/test/threadpool_tests.cpp
@@ -56,8 +56,7 @@ std::vector<std::future<void>> BlockWorkers(ThreadPool& threadPool, const std::s
 
     // Fill all workers with blocking tasks
     std::vector<std::future<void>> blocking_tasks;
-    for (int i = 0; i < num_of_threads_to_block; i++) {
-        std::promise<void>& ready = ready_promises[i];
+    for (auto& ready : ready_promises) {
         blocking_tasks.emplace_back(threadPool.Submit([blocker_future, &ready]() {
             ready.set_value();
             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:

-reverlock_tests.cpp
+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.

<details><summary>threadpool.h-diff which passes tests</summary>

--- a/src/util/threadpool.h
+++ b/src/util/threadpool.h
@@ -54,8 +54,6 @@ private:
     // This ensures threads blocked on m_cv reliably observe the change and proceed correctly without missing signals.
     // Ref: https://en.cppreference.com/w/cpp/thread/condition_variable
     bool m_interrupt GUARDED_BY(m_mutex){false};
-    // Toggled throughout Stop() to prevent Start() from being called while workers are being joined.
-    bool m_stopping GUARDED_BY(m_mutex){false};
     std::vector<std::thread> m_workers GUARDED_BY(m_mutex);
 
     void WorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
@@ -107,9 +105,8 @@ public:
     {
         assert(num_workers > 0);
         LOCK(m_mutex);
-        if (m_stopping) throw std::runtime_error("Thread pool is stopping");
+        if (m_interrupt) throw std::runtime_error("Thread pool has been interrupted or is stopping");
         if (!m_workers.empty()) throw std::runtime_error("Thread pool already started");
-        m_interrupt = false; // Reset
 
         // Create workers
         m_workers.reserve(num_workers);
@@ -137,7 +134,6 @@ public:
             // otherwise a self-join deadlock would occur.
             auto id = std::this_thread::get_id();
             for (const auto& worker : m_workers) assert(worker.get_id() != id);
-            m_stopping = true;
             // Early shutdown to return right away on any concurrent Submit() call
             m_interrupt = true;
             threads_to_join.swap(m_workers);
@@ -152,8 +148,7 @@ public:
         LOCK(m_mutex);
         Assume(m_work_queue.empty());
         // Re-allow Start() now that all workers have exited
-        m_stopping = false;
-        // Note: m_interrupt is left true until next Start()
+        m_interrupt = false;
     }
 
     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?

// Test 11, Start() must not cause a deadlock when called during Stop()
BOOST_AUTO_TEST_CASE(start_mid_stop_does_not_deadlock)
{
    ThreadPool pool(POOL_NAME);
    pool.Start(NUM_WORKERS_DEFAULT);

    std::latch stop_called{1};
    std::latch second_start_called{1};
    auto future = Submit(pool, [&stop_called, &second_start_called]{
        stop_called.wait();
        second_start_called.wait();
    });

    std::latch stopper_running{1};
    std::thread stopper([&stopper_running, &pool] {
        stopper_running.count_down();
        pool.Stop();
    });
    stopper_running.wait();
    // Wait a bit in case the stopper thread was context switched before the Stop()-call.
    UninterruptibleSleep(500ms);
    stop_called.count_down();

    try {
        pool.Start(NUM_WORKERS_DEFAULT);
    } catch (const std::runtime_error&) {
        // Expected: Start() was correctly rejected while Stop() was in progress
    }
    second_start_called.count_down();
    // Without the fix, calling Start() right above would set
    // ThreadPool::m_interrupt = false, causing the worker threads from the
    // initial Start() to never complete the join at the end of Stop(), causing
    // the test to hang here:
    stopper.join();

    pool.Stop();
}

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:

// Test 11, Start() must not cause a deadlock when called during Stop()
BOOST_AUTO_TEST_CASE(start_mid_stop_does_not_deadlock)
{
    ThreadPool threadPool(POOL_NAME);
    threadPool.Start(NUM_WORKERS_DEFAULT);

    // Keep all workers busy so Stop() gets stuck waiting for them to finish during join()
    std::counting_semaphore<> workers_blocker(0);
    const auto blocking_tasks = BlockWorkers(threadPool, workers_blocker, NUM_WORKERS_DEFAULT);

    std::thread stopper_thread([&threadPool] { threadPool.Stop(); });

    // Stop() takes ownership of the workers before joining them, so WorkersCount()
    // hits 0 the moment Stop() is hanging waiting them to join. That is out signal
    // to call Start() right into the middle of the joining phase.
    while (threadPool.WorkersCount() != 0) {
        UninterruptibleSleep(200ms); // let the OS breathe so it can switch context
    }
    // Now we know for sure the stopper thread is hanging while workers are still alive.
    // Restart the pool and resume workers so the stopper thread can proceed.
    // This will throw an exception only if the pool handles Start-Stop race properly,
    // otherwise it will proceed and hang the stopper_thread.
    try {
        threadPool.Start(NUM_WORKERS_DEFAULT);
    } catch (std::exception& e) {
        BOOST_CHECK_EQUAL(e.what(), "Thread pool has been interrupted or is stopping");
    }
    workers_blocker.release(NUM_WORKERS_DEFAULT);
    WAIT_FOR(blocking_tasks);

    // If Stop() is stuck, joining the stopper thread will deadlock
    stopper_thread.join();
}

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:

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

Recommendation of using yield():

₿ hyperfine "./build/bin/test_bitcoin -t threadpool_tests/start_mid_stop_does_not_deadlock"
Benchmark 1: ./build/bin/test_bitcoin -t threadpool_tests/start_mid_stop_does_not_deadlock
  Time (mean ± σ):     111.0 ms ±   0.3 ms    [User: 107.3 ms, System: 4.1 ms]
  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:

BOOST_AUTO_TEST_CASE(stop_active_wait_drains_queue)
{
    ThreadPool threadPool(POOL_NAME);
    threadPool.Start(NUM_WORKERS_DEFAULT);

    std::counting_semaphore<> blocker(0);
    const auto blocking_tasks = BlockWorkers(threadPool, blocker, NUM_WORKERS_DEFAULT);

    auto main_thread_id = std::this_thread::get_id();
    std::atomic<int> main_thread_tasks{0};
    const int num_tasks = 20;
    for (int i = 0; i < num_tasks; i++) {
        (void)Submit(threadPool, [&main_thread_tasks, main_thread_id]() {
            if (std::this_thread::get_id() == main_thread_id)
                main_thread_tasks.fetch_add(1, std::memory_order_relaxed);
        });
    }
    BOOST_CHECK_EQUAL(threadPool.WorkQueueSize(), static_cast<size_t>(num_tasks));

    // Delay release so Stop() has a window to drain tasks from the calling thread
    std::thread unblocker([&blocker]() {
        UninterruptibleSleep(300ms);
        blocker.release(NUM_WORKERS_DEFAULT);
    });

    threadPool.Stop();
    unblocker.join();

    BOOST_CHECK_GT(main_thread_tasks.load(), 0);
    WAIT_FOR(blocking_tasks);
}

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 12: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.

        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.

<details><summary>Full patch on top of that commit</summary>

diff --git a/src/test/arith_uint256_tests.cpp b/src/test/arith_uint256_tests.cpp
index 0a15458041..016b465871 100644
--- a/src/test/arith_uint256_tests.cpp
+++ b/src/test/arith_uint256_tests.cpp
@@ -3,7 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <arith_uint256.h>
-#include <test/util/setup_common.h>
+#include <test/util/common.h>
 #include <uint256.h>
 
 #include <boost/test/unit_test.hpp>
diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 0fbe0f3c13..e4200cace2 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -10,6 +10,7 @@
 #include <test/util/random.h>
 #include <test/util/txmempool.h>
 
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp
index d7d10dfb1a..25762e070d 100644
--- a/src/test/blockfilter_index_tests.cpp
+++ b/src/test/blockfilter_index_tests.cpp
@@ -12,6 +12,7 @@
 #include <node/miner.h>
 #include <pow.h>
 #include <test/util/blockfilter.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp
index c8334dabe1..e89909dfaa 100644
--- a/src/test/blockfilter_tests.cpp
+++ b/src/test/blockfilter_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <test/data/blockfilters.json.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 
 #include <blockfilter.h>
diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
index db59931ba7..28bb7b70a7 100644
--- a/src/test/blockmanager_tests.cpp
+++ b/src/test/blockmanager_tests.cpp
@@ -14,6 +14,7 @@
 #include <validation.h>
 
 #include <boost/test/unit_test.hpp>
+#include <test/util/common.h>
 #include <test/util/logging.h>
 #include <test/util/setup_common.h>
 
diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp
index 8e02cfd08c..ed333c34c0 100644
--- a/src/test/bloom_tests.cpp
+++ b/src/test/bloom_tests.cpp
@@ -13,6 +13,7 @@
 #include <random.h>
 #include <serialize.h>
 #include <streams.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <uint256.h>
diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp
index 5588d4cdbc..b348793bfb 100644
--- a/src/test/crypto_tests.cpp
+++ b/src/test/crypto_tests.cpp
@@ -17,6 +17,7 @@
 #include <crypto/muhash.h>
 #include <random.h>
 #include <streams.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp
index d3a9e54348..3896ea64da 100644
--- a/src/test/dbwrapper_tests.cpp
+++ b/src/test/dbwrapper_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <dbwrapper.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <uint256.h>
diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
index ec17fe3997..d349ceea44 100644
--- a/src/test/getarg_tests.cpp
+++ b/src/test/getarg_tests.cpp
@@ -5,6 +5,7 @@
 #include <common/args.h>
 #include <common/settings.h>
 #include <logging.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <univalue.h>
 #include <util/strencodings.h>
diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp
index f9426fa311..bba612f8b4 100644
--- a/src/test/headers_sync_chainwork_tests.cpp
+++ b/src/test/headers_sync_chainwork_tests.cpp
@@ -8,6 +8,7 @@
 #include <headerssync.h>
 #include <net_processing.h>
 #include <pow.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp
index da0f5eecce..1a98256ce2 100644
--- a/src/test/interfaces_tests.cpp
+++ b/src/test/interfaces_tests.cpp
@@ -5,6 +5,7 @@
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <interfaces/chain.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <script/solver.h>
 #include <validation.h>
diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp
index acced716f0..b0c052af0f 100644
--- a/src/test/key_tests.cpp
+++ b/src/test/key_tests.cpp
@@ -9,6 +9,7 @@
 #include <span.h>
 #include <streams.h>
 #include <secp256k1_extrakeys.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <uint256.h>
diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
index 8ead946f7f..f232bb4181 100644
--- a/src/test/logging_tests.cpp
+++ b/src/test/logging_tests.cpp
@@ -6,6 +6,7 @@
 #include <logging.h>
 #include <logging/timer.h>
 #include <scheduler.h>
+#include <test/util/common.h>
 #include <test/util/logging.h>
 #include <test/util/setup_common.h>
 #include <tinyformat.h>
diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp
index dc4791419e..a7212147b6 100644
--- a/src/test/merkle_tests.cpp
+++ b/src/test/merkle_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/merkle.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 
diff --git a/src/test/merkleblock_tests.cpp b/src/test/merkleblock_tests.cpp
index 00a1f0b92d..a461e04300 100644
--- a/src/test/merkleblock_tests.cpp
+++ b/src/test/merkleblock_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <merkleblock.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <uint256.h>
 
diff --git a/src/test/minisketch_tests.cpp b/src/test/minisketch_tests.cpp
index 1d6f2a12ef..ba895478de 100644
--- a/src/test/minisketch_tests.cpp
+++ b/src/test/minisketch_tests.cpp
@@ -5,6 +5,7 @@
 #include <minisketch.h>
 #include <node/minisketchwrapper.h>
 #include <random.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 
diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp
index 932b83fee4..0b1cc98086 100644
--- a/src/test/multisig_tests.cpp
+++ b/src/test/multisig_tests.cpp
@@ -9,6 +9,7 @@
 #include <script/script_error.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <test/util/transaction_utils.h>
 #include <tinyformat.h>
diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
index 4770cabf7a..167b88f5f3 100644
--- a/src/test/netbase_tests.cpp
+++ b/src/test/netbase_tests.cpp
@@ -10,7 +10,7 @@
 #include <protocol.h>
 #include <serialize.h>
 #include <streams.h>
-#include <test/util/setup_common.h>
+#include <test/util/common.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
 
@@ -22,7 +22,7 @@
 using namespace std::literals;
 using namespace util::hex_literals;
 
-BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)
+BOOST_AUTO_TEST_SUITE(netbase_tests)
 
 static CNetAddr ResolveIP(const std::string& ip)
 {
diff --git a/src/test/node_init_tests.cpp b/src/test/node_init_tests.cpp
index a4292e66ca..44635dd4aa 100644
--- a/src/test/node_init_tests.cpp
+++ b/src/test/node_init_tests.cpp
@@ -7,6 +7,7 @@
 #include <rpc/server.h>
 
 #include <boost/test/unit_test.hpp>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 
 using node::NodeContext;
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 933736dcc1..6ebfab34c5 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -10,6 +10,7 @@
 #include <pubkey.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <test/util/transaction_utils.h>
diff --git a/src/test/pcp_tests.cpp b/src/test/pcp_tests.cpp
index c71c9de160..51b004a5d5 100644
--- a/src/test/pcp_tests.cpp
+++ b/src/test/pcp_tests.cpp
@@ -5,6 +5,7 @@
 #include <common/pcp.h>
 #include <netbase.h>
 #include <test/util/logging.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <util/time.h>
 
diff --git a/src/test/pow_tests.cpp b/src/test/pow_tests.cpp
index 8de76ff21f..f6123401ad 100644
--- a/src/test/pow_tests.cpp
+++ b/src/test/pow_tests.cpp
@@ -6,6 +6,7 @@
 #include <chainparams.h>
 #include <pow.h>
 #include <test/util/random.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <util/chaintype.h>
 
diff --git a/src/test/rest_tests.cpp b/src/test/rest_tests.cpp
index 5dc122507f..fefc41bd73 100644
--- a/src/test/rest_tests.cpp
+++ b/src/test/rest_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <rest.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
index f8d75a8022..b435fb8992 100644
--- a/src/test/rpc_tests.cpp
+++ b/src/test/rpc_tests.cpp
@@ -9,6 +9,7 @@
 #include <rpc/client.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <univalue.h>
 #include <util/time.h>
diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
index c6d3b125eb..63215895ff 100644
--- a/src/test/script_standard_tests.cpp
+++ b/src/test/script_standard_tests.cpp
@@ -10,16 +10,24 @@
 #include <script/script.h>
 #include <script/signingprovider.h>
 #include <script/solver.h>
-#include <test/util/setup_common.h>
+#include <test/util/common.h>
 #include <util/strencodings.h>
 
 #include <boost/test/unit_test.hpp>
+#include <boost/test/unit_test_suite.hpp>
 
 #include <univalue.h>
 
 using namespace util::hex_literals;
 
-BOOST_FIXTURE_TEST_SUITE(script_standard_tests, BasicTestingSetup)
+namespace {
+struct ScriptStandardTest {
+    ECC_Context ecc;
+    ScriptStandardTest() { SelectParams(ChainType::MAIN); }
+};
+} // namespace
+
+BOOST_FIXTURE_TEST_SUITE(script_standard_tests, ScriptStandardTest)
 
 BOOST_AUTO_TEST_CASE(dest_default_is_no_dest)
 {
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 2134bcc179..1478cf9125 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -19,6 +19,7 @@
 #include <streams.h>
 #include <test/util/json.h>
 #include <test/util/random.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <test/util/transaction_utils.h>
 #include <util/fs.h>
@@ -26,7 +27,6 @@
 #include <util/string.h>
 
 #include <cstdint>
-#include <fstream>
 #include <string>
 #include <vector>
 
diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp
index 33a9133d2e..f646ba5faf 100644
--- a/src/test/serialize_tests.cpp
+++ b/src/test/serialize_tests.cpp
@@ -5,6 +5,7 @@
 #include <hash.h>
 #include <serialize.h>
 #include <streams.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <util/strencodings.h>
 
diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp
index 5d65c35a8c..5afdea43d8 100644
--- a/src/test/sighash_tests.cpp
+++ b/src/test/sighash_tests.cpp
@@ -11,6 +11,7 @@
 #include <serialize.h>
 #include <streams.h>
 #include <test/data/sighash.json.h>
+#include <test/util/common.h>
 #include <test/util/json.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
diff --git a/src/test/txdownload_tests.cpp b/src/test/txdownload_tests.cpp
index e9c6160ebf..d7ddef905b 100644
--- a/src/test/txdownload_tests.cpp
+++ b/src/test/txdownload_tests.cpp
@@ -8,6 +8,7 @@
 #include <node/txdownloadman_impl.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
index 24760664c1..06c5816b72 100644
--- a/src/test/txpackage_tests.cpp
+++ b/src/test/txpackage_tests.cpp
@@ -11,6 +11,7 @@
 #include <script/script.h>
 #include <serialize.h>
 #include <streams.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/script.h>
 #include <test/util/setup_common.h>
diff --git a/src/test/txreconciliation_tests.cpp b/src/test/txreconciliation_tests.cpp
index d4ed3511f4..8d1a47b6e3 100644
--- a/src/test/txreconciliation_tests.cpp
+++ b/src/test/txreconciliation_tests.cpp
@@ -4,11 +4,11 @@
 
 #include <node/txreconciliation.h>
 
-#include <test/util/setup_common.h>
+#include <test/util/common.h>
 
 #include <boost/test/unit_test.hpp>
 
-BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup)
+BOOST_AUTO_TEST_SUITE(txreconciliation_tests)
 
 BOOST_AUTO_TEST_CASE(RegisterPeerTest)
 {
diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp
index b16b60474a..2a4f213c4b 100644
--- a/src/test/txvalidation_tests.cpp
+++ b/src/test/txvalidation_tests.cpp
@@ -11,6 +11,7 @@
 #include <primitives/transaction.h>
 #include <random.h>
 #include <script/script.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <test/util/txmempool.h>
 #include <validation.h>
diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp
index a4ebc049fb..0de391d8cc 100644
--- a/src/test/uint256_tests.cpp
+++ b/src/test/uint256_tests.cpp
@@ -4,7 +4,7 @@
 
 #include <primitives/transaction_identifier.h>
 #include <streams.h>
-#include <test/util/setup_common.h>
+#include <test/util/common.h>
 #include <uint256.h>
 #include <util/strencodings.h>
 
diff --git a/src/test/util/common.h b/src/test/util/common.h
index 788df0d3fe..9d5b70d9c8 100644
--- a/src/test/util/common.h
+++ b/src/test/util/common.h
@@ -5,6 +5,8 @@
 #ifndef BITCOIN_TEST_UTIL_COMMON_H
 #define BITCOIN_TEST_UTIL_COMMON_H
 
+#include <primitives/transaction_identifier.h>
+#include <arith_uint256.h>
 #include <string>
 
 /**
@@ -23,4 +25,47 @@ private:
     const std::string m_reason;
 };
 
+// Make types usable in BOOST_CHECK_* @{
+namespace std {
+template <typename T> requires std::is_enum_v<T>
+inline std::ostream& operator<<(std::ostream& os, const T& e)
+{
+    return os << static_cast<std::underlying_type_t<T>>(e);
+}
+
+template <typename T>
+inline std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
+{
+    return v ? os << *v
+             : os << "std::nullopt";
+}
+} // namespace std
+
+inline std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
+{
+    return os << num.ToString();
+}
+
+inline std::ostream& operator<<(std::ostream& os, const uint160& num)
+{
+    return os << num.ToString();
+}
+
+inline std::ostream& operator<<(std::ostream& os, const uint256& num)
+{
+    return os << num.ToString();
+}
+
+inline std::ostream& operator<<(std::ostream& os, const Txid& txid)
+{
+    return os << txid.ToString();
+}
+
+inline std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid)
+{
+    return os << wtxid.ToString();
+}
+
+// @}
+
 #endif // BITCOIN_TEST_UTIL_COMMON_H
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 995f532fd7..6fdc9e1882 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -620,26 +620,3 @@ CBlock getBlock13b8a()
     stream >> TX_WITH_WITNESS(block);
     return block;
 }
-
-std::ostream& operator<<(std::ostream& os, const arith_uint256& num)
-{
-    return os << num.ToString();
-}
-
-std::ostream& operator<<(std::ostream& os, const uint160& num)
-{
-    return os << num.ToString();
-}
-
-std::ostream& operator<<(std::ostream& os, const uint256& num)
-{
-    return os << num.ToString();
-}
-
-std::ostream& operator<<(std::ostream& os, const Txid& txid) {
-    return os << txid.ToString();
-}
-
-std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid) {
-    return os << wtxid.ToString();
-}
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index 1f71b21a25..c02a4fd331 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -261,27 +261,4 @@ std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::
 
 CBlock getBlock13b8a();
 
-// Make types usable in BOOST_CHECK_* @{
-namespace std {
-template <typename T> requires std::is_enum_v<T>
-inline std::ostream& operator<<(std::ostream& os, const T& e)
-{
-    return os << static_cast<std::underlying_type_t<T>>(e);
-}
-
-template <typename T>
-inline std::ostream& operator<<(std::ostream& os, const std::optional<T>& v)
-{
-    return v ? os << *v
-             : os << "std::nullopt";
-}
-} // namespace std
-
-std::ostream& operator<<(std::ostream& os, const arith_uint256& num);
-std::ostream& operator<<(std::ostream& os, const uint160& num);
-std::ostream& operator<<(std::ostream& os, const uint256& num);
-std::ostream& operator<<(std::ostream& os, const Txid& txid);
-std::ostream& operator<<(std::ostream& os, const Wtxid& wtxid);
-// @}
-
 #endif // BITCOIN_TEST_UTIL_SETUP_COMMON_H
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index 69a6c9c110..7757695085 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -216,15 +216,15 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
 
         LineReader reader1(input, /*max_line_length=*/22);
         // First line is exactly the length of max_line_length
-        BOOST_CHECK_EQUAL(*reader1.ReadLine(), "once upon a time there");
+        BOOST_CHECK_EQUAL(reader1.ReadLine(), "once upon a time there");
         // Second line is +1 character too long
         BOOST_CHECK_EXCEPTION(reader1.ReadLine(), std::runtime_error, HasReason{"max_line_length exceeded by LineReader"});
 
         // Increase max_line_length by 1
         LineReader reader2(input, /*max_line_length=*/23);
         // Both lines fit within limit
-        BOOST_CHECK_EQUAL(*reader2.ReadLine(), "once upon a time there");
-        BOOST_CHECK_EQUAL(*reader2.ReadLine(), "was a dog who liked tea");
+        BOOST_CHECK_EQUAL(reader2.ReadLine(), "once upon a time there");
+        BOOST_CHECK_EQUAL(reader2.ReadLine(), "was a dog who liked tea");
         // End of buffer reached
         BOOST_CHECK(!reader2.ReadLine());
     }
@@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
         // Empty lines are empty
         const std::vector<std::byte> input{StringToBuffer("\n")};
         LineReader reader(input, /*max_line_length=*/1024);
-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "");
+        BOOST_CHECK_EQUAL(reader.ReadLine(), "");
         BOOST_CHECK(!reader.ReadLine());
     }
     {
@@ -251,15 +251,15 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
     {
         const std::vector<std::byte> input{StringToBuffer("a\nb\n")};
         LineReader reader(input, /*max_line_length=*/1);
-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "a");
-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "b");
+        BOOST_CHECK_EQUAL(reader.ReadLine(), "a");
+        BOOST_CHECK_EQUAL(reader.ReadLine(), "b");
         BOOST_CHECK(!reader.ReadLine());
     }
     {
         // If ReadLine fails, the iterator is reset and we can ReadLength instead
         const std::vector<std::byte> input{StringToBuffer("a\nbaboon\n")};
         LineReader reader(input, /*max_line_length=*/1);
-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "a");
+        BOOST_CHECK_EQUAL(reader.ReadLine(), "a");
         // "baboon" is too long
         BOOST_CHECK_EXCEPTION(reader.ReadLine(), std::runtime_error, HasReason{"max_line_length exceeded by LineReader"});
         BOOST_CHECK_EQUAL(reader.ReadLength(1), "b");
@@ -268,7 +268,7 @@ BOOST_AUTO_TEST_CASE(line_reader_test)
         // "on" is too long
         BOOST_CHECK_EXCEPTION(reader.ReadLine(), std::runtime_error, HasReason{"max_line_length exceeded by LineReader"});
         BOOST_CHECK_EQUAL(reader.ReadLength(1), "o");
-        BOOST_CHECK_EQUAL(*reader.ReadLine(), "n"); // now the remainder of the buffer fits in one line
+        BOOST_CHECK_EQUAL(reader.ReadLine(), "n"); // now the remainder of the buffer fits in one line
         BOOST_CHECK(!reader.ReadLine());
     }
     {
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index f91b30a307..083a32da96 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -10,6 +10,7 @@
 #include <node/miner.h>
 #include <pow.h>
 #include <random.h>
+#include <test/util/common.h>
 #include <test/util/random.h>
 #include <test/util/script.h>
 #include <test/util/setup_common.h>
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index 9e2c710977..df5e522edd 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -9,6 +9,7 @@
 #include <rpc/blockchain.h>
 #include <sync.h>
 #include <test/util/chainstate.h>
+#include <test/util/common.h>
 #include <test/util/coins.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 40f99690ce..06de793d25 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -12,6 +12,7 @@
 #include <rpc/blockchain.h>
 #include <sync.h>
 #include <test/util/chainstate.h>
+#include <test/util/common.h>
 #include <test/util/logging.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
index 66c284b979..c35cef7f45 100644
--- a/src/test/validation_flush_tests.cpp
+++ b/src/test/validation_flush_tests.cpp
@@ -5,6 +5,7 @@
 #include <sync.h>
 #include <test/util/coins.h>
 #include <test/util/random.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp
index ea1044023e..ad79205311 100644
--- a/src/test/versionbits_tests.cpp
+++ b/src/test/versionbits_tests.cpp
@@ -6,6 +6,7 @@
 #include <chainparams.h>
 #include <consensus/params.h>
 #include <test/util/random.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <util/chaintype.h>
 #include <versionbits.h>
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 7f55d76585..92a35b08af 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -7,6 +7,7 @@
 #include <policy/policy.h>
 #include <primitives/transaction.h>
 #include <random.h>
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <util/translation.h>
 #include <wallet/coincontrol.h>
diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp
index 8b71634ed8..532bdce384 100644
--- a/src/wallet/test/db_tests.cpp
+++ b/src/wallet/test/db_tests.cpp
@@ -4,6 +4,7 @@
 
 #include <boost/test/unit_test.hpp>
 
+#include <test/util/common.h>
 #include <test/util/setup_common.h>
 #include <util/check.h>
 #include <util/fs.h>
@@ -14,7 +15,6 @@
 #include <wallet/walletutil.h>
 
 #include <cstddef>
-#include <fstream>
 #include <memory>
 #include <span>
 #include <string>
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 36782cd187..54f453623e 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -17,6 +17,7 @@
 #include <policy/policy.h>
 #include <rpc/server.h>
 #include <script/solver.h>
+#include <test/util/common.h>
 #include <test/util/logging.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
diff --git a/src/wallet/test/wallet_transaction_tests.cpp b/src/wallet/test/wallet_transaction_tests.cpp
index e49439065a..38be52f41f 100644
--- a/src/wallet/test/wallet_transaction_tests.cpp
+++ b/src/wallet/test/wallet_transaction_tests.cpp
@@ -4,6 +4,7 @@
 
 #include <wallet/transaction.h>
 
+#include <test/util/common.h>
 #include <wallet/test/wallet_test_fixture.h>
 
 #include <boost/test/unit_test.hpp>
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index bc994ba606..1463ee98b3 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -4,6 +4,7 @@
 
 #include <wallet/test/util.h>
 #include <wallet/wallet.h>
+#include <test/util/common.h>
 #include <test/util/logging.h>
 #include <test/util/setup_common.h>

</details>

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:

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

template <typename T> requires HasToString<T>
inline std::ostream& operator<<(std::ostream& os, const T& obj)
{
    return os << obj.ToString();
}

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?

template <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:

    const size_t num_tasks = 20;
    for (size_t i = 0; i < num_tasks; i++) {
        (void)Submit(threadPool, [&main_thread_tasks, main_thread_id]() {
            if (std::this_thread::get_id() == main_thread_id)
                main_thread_tasks.fetch_add(1, std::memory_order_relaxed);
        });
    }
    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:

template <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:

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

With BasicTestingSetup:

₿ cmake --build build -t test_bitcoin && hyperfine "./build/bin/test_bitcoin -t txreconciliation_tests"
...
Benchmark 1: ./build/bin/test_bitcoin -t txreconciliation_tests
  Time (mean ± σ):     113.5 ms ±   0.3 ms    [User: 108.7 ms, System: 4.4 ms]
  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?

hodlinator commented at 2:29 PM on March 11, 2026:

Having NoLogFileSetup as a base to BasicTestingSetup and then in some test wanting to use equivalent functionality of BasicTestingSetup but with logging to a file enabled would be messy.

Seems more straightforward to:

--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -138,6 +138,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
             "-debug",
             "-debugexclude=libevent",
             "-debugexclude=leveldb",
+            "-nodebuglogfile",
         },
         opts.extra_args);
     if (G_TEST_COMMAND_LINE_ARGUMENTS) {

Change MakeNoLogFileContext -> MakeNoDebugLogContext
Have tests opt-in to "-debuglogfile" via TestOpts.extra_args or some new way to toggle m_print_to_file on the fly.

maflcko commented at 2:50 PM on March 11, 2026:

Have tests opt-in to "-debuglogfile" via TestOpts.extra_args or some new way to toggle m_print_to_file on the fly.

Yeah, sounds good. Is there even a reason to have tests set -debuglogfile at all?

hodlinator commented at 9:07 PM on March 25, 2026:

Closing the loop for this thread: #34922 is now up for review.

Is there even a reason to have tests set -debuglogfile at all?

I decided to default to logging to file if the test creates a datadir for now in the PR.

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?

    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 🔭

<details><summary>Show signature</summary>

Signature:

untrusted 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}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 566f7f81de967065afebc153ec41dc7b8d8300d0 🔭
djAJqobfVijAJhOWxsBW8FcWoW6+27+15S5YpVNGfATR3dMyeVI0VKItWZUCTQjN3RbjDvRiymA04OyDpnk4Dg==

</details>

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 🕹

<details><summary>Show signature</summary>

Signature:

untrusted 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}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: 775d6d6d8f8796eacf690875fe2a78f397aab320 🕹
QwvFHlDaFCAgy1I+7OiI8E8aDUrgH/gePWpp2F1sMNRNah9d/I4//SDl6l8pL1i8ZU9mFExzfDFxmA/obzihBA==

</details>

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 🕗

<details><summary>Show signature</summary>

Signature:

untrusted 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}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 408d5b12e80151feded31c2a5509e2dc5f15edf3 🕗
JJta6Ss0/OBWFyZepOHB0Iyzl9klXuQr0EmD24LbgoVXIFsDBSWAgN/CYlsllIFYf4meXWND7PgQfKLKTR67AQ==

</details>

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