From f9758d7b183d589a071e608b1cd77e2220fd9cef Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Mon, 27 Nov 2023 22:23:36 +0100
Subject: [PATCH] kernel: Remove dependency on CScheduler

By defining a virtual interface class for the scheduler client, users of
the kernel can now define their own event consuming infrastructure,
without having to spawn threads or rely on the scheduler design.

Removing CScheduler also allows removing the thread and
exception modules from the kernel library.
---
 src/Makefile.am                 |  7 +++---
 src/bitcoin-chainstate.cpp      | 16 +++-----------
 src/bitcoind.cpp                |  3 ++-
 src/node/interfaces.cpp         |  3 ++-
 src/scheduler.h                 |  9 ++++----
 src/test/util/setup_common.cpp  |  2 +-
 src/util/task_queue_interface.h | 39 +++++++++++++++++++++++++++++++++
 src/validationinterface.cpp     | 15 ++++++-------
 src/validationinterface.h       |  7 +++---
 9 files changed, 67 insertions(+), 34 deletions(-)
 create mode 100644 src/util/task_queue_interface.h

diff --git a/src/Makefile.am b/src/Makefile.am
index b6f0daaabadbdd..c3e60d2067b425 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -306,6 +306,7 @@ BITCOIN_CORE_H = \
   util/golombrice.h \
   util/hash_type.h \
   util/hasher.h \
+  util/immediate_task_queue.h \
   util/insert.h \
   util/macros.h \
   util/message.h \
@@ -321,6 +322,7 @@ BITCOIN_CORE_H = \
   util/spanparsing.h \
   util/string.h \
   util/syserror.h \
+  util/task_queue_interface.h \
   util/thread.h \
   util/threadinterrupt.h \
   util/threadnames.h \
@@ -739,6 +741,7 @@ libbitcoin_util_a_SOURCES = \
   util/fs.cpp \
   util/fs_helpers.cpp \
   util/hasher.cpp \
+  util/immediate_task_queue.cpp \
   util/sock.cpp \
   util/syserror.cpp \
   util/message.cpp \
@@ -960,7 +963,6 @@ libbitcoinkernel_la_SOURCES = \
   pubkey.cpp \
   random.cpp \
   randomenv.cpp \
-  scheduler.cpp \
   script/interpreter.cpp \
   script/script.cpp \
   script/script_error.cpp \
@@ -977,10 +979,10 @@ libbitcoinkernel_la_SOURCES = \
   util/batchpriority.cpp \
   util/chaintype.cpp \
   util/check.cpp \
-  util/exception.cpp \
   util/fs.cpp \
   util/fs_helpers.cpp \
   util/hasher.cpp \
+  util/immediate_task_queue.cpp \
   util/moneystr.cpp \
   util/rbf.cpp \
   util/serfloat.cpp \
@@ -988,7 +990,6 @@ libbitcoinkernel_la_SOURCES = \
   util/strencodings.cpp \
   util/string.cpp \
   util/syserror.cpp \
-  util/thread.cpp \
   util/threadnames.cpp \
   util/time.cpp \
   util/tokenpipe.cpp \
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 7d972325fb3d1b..37ade5b72e5453 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -23,11 +23,11 @@
 #include <node/caches.h>
 #include <node/chainstate.h>
 #include <random.h>
-#include <scheduler.h>
 #include <script/sigcache.h>
 #include <util/chaintype.h>
 #include <util/fs.h>
-#include <util/thread.h>
+#include <util/immediate_task_queue.h>
+#include <util/task_queue_interface.h>
 #include <validation.h>
 #include <validationinterface.h>
 
@@ -68,16 +68,7 @@ int main(int argc, char* argv[])
     Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
     Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));
 
-
-    // SETUP: Scheduling and Background Signals
-    CScheduler scheduler{};
-    // Start the lightweight task scheduler thread
-    scheduler.m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { scheduler.serviceQueue(); });
-
-    ValidationSignals validation_signals{scheduler};
-
-    // Gather some entropy once per minute.
-    scheduler.scheduleEvery(RandAddPeriodic, std::chrono::minutes{1});
+    ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskQueue>()};
 
     class KernelNotifications : public kernel::Notifications
     {
@@ -288,7 +279,6 @@ int main(int argc, char* argv[])
 epilogue:
     // Without this precise shutdown sequence, there will be a lot of nullptr
     // dereferencing and UB.
-    scheduler.stop();
     if (chainman.m_thread_load.joinable()) chainman.m_thread_load.join();
 
     validation_signals.FlushBackgroundCallbacks();
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index de1f62698179bf..3e5ff45ee5d743 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -20,6 +20,7 @@
 #include <node/context.h>
 #include <node/interface_ui.h>
 #include <noui.h>
+#include <scheduler.h>
 #include <util/check.h>
 #include <util/exception.h>
 #include <util/strencodings.h>
@@ -185,7 +186,7 @@ static bool AppInit(NodeContext& node)
         }
 
         node.scheduler = std::make_unique<CScheduler>();
-        node.validation_signals = std::make_unique<ValidationSignals>(*node.scheduler);
+        node.validation_signals = std::make_unique<ValidationSignals>(std::make_unique<SingleThreadedSchedulerClient>(*node.scheduler));
         node.kernel = std::make_unique<kernel::Context>();
         if (!AppInitSanityChecks(*node.kernel))
         {
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 126db19cadcdf9..f0591d20e79ddf 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -39,6 +39,7 @@
 #include <primitives/transaction.h>
 #include <rpc/protocol.h>
 #include <rpc/server.h>
+#include <scheduler.h>
 #include <support/allocators/secure.h>
 #include <sync.h>
 #include <txmempool.h>
@@ -100,7 +101,7 @@ class NodeImpl : public Node
         if (!AppInitParameterInteraction(args())) return false;
 
         m_context->scheduler = std::make_unique<CScheduler>();
-        m_context->validation_signals = std::make_unique<ValidationSignals>(*m_context->scheduler);
+        m_context->validation_signals = std::make_unique<ValidationSignals>(std::make_unique<SingleThreadedSchedulerClient>(*m_context->scheduler));
         m_context->kernel = std::make_unique<kernel::Context>();
         if (!AppInitSanityChecks(*m_context->kernel)) return false;
 
diff --git a/src/scheduler.h b/src/scheduler.h
index 9212582b978eac..8de5edf8faf339 100644
--- a/src/scheduler.h
+++ b/src/scheduler.h
@@ -8,6 +8,7 @@
 #include <attributes.h>
 #include <sync.h>
 #include <threadsafety.h>
+#include <util/task_queue_interface.h>
 
 #include <chrono>
 #include <condition_variable>
@@ -120,7 +121,7 @@ class CScheduler
  * B() will be able to observe all of the effects of callback A() which executed
  * before it.
  */
-class SingleThreadedSchedulerClient
+class SingleThreadedSchedulerClient : public util::TaskQueueInterface
 {
 private:
     CScheduler& m_scheduler;
@@ -141,15 +142,15 @@ class SingleThreadedSchedulerClient
      * Practically, this means that callbacks can behave as if they are executed
      * in order by a single thread.
      */
-    void AddToProcessQueue(std::function<void()> func) EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex);
+    void AddToProcessQueue(std::function<void()> func) override EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex);
 
     /**
      * Processes all remaining queue members on the calling thread, blocking until queue is empty
      * Must be called after the CScheduler has no remaining processing threads!
      */
-    void EmptyQueue() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex);
+    void EmptyQueue() override EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex);
 
-    size_t CallbacksPending() EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex);
+    size_t CallbacksPending() override EXCLUSIVE_LOCKS_REQUIRED(!m_callbacks_mutex);
 };
 
 #endif // BITCOIN_SCHEDULER_H
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 5015a6117818bc..879b1664f14113 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -139,7 +139,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
     AppInitParameterInteraction(*m_node.args);
     LogInstance().StartLogging();
     m_node.scheduler = std::make_unique<CScheduler>();
-    m_node.validation_signals = std::make_unique<ValidationSignals>(*m_node.scheduler);
+    m_node.validation_signals = std::make_unique<ValidationSignals>(std::make_unique<SingleThreadedSchedulerClient>(*m_node.scheduler));
     m_node.kernel = std::make_unique<kernel::Context>();
     SetupEnvironment();
 
diff --git a/src/util/task_queue_interface.h b/src/util/task_queue_interface.h
new file mode 100644
index 00000000000000..b29cc2d4fa80b1
--- /dev/null
+++ b/src/util/task_queue_interface.h
@@ -0,0 +1,39 @@
+// Copyright (c) 2023-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_TASK_QUEUE_INTERFACE_H
+#define BITCOIN_UTIL_TASK_QUEUE_INTERFACE_H
+
+#include <cstddef>
+#include <functional>
+
+namespace util {
+
+class TaskQueueInterface
+{
+public:
+    virtual ~TaskQueueInterface() {}
+
+    /**
+     * This is called for each subscriber on each validation interface event.
+     * The callback can either be queued for later/asynchronous/threaded
+     * processing, or be executed immediately for synchronous processing.
+     * Synchronous processing will block validation.
+     */
+    virtual void AddToProcessQueue(std::function<void()> func) = 0;
+
+    /**
+     * This is called to force the processing of all queued events.
+     */
+    virtual void EmptyQueue() = 0;
+
+    /**
+     * Returns the number of queued events.
+     */
+    virtual size_t CallbacksPending() = 0;
+};
+
+} // namespace util
+
+#endif // BITCOIN_UTIL_TASK_QUEUE_INTERFACE_H
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 137b994c9ba83d..8d0dfbc954c465 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -5,7 +5,6 @@
 
 #include <validationinterface.h>
 
-#include <attributes.h>
 #include <chain.h>
 #include <consensus/validation.h>
 #include <kernel/chain.h>
@@ -13,7 +12,7 @@
 #include <logging.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
-#include <scheduler.h>
+#include <util/check.h>
 
 #include <future>
 #include <unordered_map>
@@ -61,17 +60,17 @@ template<typename F> void ValidationSignalsImpl::Iterate(F&& f) EXCLUSIVE_LOCKS_
     }
 }
 
-ValidationSignals::ValidationSignals(CScheduler& scheduler)
-    : m_schedulerClient{scheduler} {}
+ValidationSignals::ValidationSignals(std::unique_ptr<util::TaskQueueInterface> schedulerclient)
+    : m_schedulerClient{std::move(Assert(schedulerclient))} {}
 
 void ValidationSignals::FlushBackgroundCallbacks()
 {
-    m_schedulerClient.EmptyQueue();
+    m_schedulerClient->EmptyQueue();
 }
 
 size_t ValidationSignals::CallbacksPending()
 {
-    return m_schedulerClient.CallbacksPending();
+    return m_schedulerClient->CallbacksPending();
 }
 
 void ValidationSignals::RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface> callbacks)
@@ -105,7 +104,7 @@ void ValidationSignals::UnregisterAllValidationInterfaces()
 
 void ValidationSignals::CallFunctionInValidationInterfaceQueue(std::function<void()> func)
 {
-    m_schedulerClient.AddToProcessQueue(std::move(func));
+    m_schedulerClient->AddToProcessQueue(std::move(func));
 }
 
 void ValidationSignals::SyncWithValidationInterfaceQueue()
@@ -127,7 +126,7 @@ void ValidationSignals::SyncWithValidationInterfaceQueue()
     do {                                                       \
         auto local_name = (name);                              \
         LOG_EVENT("Enqueuing " fmt, local_name, __VA_ARGS__);  \
-        m_schedulerClient.AddToProcessQueue([=] { \
+        m_schedulerClient->AddToProcessQueue([=] { \
             LOG_EVENT(fmt, local_name, __VA_ARGS__);           \
             event();                                           \
         });                                                    \
diff --git a/src/validationinterface.h b/src/validationinterface.h
index 96a63e53426251..7b720348b68fc2 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -9,8 +9,8 @@
 #include <kernel/chain.h>
 #include <kernel/cs_main.h>
 #include <primitives/transaction.h> // CTransaction(Ref)
-#include <scheduler.h>
 #include <sync.h>
+#include <util/task_queue_interface.h>
 
 #include <cstddef>
 #include <cstdint>
@@ -18,6 +18,7 @@
 #include <list>
 #include <memory>
 #include <unordered_map>
+#include <utility>
 
 class BlockValidationState;
 class CBlock;
@@ -198,10 +199,10 @@ class ValidationSignals {
     // We are not allowed to assume the scheduler only runs in one thread,
     // but must ensure all callbacks happen in-order, so we end up creating
     // our own queue here :(
-    SingleThreadedSchedulerClient m_schedulerClient;
+    std::unique_ptr<util::TaskQueueInterface> m_schedulerClient;
 
 public:
-    explicit ValidationSignals(CScheduler& scheduler LIFETIMEBOUND);
+    explicit ValidationSignals(std::unique_ptr<util::TaskQueueInterface> schedulerclient);
 
     /** Call any remaining callbacks on the calling thread */
     void FlushBackgroundCallbacks();