Skip to content

Commit 1d24d38

Browse files
committed
Merge bitcoin/bitcoin#30435: init: change shutdown order of load block thread and scheduler
5fd4836 init: change shutdown order of load block thread and scheduler (Martin Zumsande) Pull request description: This avoids situations during a reindex, in which the shutdown doesn't finish since `LimitValidationInterfaceQueue()` is called by the load block thread when the scheduler is already stopped, in which case it would block indefinitely. This can lead to intermittent failures in `feature_reindex.py` (#30424), which I could locally reproduce with ```diff diff --git a/src/validation.cpp b/src/validation.cpp index 74f0e49..be1706fdaf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3446,6 +3446,7 @@ static void LimitValidationInterfaceQueue(ValidationSignals& signals) LOCKS_EXCL AssertLockNotHeld(cs_main); if (signals.CallbacksPending() > 10) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); signals.SyncWithValidationInterfaceQueue(); } } ``` It has also been reported by users running `reindex-chainstate` (#23234). I thought for a bit about potential downsides of changing this order, but couldn't find any. Fixes #30424 Fixes #23234 ACKs for top commit: maflcko: review ACK 5fd4836 hebasto: re-ACK 5fd4836. tdb3: ACK 5fd4836 BrandonOdiwuor: Code Review ACK 5fd4836 Tree-SHA512: 3b8894e99551c5d4392b55eaa718eee05841a7287aeef2978699e1d633d5234399fa2f5a3e71eac1508d97845906bd33e0e63e5351855139e7be04c421359b36
2 parents 24dffdd + 5fd4836 commit 1d24d38

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

src/init.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,11 @@ void Shutdown(NodeContext& node)
296296

297297
StopTorControl();
298298

299+
if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join();
299300
// After everything has been shut down, but before things get flushed, stop the
300-
// scheduler and load block thread.
301+
// the scheduler. After this point, SyncWithValidationInterfaceQueue() should not be called anymore
302+
// as this would prevent the shutdown from completing.
301303
if (node.scheduler) node.scheduler->stop();
302-
if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join();
303304

304305
// After the threads that potentially access these pointers have been stopped,
305306
// destruct and reset all to nullptr.

0 commit comments

Comments
 (0)