Merge bitcoin/bitcoin#25040: refactor: Pass lifetimebound reference to SingleThreadedSchedulerClient

fa4652ce5995ace831b6a4d3125bfcac9563ff6f Pass lifetimebound reference to SingleThreadedSchedulerClient (MacroFake)

Pull request description:

  Currently a pointer is passed, which is confusing and requires run-time asserts to avoid nullptr dereference.

  All call sites can pass a reference, so do that. Also mark it LIFETIMEBOUND to avoid call sites passing a temporary. Also, unrelated cleanup in touched lines.

ACKs for top commit:
  pk-b2:
    ACK fa4652ce59
  jonatack:
    Code review ACK fa4652ce5995ace831b6a4d3125bfcac9563ff6f rebased to master, debug build, unit tests
  vincenzopalazzo:
    ACK fa4652ce59

Tree-SHA512: cd7ec77347e195d659b8892d34c1e9644d4f88552a4d5fa310dc1756eb27050a99d3098b0b0d27f8474230f82c178fd9e22e7018d8248d5e47a7f4caad395e25

src/scheduler.cpp

@@ -141,7 +141,7 @@ void SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()
         if (m_are_callbacks_running) return;
         if (m_callbacks_pending.empty()) return;
     }
-    m_pscheduler->schedule(std::bind(&SingleThreadedSchedulerClient::ProcessQueue, this), std::chrono::system_clock::now());
+    m_scheduler.schedule([this] { this->ProcessQueue(); }, std::chrono::system_clock::now());
 }
 
 void SingleThreadedSchedulerClient::ProcessQueue()
@@ -177,8 +177,6 @@ void SingleThreadedSchedulerClient::ProcessQueue()
 
 void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void()> func)
 {
-    assert(m_pscheduler);
-
     {
         LOCK(m_callbacks_mutex);
         m_callbacks_pending.emplace_back(std::move(func));
@@ -188,7 +186,7 @@ void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void()> func
 
 void SingleThreadedSchedulerClient::EmptyQueue()
 {
-    assert(!m_pscheduler->AreThreadsServicingQueue());
+    assert(!m_scheduler.AreThreadsServicingQueue());
     bool should_continue = true;
     while (should_continue) {
         ProcessQueue();

src/scheduler.h

@@ -5,13 +5,18 @@
 #ifndef BITCOIN_SCHEDULER_H
 #define BITCOIN_SCHEDULER_H
 
+#include <attributes.h>
+#include <sync.h>
+#include <threadsafety.h>
+
+#include <chrono>
 #include <condition_variable>
+#include <cstddef>
 #include <functional>
 #include <list>
 #include <map>
 #include <thread>
-
-#include <sync.h>
+#include <utility>
 
 /**
  * Simple class for background tasks that should be run
@@ -117,7 +122,7 @@ private:
 class SingleThreadedSchedulerClient
 {
 private:
-    CScheduler* m_pscheduler;
+    CScheduler& m_scheduler;
 
     Mutex m_callbacks_mutex;
     std::list<std::function<void()>> m_callbacks_pending GUARDED_BY(m_callbacks_mutex);
@@ -127,7 +132,7 @@ private:
     void ProcessQueue();
 
 public:
-    explicit SingleThreadedSchedulerClient(CScheduler* pschedulerIn) : m_pscheduler(pschedulerIn) {}
+    explicit SingleThreadedSchedulerClient(CScheduler& scheduler LIFETIMEBOUND) : m_scheduler{scheduler} {}
 
     /**
      * Add a callback to be executed. Callbacks are executed serially

src/test/scheduler_tests.cpp

@@ -128,8 +128,8 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)
     CScheduler scheduler;
 
     // each queue should be well ordered with respect to itself but not other queues
-    SingleThreadedSchedulerClient queue1(&scheduler);
-    SingleThreadedSchedulerClient queue2(&scheduler);
+    SingleThreadedSchedulerClient queue1(scheduler);
+    SingleThreadedSchedulerClient queue2(scheduler);
 
     // create more threads than queues
     // if the queues only permit execution of one task at once then
@@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)
     // if they don't we'll get out of order behaviour
     std::vector<std::thread> threads;
     for (int i = 0; i < 5; ++i) {
-        threads.emplace_back(std::bind(&CScheduler::serviceQueue, &scheduler));
+        threads.emplace_back([&] { scheduler.serviceQueue(); });
     }
 
     // these are not atomic, if SinglethreadedSchedulerClient prevents

src/validationinterface.cpp

@@ -45,7 +45,7 @@ public:
     // our own queue here :(
     SingleThreadedSchedulerClient m_schedulerClient;
 
-    explicit MainSignalsInstance(CScheduler *pscheduler) : m_schedulerClient(pscheduler) {}
+    explicit MainSignalsInstance(CScheduler& scheduler LIFETIMEBOUND) : m_schedulerClient(scheduler) {}
 
     void Register(std::shared_ptr<CValidationInterface> callbacks)
     {
@@ -97,7 +97,7 @@ static CMainSignals g_signals;
 void CMainSignals::RegisterBackgroundSignalScheduler(CScheduler& scheduler)
 {
     assert(!m_internals);
-    m_internals.reset(new MainSignalsInstance(&scheduler));
+    m_internals = std::make_unique<MainSignalsInstance>(scheduler);
 }
 
 void CMainSignals::UnregisterBackgroundSignalScheduler()