From 3192975f1d177aa9f0bbd823c6387cfbfa943610 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 27 Jun 2017 19:07:52 -0400 Subject: [PATCH] Flush CValidationInterface callbacks prior to destruction Note that the CScheduler thread cant be running at this point, it has already been stopped with the rest of the init threadgroup. Thus, just calling any remaining loose callbacks during Shutdown() is sane. --- src/init.cpp | 13 +++++++++++++ src/scheduler.cpp | 9 +++++++++ src/scheduler.h | 3 +++ src/test/test_bitcoin.cpp | 1 + src/validationinterface.cpp | 4 ++++ src/validationinterface.h | 2 ++ 6 files changed, 32 insertions(+) diff --git a/src/init.cpp b/src/init.cpp index 94bba6820e497..12f29a4ca877f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -215,6 +215,19 @@ void Shutdown() fFeeEstimatesInitialized = false; } + // FlushStateToDisk generates a SetBestChain callback, which we should avoid missing + FlushStateToDisk(); + + // After there are no more peers/RPC left to give us new data which may generate + // CValidationInterface callbacks, flush them... + GetMainSignals().FlushBackgroundCallbacks(); + + // Any future callbacks will be dropped. This should absolutely be safe - if + // missing a callback results in an unrecoverable situation, unclean shutdown + // would too. The only reason to do the above flushes is to let the wallet catch + // up with our current chain to avoid any strange pruning edge cases and make + // next startup faster by avoiding rescan. + { LOCK(cs_main); if (pcoinsTip != NULL) { diff --git a/src/scheduler.cpp b/src/scheduler.cpp index a76a87e10a6ba..35bf0da4becac 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -191,3 +191,12 @@ void SingleThreadedSchedulerClient::AddToProcessQueue(std::function } MaybeScheduleProcessQueue(); } + +void SingleThreadedSchedulerClient::EmptyQueue() { + bool should_continue = true; + while (should_continue) { + ProcessQueue(); + LOCK(m_cs_callbacks_pending); + should_continue = !m_callbacks_pending.empty(); + } +} diff --git a/src/scheduler.h b/src/scheduler.h index 82036afdf0222..6a079f7749e8d 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -101,6 +101,9 @@ class SingleThreadedSchedulerClient { public: SingleThreadedSchedulerClient(CScheduler *pschedulerIn) : m_pscheduler(pschedulerIn) {} void AddToProcessQueue(std::function func); + + // Processes all remaining queue members on the calling thread, blocking until queue is empty + void EmptyQueue(); }; #endif diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index e6a8cc779dbf2..3ba81ed17b5e7 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -94,6 +94,7 @@ TestingSetup::~TestingSetup() UnregisterNodeSignals(GetNodeSignals()); threadGroup.interrupt_all(); threadGroup.join_all(); + GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); UnloadBlockIndex(); delete pcoinsTip; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 8edc7c398dae6..bf20d606f833b 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -44,6 +44,10 @@ void CMainSignals::UnregisterBackgroundSignalScheduler() { m_internals.reset(nullptr); } +void CMainSignals::FlushBackgroundCallbacks() { + m_internals->m_schedulerClient.EmptyQueue(); +} + CMainSignals& GetMainSignals() { return g_signals; diff --git a/src/validationinterface.h b/src/validationinterface.h index fbfe273b10c8f..568da66df283c 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -79,6 +79,8 @@ class CMainSignals { 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(); void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const CTransactionRef &);