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

Tree-SHA512: 3b8894e99551c5d4392b55eaa718eee05841a7287aeef2978699e1d633d5234399fa2f5a3e71eac1508d97845906bd33e0e63e5351855139e7be04c421359b36

Author: fanquake

src/init.cpp

@@ -238,9 +238,9 @@ void Shutdown(NodeContext& node)
 
     // After everything has been shut down, but before things get flushed, stop the
     // CScheduler/checkqueue, scheduler and load block thread.
+    if (node.chainman && node.chainman->m_load_block.joinable()) node.chainman->m_load_block.join();
     if (node.scheduler) node.scheduler->stop();
     if (node.reverification_scheduler) node.reverification_scheduler->stop();
-    if (node.chainman && node.chainman->m_load_block.joinable()) node.chainman->m_load_block.join();
     StopScriptCheckWorkerThreads();
 
     // After the threads that potentially access these pointers have been stopped,

test/functional/feature_filelock.py

@@ -27,8 +27,8 @@ def run_test(self):
         self.log.info("Check that we can't start a second bitcoind instance using the same datadir")
         expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running."
         self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir}', '-noserver'], expected_msg=expected_msg)
-        cookie_file = datadir / ".cookie"
-        assert cookie_file.exists()  # should not be deleted during the second bitcoind instance shutdown
+        cookie_file = datadir + "/.cookie"
+        assert os.path.isfile(cookie_file)  # should not be deleted during the second bitcoind instance shutdown
         if self.is_wallet_compiled():
             def check_wallet_filelock(descriptors):
                 wallet_name = ''.join([random.choice(string.ascii_lowercase) for _ in range(6)])