[refactor] Make MainSignals RAII styled

Author: TheCharlatan

src/bitcoin-chainstate.cpp

@@ -74,13 +74,11 @@ int main(int argc, char* argv[])
     // Start the lightweight task scheduler thread
     scheduler.m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { scheduler.serviceQueue(); });
 
-    CMainSignals validation_signals{};
+    CMainSignals validation_signals{scheduler};
 
     // Gather some entropy once per minute.
     scheduler.scheduleEvery(RandAddPeriodic, std::chrono::minutes{1});
 
-    validation_signals.RegisterBackgroundSignalScheduler(scheduler);
-
     class KernelNotifications : public kernel::Notifications
     {
     public:
@@ -303,5 +301,4 @@ int main(int argc, char* argv[])
             }
         }
     }
-    validation_signals.UnregisterBackgroundSignalScheduler();
 }

src/init.cpp

@@ -377,7 +377,6 @@ void Shutdown(NodeContext& node)
     node.chain_clients.clear();
     if (node.validation_signals) {
         node.validation_signals->UnregisterAllValidationInterfaces();
-        node.validation_signals->UnregisterBackgroundSignalScheduler();
     }
     node.mempool.reset();
     node.fee_estimator.reset();
@@ -1143,17 +1142,18 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     assert(!node.scheduler);
     node.scheduler = std::make_unique<CScheduler>();
+    auto& scheduler = *node.scheduler;
 
     // Start the lightweight task scheduler thread
-    node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { node.scheduler->serviceQueue(); });
+    scheduler.m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { scheduler.serviceQueue(); });
 
     // Gather some entropy once per minute.
-    node.scheduler->scheduleEvery([]{
+    scheduler.scheduleEvery([]{
         RandAddPeriodic();
     }, std::chrono::minutes{1});
 
     // Check disk space every 5 minutes to avoid db corruption.
-    node.scheduler->scheduleEvery([&args, &node]{
+    scheduler.scheduleEvery([&args, &node]{
         constexpr uint64_t min_disk_space = 50 << 20; // 50 MB
         if (!CheckDiskSpace(args.GetBlocksDirPath(), min_disk_space)) {
             LogPrintf("Shutting down due to lack of disk space!\n");
@@ -1164,9 +1164,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     }, std::chrono::minutes{5});
 
     assert(!node.validation_signals);
-    node.validation_signals = std::make_unique<CMainSignals>();
+    node.validation_signals = std::make_unique<CMainSignals>(scheduler);
     auto& validation_signals = *node.validation_signals;
-    validation_signals.RegisterBackgroundSignalScheduler(*node.scheduler);
 
     // Create client interfaces for wallets that are supposed to be loaded
     // according to -wallet and -disablewallet options. This only constructs
@@ -1271,7 +1270,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
         // Flush estimates to disk periodically
         CBlockPolicyEstimator* fee_estimator = node.fee_estimator.get();
-        node.scheduler->scheduleEvery([fee_estimator] { fee_estimator->FlushFeeEstimates(); }, FEE_FLUSH_INTERVAL);
+        scheduler.scheduleEvery([fee_estimator] { fee_estimator->FlushFeeEstimates(); }, FEE_FLUSH_INTERVAL);
         validation_signals.RegisterValidationInterface(fee_estimator);
     }
 
@@ -1910,7 +1909,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     connOptions.m_i2p_accept_incoming = args.GetBoolArg("-i2pacceptincoming", DEFAULT_I2P_ACCEPT_INCOMING);
 
-    if (!node.connman->Start(*node.scheduler, connOptions)) {
+    if (!node.connman->Start(scheduler, connOptions)) {
         return false;
     }
 
@@ -1930,15 +1929,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     uiInterface.InitMessage(_("Done loading").translated);
 
     for (const auto& client : node.chain_clients) {
-        client->start(*node.scheduler);
+        client->start(scheduler);
     }
 
     BanMan* banman = node.banman.get();
-    node.scheduler->scheduleEvery([banman]{
+    scheduler.scheduleEvery([banman]{
         banman->DumpBanlist();
     }, DUMP_BANS_INTERVAL);
 
-    if (node.peerman) node.peerman->StartScheduledTasks(*node.scheduler);
+    if (node.peerman) node.peerman->StartScheduledTasks(scheduler);
 
 #if HAVE_SYSTEM
     StartupNotify(args);

src/test/util/setup_common.cpp

@@ -171,8 +171,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto
     // from blocking due to queue overrun.
     m_node.scheduler = std::make_unique<CScheduler>();
     m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); });
-    m_node.validation_signals = std::make_unique<CMainSignals>();
-    m_node.validation_signals->RegisterBackgroundSignalScheduler(*m_node.scheduler);
+    m_node.validation_signals = std::make_unique<CMainSignals>(*m_node.scheduler);
 
     m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>(FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES);
     m_node.mempool = std::make_unique<CTxMemPool>(MemPoolOptionsForTest(m_node));
@@ -205,7 +204,6 @@ ChainTestingSetup::~ChainTestingSetup()
 {
     if (m_node.scheduler) m_node.scheduler->stop();
     m_node.validation_signals->FlushBackgroundCallbacks();
-    m_node.validation_signals->UnregisterBackgroundSignalScheduler();
     m_node.connman.reset();
     m_node.banman.reset();
     m_node.addrman.reset();

src/validationinterface.cpp

@@ -94,31 +94,18 @@ class MainSignalsImpl
     }
 };
 
-CMainSignals::CMainSignals() {}
+CMainSignals::CMainSignals(CScheduler& scheduler)
+    : m_internals{std::make_unique<MainSignalsImpl>(scheduler)} {}
 
 CMainSignals::~CMainSignals() {}
 
-void CMainSignals::RegisterBackgroundSignalScheduler(CScheduler& scheduler)
-{
-    assert(!m_internals);
-    m_internals = std::make_unique<MainSignalsImpl>(scheduler);
-}
-
-void CMainSignals::UnregisterBackgroundSignalScheduler()
-{
-    m_internals.reset(nullptr);
-}
-
 void CMainSignals::FlushBackgroundCallbacks()
 {
-    if (m_internals) {
-        m_internals->m_schedulerClient.EmptyQueue();
-    }
+    m_internals->m_schedulerClient.EmptyQueue();
 }
 
 size_t CMainSignals::CallbacksPending()
 {
-    if (!m_internals) return 0;
     return m_internals->m_schedulerClient.CallbacksPending();
 }
 
@@ -143,16 +130,11 @@ void CMainSignals::UnregisterSharedValidationInterface(std::shared_ptr<CValidati
 
 void CMainSignals::UnregisterValidationInterface(CValidationInterface* callbacks)
 {
-    if (m_internals) {
-        m_internals->Unregister(callbacks);
-    }
+    m_internals->Unregister(callbacks);
 }
 
 void CMainSignals::UnregisterAllValidationInterfaces()
 {
-    if (!m_internals) {
-        return;
-    }
     m_internals->Clear();
 }
 

src/validationinterface.h

@@ -160,13 +160,10 @@ class CMainSignals {
     std::unique_ptr<MainSignalsImpl> m_internals;
 
 public:
-    CMainSignals();
+    CMainSignals(CScheduler& scheduler LIFETIMEBOUND);
+
     ~CMainSignals();
 
-    /** Register a CScheduler to give callbacks which should run in the background (may only be called once) */
-    void RegisterBackgroundSignalScheduler(CScheduler& scheduler);
-    /** Unregister a CScheduler to give callbacks which should run in the background - these callbacks will now be dropped! */
-    void UnregisterBackgroundSignalScheduler();
     /** Call any remaining callbacks on the calling thread */
     void FlushBackgroundCallbacks();