From 4abde2c4e3fd9b66394b79874583bdc2a9132c36 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Mon, 27 Nov 2023 11:46:36 +0100 Subject: [PATCH] [refactor] Make MainSignals RAII styled --- src/bitcoin-chainstate.cpp | 5 +---- src/init.cpp | 21 ++++++++++----------- src/test/util/setup_common.cpp | 4 +--- src/validationinterface.cpp | 26 ++++---------------------- src/validationinterface.h | 7 ++----- 5 files changed, 18 insertions(+), 45 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 0514021a8ea..f90ae93eca6 100644 --- a/src/bitcoin-chainstate.cpp +++ b/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 @@ epilogue: } } } - validation_signals.UnregisterBackgroundSignalScheduler(); } diff --git a/src/init.cpp b/src/init.cpp index a5eeef589a2..a90f868977f 100644 --- a/src/init.cpp +++ b/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(); + 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(); + node.validation_signals = std::make_unique(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); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index aad26f2013a..8e9e771b53e 100644 --- a/src/test/util/setup_common.cpp +++ b/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(); m_node.scheduler->m_service_thread = std::thread(util::TraceThread, "scheduler", [&] { m_node.scheduler->serviceQueue(); }); - m_node.validation_signals = std::make_unique(); - m_node.validation_signals->RegisterBackgroundSignalScheduler(*m_node.scheduler); + m_node.validation_signals = std::make_unique(*m_node.scheduler); m_node.fee_estimator = std::make_unique(FeeestPath(*m_node.args), DEFAULT_ACCEPT_STALE_FEE_ESTIMATES); m_node.mempool = std::make_unique(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(); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index d1d562ff320..c61473be0a0 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -94,31 +94,18 @@ public: } }; -CMainSignals::CMainSignals() {} +CMainSignals::CMainSignals(CScheduler& scheduler) + : m_internals{std::make_unique(scheduler)} {} CMainSignals::~CMainSignals() {} -void CMainSignals::RegisterBackgroundSignalScheduler(CScheduler& scheduler) -{ - assert(!m_internals); - m_internals = std::make_unique(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_ptrUnregister(callbacks); - } + m_internals->Unregister(callbacks); } void CMainSignals::UnregisterAllValidationInterfaces() { - if (!m_internals) { - return; - } m_internals->Clear(); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 57e2716c119..fa63570c1fe 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -160,13 +160,10 @@ private: std::unique_ptr 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();