Merge #18234: refactor: Replace boost::mutex,condition_var,chrono with std equivalents in scheduler

70a6b529f306ff72ea1badf25e970a92b2b17ab3 lint-cppcheck: Remove -DHAVE_WORKING_BOOST_SLEEP_FOR (Anthony Towns)
294937b39de5924e772f8ed90d35c53290c8acab scheduler_tests: re-enable mockforward test (Anthony Towns)
cea19f685915be8affb2203184a549576194413f Drop unused reverselock.h (Anthony Towns)
d0ebd93270758ea97ea956b8821e17a2d001ea94 scheduler: switch from boost to std (Anthony Towns)
b9c426012770d166e6ebfab27689be44e6e89aa5 sync.h: add REVERSE_LOCK (Anthony Towns)
306f71b4eb4a0fd8e64f47dc008bc235b80b13d9 scheduler: don't rely on boost interrupt on shutdown (Anthony Towns)

Pull request description:

  Replacing boost functionality with C++11 stuff.

  Motivated by #18227, but should stand alone. Changing from `boost::condition_var` to `std::condition_var` means `threadGroup.interrupt_all` isn't enough to interrupt `serviceQueue` anymore, so that means calling `stop()` before `join_all()` is needed. And the existing reverselock.h code doesn't work with sync.h's DebugLock code (because the reversed lock won't be removed from `g_lockstack` which then leads to incorrect potential deadlock warnings), so I've replaced that with a dedicated class and macro that's aware of our debug lock behaviour.

  Fixes #16027, Fixes #14200, Fixes #18227

ACKs for top commit:
  laanwj:
    ACK 70a6b529f306ff72ea1badf25e970a92b2b17ab3

Tree-SHA512: d1da13adeabcf9186d114e2dad9a4fdbe2e440f7afbccde0c13dfbaf464efcd850b69d3371c5bf8b179d7ceb9d81f4af3cc22960b90834e41eaaf6d52ef7d331

4 files changed, 54 insertions, 24 deletions

diff --git a/src/test/reverselock_tests.cpp b/src/test/reverselock_tests.cpp
index 532fe143a..4e51b8c02 100644
--- a/src/test/reverselock_tests.cpp
+++ b/src/test/reverselock_tests.cpp
@@ -2,7 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <reverselock.h>
+#include <sync.h>
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
@@ -11,21 +11,50 @@ BOOST_FIXTURE_TEST_SUITE(reverselock_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(reverselock_basics)
 {
-    boost::mutex mutex;
-    boost::unique_lock<boost::mutex> lock(mutex);
+    Mutex mutex;
+    WAIT_LOCK(mutex, lock);
 
     BOOST_CHECK(lock.owns_lock());
     {
-        reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
+        REVERSE_LOCK(lock);
         BOOST_CHECK(!lock.owns_lock());
     }
     BOOST_CHECK(lock.owns_lock());
 }
 
+BOOST_AUTO_TEST_CASE(reverselock_multiple)
+{
+    Mutex mutex2;
+    Mutex mutex;
+    WAIT_LOCK(mutex2, lock2);
+    WAIT_LOCK(mutex, lock);
+
+    // Make sure undoing two locks succeeds
+    {
+        REVERSE_LOCK(lock);
+        BOOST_CHECK(!lock.owns_lock());
+        REVERSE_LOCK(lock2);
+        BOOST_CHECK(!lock2.owns_lock());
+    }
+    BOOST_CHECK(lock.owns_lock());
+    BOOST_CHECK(lock2.owns_lock());
+}
+
 BOOST_AUTO_TEST_CASE(reverselock_errors)
 {
-    boost::mutex mutex;
-    boost::unique_lock<boost::mutex> lock(mutex);
+    Mutex mutex2;
+    Mutex mutex;
+    WAIT_LOCK(mutex2, lock2);
+    WAIT_LOCK(mutex, lock);
+
+#ifdef DEBUG_LOCKORDER
+    // Make sure trying to reverse lock a previous lock fails
+    try {
+        REVERSE_LOCK(lock2);
+        BOOST_CHECK(false); // REVERSE_LOCK(lock2) succeeded
+    } catch(...) { }
+    BOOST_CHECK(lock2.owns_lock());
+#endif
 
     // Make sure trying to reverse lock an unlocked lock fails
     lock.unlock();
@@ -34,7 +63,7 @@ BOOST_AUTO_TEST_CASE(reverselock_errors)
 
     bool failed = false;
     try {
-        reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
+        REVERSE_LOCK(lock);
     } catch(...) {
         failed = true;
     }
@@ -49,7 +78,7 @@ BOOST_AUTO_TEST_CASE(reverselock_errors)
     lock.lock();
     BOOST_CHECK(lock.owns_lock());
     {
-        reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
+        REVERSE_LOCK(lock);
         BOOST_CHECK(!lock.owns_lock());
     }
 
diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp
index 00eec58bd..7d26840b7 100644
--- a/src/test/scheduler_tests.cpp
+++ b/src/test/scheduler_tests.cpp
@@ -11,13 +11,13 @@
 
 BOOST_AUTO_TEST_SUITE(scheduler_tests)
 
-static void microTask(CScheduler& s, boost::mutex& mutex, int& counter, int delta, boost::chrono::system_clock::time_point rescheduleTime)
+static void microTask(CScheduler& s, boost::mutex& mutex, int& counter, int delta, std::chrono::system_clock::time_point rescheduleTime)
 {
     {
         boost::unique_lock<boost::mutex> lock(mutex);
         counter += delta;
     }
-    boost::chrono::system_clock::time_point noTime = boost::chrono::system_clock::time_point::min();
+    std::chrono::system_clock::time_point noTime = std::chrono::system_clock::time_point::min();
     if (rescheduleTime != noTime) {
         CScheduler::Function f = std::bind(&microTask, std::ref(s), std::ref(mutex), std::ref(counter), -delta + 1, noTime);
         s.schedule(f, rescheduleTime);
@@ -45,15 +45,15 @@ BOOST_AUTO_TEST_CASE(manythreads)
     auto randomMsec = [](FastRandomContext& rc) -> int { return -11 + (int)rc.randrange(1012); }; // [-11, 1000]
     auto randomDelta = [](FastRandomContext& rc) -> int { return -1000 + (int)rc.randrange(2001); }; // [-1000, 1000]
 
-    boost::chrono::system_clock::time_point start = boost::chrono::system_clock::now();
-    boost::chrono::system_clock::time_point now = start;
-    boost::chrono::system_clock::time_point first, last;
+    std::chrono::system_clock::time_point start = std::chrono::system_clock::now();
+    std::chrono::system_clock::time_point now = start;
+    std::chrono::system_clock::time_point first, last;
     size_t nTasks = microTasks.getQueueInfo(first, last);
     BOOST_CHECK(nTasks == 0);
 
     for (int i = 0; i < 100; ++i) {
-        boost::chrono::system_clock::time_point t = now + boost::chrono::microseconds(randomMsec(rng));
-        boost::chrono::system_clock::time_point tReschedule = now + boost::chrono::microseconds(500 + randomMsec(rng));
+        std::chrono::system_clock::time_point t = now + std::chrono::microseconds(randomMsec(rng));
+        std::chrono::system_clock::time_point tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng));
         int whichCounter = zeroToNine(rng);
         CScheduler::Function f = std::bind(&microTask, std::ref(microTasks),
                                              std::ref(counterMutex[whichCounter]), std::ref(counter[whichCounter]),
@@ -71,14 +71,14 @@ BOOST_AUTO_TEST_CASE(manythreads)
         microThreads.create_thread(std::bind(&CScheduler::serviceQueue, &microTasks));
 
     UninterruptibleSleep(std::chrono::microseconds{600});
-    now = boost::chrono::system_clock::now();
+    now = std::chrono::system_clock::now();
 
     // More threads and more tasks:
     for (int i = 0; i < 5; i++)
         microThreads.create_thread(std::bind(&CScheduler::serviceQueue, &microTasks));
     for (int i = 0; i < 100; i++) {
-        boost::chrono::system_clock::time_point t = now + boost::chrono::microseconds(randomMsec(rng));
-        boost::chrono::system_clock::time_point tReschedule = now + boost::chrono::microseconds(500 + randomMsec(rng));
+        std::chrono::system_clock::time_point t = now + std::chrono::microseconds(randomMsec(rng));
+        std::chrono::system_clock::time_point tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng));
         int whichCounter = zeroToNine(rng);
         CScheduler::Function f = std::bind(&microTask, std::ref(microTasks),
                                              std::ref(counterMutex[whichCounter]), std::ref(counter[whichCounter]),
@@ -142,7 +142,6 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)
     BOOST_CHECK_EQUAL(counter2, 100);
 }
 
-/* disabled for now. See discussion in https://github.com/bitcoin/bitcoin/pull/18174
 BOOST_AUTO_TEST_CASE(mockforward)
 {
     CScheduler scheduler;
@@ -157,14 +156,14 @@ BOOST_AUTO_TEST_CASE(mockforward)
     scheduler.scheduleFromNow(dummy, 8*min_in_milli);
 
     // check taskQueue
-    boost::chrono::system_clock::time_point first, last;
+    std::chrono::system_clock::time_point first, last;
     size_t num_tasks = scheduler.getQueueInfo(first, last);
     BOOST_CHECK_EQUAL(num_tasks, 3ul);
 
     std::thread scheduler_thread([&]() { scheduler.serviceQueue(); });
 
     // bump the scheduler forward 5 minutes
-    scheduler.MockForward(boost::chrono::seconds(5*60));
+    scheduler.MockForward(std::chrono::seconds(5*60));
 
     // ensure scheduler has chance to process all tasks queued for before 1 ms from now.
     scheduler.scheduleFromNow([&scheduler]{ scheduler.stop(false); }, 1);
@@ -178,11 +177,10 @@ BOOST_AUTO_TEST_CASE(mockforward)
     BOOST_CHECK_EQUAL(counter, 2);
 
     // check that the time of the remaining job has been updated
-    boost::chrono::system_clock::time_point now = boost::chrono::system_clock::now();
-    int delta = boost::chrono::duration_cast<boost::chrono::seconds>(first - now).count();
+    std::chrono::system_clock::time_point now = std::chrono::system_clock::now();
+    int delta = std::chrono::duration_cast<std::chrono::seconds>(first - now).count();
     // should be between 2 & 3 minutes from now
     BOOST_CHECK(delta > 2*60 && delta < 3*60);
 }
-*/
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp
index 6474711ca..3550a0231 100644
--- a/src/test/txindex_tests.cpp
+++ b/src/test/txindex_tests.cpp
@@ -70,6 +70,8 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
     // shutdown sequence (c.f. Shutdown() in init.cpp)
     txindex.Stop();
 
+    // txindex job may be scheduled, so stop scheduler before destructing
+    m_node.scheduler->stop();
     threadGroup.interrupt_all();
     threadGroup.join_all();
 
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index fc736bc3a..53eb9ff43 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -140,6 +140,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
 
 TestingSetup::~TestingSetup()
 {
+    if (m_node.scheduler) m_node.scheduler->stop();
     threadGroup.interrupt_all();
     threadGroup.join_all();
     GetMainSignals().FlushBackgroundCallbacks();