From 4780a106b3d4478b4f24d03b272790cfc696e7d3 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                 |  4 +---
 src/bitcoin-chainstate.cpp      | 29 +++++++++++++++++----------
 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 | 35 +++++++++++++++++++++++++++++++++
 src/validationinterface.cpp     | 15 +++++++-------
 src/validationinterface.h       |  7 ++++---
 9 files changed, 76 insertions(+), 31 deletions(-)
 create mode 100644 src/util/task_queue_interface.h

diff --git a/src/Makefile.am b/src/Makefile.am
index b6f0daaabadbdd..e1e143b5dd394e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -321,6 +321,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 \
@@ -960,7 +961,6 @@ libbitcoinkernel_la_SOURCES = \
   pubkey.cpp \
   random.cpp \
   randomenv.cpp \
-  scheduler.cpp \
   script/interpreter.cpp \
   script/script.cpp \
   script/script_error.cpp \
@@ -977,7 +977,6 @@ 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 \
@@ -988,7 +987,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..609c316571e395 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -23,11 +23,10 @@
 #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/task_queue_interface.h>
 #include <validation.h>
 #include <validationinterface.h>
 
@@ -68,16 +67,27 @@ int main(int argc, char* argv[])
     Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
     Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));
 
+    class ImmediateTaskQueue : public TaskQueueInterface
+    {
+    public:
+        ImmediateTaskQueue() {}
+        void AddToProcessQueue(std::function<void()> func) override
+        {
+            func();
+        }
 
-    // SETUP: Scheduling and Background Signals
-    CScheduler scheduler{};
-    // Start the lightweight task scheduler thread
-    scheduler.m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { scheduler.serviceQueue(); });
+        void EmptyQueue() override
+        {
+            return;
+        }
 
-    ValidationSignals validation_signals{scheduler};
+        size_t CallbacksPending() override
+        {
+            return 0;
+        }
+    };
 
-    // Gather some entropy once per minute.
-    scheduler.scheduleEvery(RandAddPeriodic, std::chrono::minutes{1});
+    ValidationSignals validation_signals{std::make_unique<ImmediateTaskQueue>()};
 
     class KernelNotifications : public kernel::Notifications
     {
@@ -288,7 +298,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..ef22f7590a61ec 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 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..b8ba88d002e5ae
--- /dev/null
+++ b/src/util/task_queue_interface.h
@@ -0,0 +1,35 @@
+// 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_TASK_QUEUE_INTERFACE_QUEUE_H
+#define BITCOIN_TASK_QUEUE_INTERFACE_QUEUE_H
+
+#include <cstddef>
+#include <functional>
+
+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;
+};
+
+#endif // BITCOIN_TASK_QUEUE_INTERFACE_QUEUE_H
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 137b994c9ba83d..7e615134734344 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<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..5d387bb8bee438 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<TaskQueueInterface> m_schedulerClient;
 
 public:
-    explicit ValidationSignals(CScheduler& scheduler LIFETIMEBOUND);
+    explicit ValidationSignals(std::unique_ptr<TaskQueueInterface> schedulerclient);
 
     /** Call any remaining callbacks on the calling thread */
     void FlushBackgroundCallbacks();