Merge bitcoin/bitcoin#30435: init: change shutdown order of load block thread and scheduler

5fd4836019 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 74f0e4975c..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 5fd4836019
  hebasto:
    re-ACK 5fd4836019.
  tdb3:
    ACK 5fd4836019
  BrandonOdiwuor:
    Code Review ACK 5fd4836019

Tree-SHA512: 3b8894e99551c5d4392b55eaa718eee05841a7287aeef2978699e1d633d5234399fa2f5a3e71eac1508d97845906bd33e0e63e5351855139e7be04c421359b36
This commit is contained in:
merge-script 2024-07-16 17:31:59 +01:00
commit 1d24d383b4
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1

View File

@ -296,10 +296,11 @@ void Shutdown(NodeContext& node)
StopTorControl(); StopTorControl();
// After everything has been shut down, but before things get flushed, stop the
// scheduler and load block thread.
if (node.scheduler) node.scheduler->stop();
if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join();
// After everything has been shut down, but before things get flushed, stop the
// the scheduler. After this point, SyncWithValidationInterfaceQueue() should not be called anymore
// as this would prevent the shutdown from completing.
if (node.scheduler) node.scheduler->stop();
// After the threads that potentially access these pointers have been stopped, // After the threads that potentially access these pointers have been stopped,
// destruct and reset all to nullptr. // destruct and reset all to nullptr.