From fa609c4f76f215c19ea4021e78c102dee2b8c3d1 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 08:04:40 -0400
Subject: [PATCH 1/7] scheduler: Remove unused REVERSE_LOCK

No longer needed after commit d61f2bb076d8f
---
 src/scheduler.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/scheduler.cpp b/src/scheduler.cpp
index c4bd47310b7..8f69bc31bf0 100644
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -30,9 +30,6 @@ void CScheduler::serviceQueue()
     // is called.
     while (!shouldStop()) {
         try {
-            if (!shouldStop() && taskQueue.empty()) {
-                REVERSE_LOCK(lock);
-            }
             while (!shouldStop() && taskQueue.empty()) {
                 // Wait until there is something to do.
                 newTaskScheduled.wait(lock);

From fa9819695aac260be0ba170eb15ecba8cb519843 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 08:46:50 -0400
Subject: [PATCH 2/7] test: Remove unused scheduler.h include from the common
 setup

The common setup is included in virtually all tests, so it should be
as slim as possible.
---
 src/test/util/setup_common.cpp | 1 +
 src/test/util/setup_common.h   | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 3b7a7c8d121..709d357b8a5 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -19,6 +19,7 @@
 #include <rpc/blockchain.h>
 #include <rpc/register.h>
 #include <rpc/server.h>
+#include <scheduler.h>
 #include <script/sigcache.h>
 #include <streams.h>
 #include <txdb.h>
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index 2477f9ad06a..37cd791f565 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -11,7 +11,6 @@
 #include <node/context.h>
 #include <pubkey.h>
 #include <random.h>
-#include <scheduler.h>
 #include <txmempool.h>
 #include <util/string.h>
 

From fab2950d703217ec34b27e677e4f33ebbf99ca08 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 08:54:58 -0400
Subject: [PATCH 3/7] doc: Switch boost::thread to std::thread in scheduler

After commit d0ebd93 the scheduler itself no longer cares if the
serviceQueue is run in a std::thread or boost::thread. Change the
documentation to std::thread because we switched to C++11.
---
 src/scheduler.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/scheduler.h b/src/scheduler.h
index 1e641954846..29f005d49c7 100644
--- a/src/scheduler.h
+++ b/src/scheduler.h
@@ -5,11 +5,6 @@
 #ifndef BITCOIN_SCHEDULER_H
 #define BITCOIN_SCHEDULER_H
 
-//
-// NOTE:
-// boost::thread should be ported to std::thread
-// when we support C++11.
-//
 #include <condition_variable>
 #include <functional>
 #include <list>
@@ -26,7 +21,7 @@
 // CScheduler* s = new CScheduler();
 // s->scheduleFromNow(doSomething, std::chrono::milliseconds{11}); // Assuming a: void doSomething() { }
 // s->scheduleFromNow([=] { this->func(argument); }, std::chrono::milliseconds{3});
-// boost::thread* t = new boost::thread(std::bind(CScheduler::serviceQueue, s));
+// std::thread* t = new std::thread([&] { s->serviceQueue(); });
 //
 // ... then at program shutdown, make sure to call stop() to clean up the thread(s) running serviceQueue:
 // s->stop();

From fa9cca0550f3d0ee8c276146f40007f76fbb97c2 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 09:12:34 -0400
Subject: [PATCH 4/7] doc: Remove unused documentation about unimplemented
 features

---
 src/scheduler.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/scheduler.h b/src/scheduler.h
index 29f005d49c7..7ebe3df37fd 100644
--- a/src/scheduler.h
+++ b/src/scheduler.h
@@ -29,7 +29,6 @@
 // delete t;
 // delete s; // Must be done after thread is interrupted/joined.
 //
-
 class CScheduler
 {
 public:
@@ -62,8 +61,6 @@ public:
      */
     void MockForward(std::chrono::seconds delta_seconds);
 
-    // To keep things as simple as possible, there is no unschedule.
-
     // Services the queue 'forever'. Should be run in a thread,
     // and interrupted using boost::interrupt_thread
     void serviceQueue();

From fac43f9889f500bcb62d830c030dec42fe791031 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 08:43:25 -0400
Subject: [PATCH 5/7] scheduler: Replace stop(true) with StopWhenDrained()

This helps understanding the code at the call site without having to
look up the name of the argument or the default value.
---
 src/scheduler.cpp            | 12 ------------
 src/scheduler.h              | 16 ++++++++++++----
 src/test/scheduler_tests.cpp |  6 +++---
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/scheduler.cpp b/src/scheduler.cpp
index 8f69bc31bf0..c5d1788f773 100644
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -68,18 +68,6 @@ void CScheduler::serviceQueue()
     newTaskScheduled.notify_one();
 }
 
-void CScheduler::stop(bool drain)
-{
-    {
-        LOCK(newTaskMutex);
-        if (drain)
-            stopWhenEmpty = true;
-        else
-            stopRequested = true;
-    }
-    newTaskScheduled.notify_all();
-}
-
 void CScheduler::schedule(CScheduler::Function f, std::chrono::system_clock::time_point t)
 {
     {
diff --git a/src/scheduler.h b/src/scheduler.h
index 7ebe3df37fd..63f20f1335d 100644
--- a/src/scheduler.h
+++ b/src/scheduler.h
@@ -65,10 +65,18 @@ public:
     // and interrupted using boost::interrupt_thread
     void serviceQueue();
 
-    // Tell any threads running serviceQueue to stop as soon as they're
-    // done servicing whatever task they're currently servicing (drain=false)
-    // or when there is no work left to be done (drain=true)
-    void stop(bool drain=false);
+    /** Tell any threads running serviceQueue to stop as soon as the current task is done */
+    void stop()
+    {
+        WITH_LOCK(newTaskMutex, stopRequested = true);
+        newTaskScheduled.notify_all();
+    }
+    /** Tell any threads running serviceQueue to stop when there is no work left to be done */
+    void StopWhenDrained()
+    {
+        WITH_LOCK(newTaskMutex, stopWhenEmpty = true);
+        newTaskScheduled.notify_all();
+    }
 
     // Returns number of tasks waiting to be serviced,
     // and first and last task times
diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp
index 1395a7f38cc..eb03caacdd6 100644
--- a/src/test/scheduler_tests.cpp
+++ b/src/test/scheduler_tests.cpp
@@ -89,7 +89,7 @@ BOOST_AUTO_TEST_CASE(manythreads)
     }
 
     // Drain the task queue then exit threads
-    microTasks.stop(true);
+    microTasks.StopWhenDrained();
     microThreads.join_all(); // ... wait until all the threads are done
 
     int counterSum = 0;
@@ -155,7 +155,7 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)
     }
 
     // finish up
-    scheduler.stop(true);
+    scheduler.StopWhenDrained();
     threads.join_all();
 
     BOOST_CHECK_EQUAL(counter1, 100);
@@ -186,7 +186,7 @@ BOOST_AUTO_TEST_CASE(mockforward)
     scheduler.MockForward(std::chrono::minutes{5});
 
     // ensure scheduler has chance to process all tasks queued for before 1 ms from now.
-    scheduler.scheduleFromNow([&scheduler] { scheduler.stop(false); }, std::chrono::milliseconds{1});
+    scheduler.scheduleFromNow([&scheduler] { scheduler.stop(); }, std::chrono::milliseconds{1});
     scheduler_thread.join();
 
     // check that the queue only has one job remaining

From fa3d41b5ab8fe62bc2e36c3e1211aace69e6da65 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 09:03:11 -0400
Subject: [PATCH 6/7] doc: Switch scheduler to doxygen comments

---
 src/scheduler.h | 56 +++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/src/scheduler.h b/src/scheduler.h
index 63f20f1335d..ffb2572f207 100644
--- a/src/scheduler.h
+++ b/src/scheduler.h
@@ -12,23 +12,23 @@
 
 #include <sync.h>
 
-//
-// Simple class for background tasks that should be run
-// periodically or once "after a while"
-//
-// Usage:
-//
-// CScheduler* s = new CScheduler();
-// s->scheduleFromNow(doSomething, std::chrono::milliseconds{11}); // Assuming a: void doSomething() { }
-// s->scheduleFromNow([=] { this->func(argument); }, std::chrono::milliseconds{3});
-// std::thread* t = new std::thread([&] { s->serviceQueue(); });
-//
-// ... then at program shutdown, make sure to call stop() to clean up the thread(s) running serviceQueue:
-// s->stop();
-// t->join();
-// delete t;
-// delete s; // Must be done after thread is interrupted/joined.
-//
+/**
+ * Simple class for background tasks that should be run
+ * periodically or once "after a while"
+ *
+ * Usage:
+ *
+ * CScheduler* s = new CScheduler();
+ * s->scheduleFromNow(doSomething, std::chrono::milliseconds{11}); // Assuming a: void doSomething() { }
+ * s->scheduleFromNow([=] { this->func(argument); }, std::chrono::milliseconds{3});
+ * std::thread* t = new std::thread([&] { s->serviceQueue(); });
+ *
+ * ... then at program shutdown, make sure to call stop() to clean up the thread(s) running serviceQueue:
+ * s->stop();
+ * t->join();
+ * delete t;
+ * delete s; // Must be done after thread is interrupted/joined.
+ */
 class CScheduler
 {
 public:
@@ -37,7 +37,7 @@ public:
 
     typedef std::function<void()> Function;
 
-    // Call func at/after time t
+    /** Call func at/after time t */
     void schedule(Function f, std::chrono::system_clock::time_point t);
 
     /** Call f once after the delta has passed */
@@ -61,8 +61,10 @@ public:
      */
     void MockForward(std::chrono::seconds delta_seconds);
 
-    // Services the queue 'forever'. Should be run in a thread,
-    // and interrupted using boost::interrupt_thread
+    /**
+     * Services the queue 'forever'. Should be run in a thread,
+     * and interrupted using boost::interrupt_thread
+     */
     void serviceQueue();
 
     /** Tell any threads running serviceQueue to stop as soon as the current task is done */
@@ -78,12 +80,14 @@ public:
         newTaskScheduled.notify_all();
     }
 
-    // Returns number of tasks waiting to be serviced,
-    // and first and last task times
+    /**
+     * Returns number of tasks waiting to be serviced,
+     * and first and last task times
+     */
     size_t getQueueInfo(std::chrono::system_clock::time_point &first,
                         std::chrono::system_clock::time_point &last) const;
 
-    // Returns true if there are threads actively running in serviceQueue()
+    /** Returns true if there are threads actively running in serviceQueue() */
     bool AreThreadsServicingQueue() const;
 
 private:
@@ -128,8 +132,10 @@ public:
      */
     void AddToProcessQueue(std::function<void ()> func);
 
-    // Processes all remaining queue members on the calling thread, blocking until queue is empty
-    // Must be called after the CScheduler has no remaining processing threads!
+    /**
+     * Processes all remaining queue members on the calling thread, blocking until queue is empty
+     * Must be called after the CScheduler has no remaining processing threads!
+     */
     void EmptyQueue();
 
     size_t CallbacksPending();

From fa8337fcdbcab8368d5bf111c4b2e5acf25e6e22 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 28 May 2020 09:16:32 -0400
Subject: [PATCH 7/7] clang-format scheduler

Can easily be reviewed with the git options
--ignore-all-space --word-diff-regex=. -U0
---
 src/scheduler.cpp | 27 +++++++++++++++++----------
 src/scheduler.h   | 15 ++++++++-------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/src/scheduler.cpp b/src/scheduler.cpp
index c5d1788f773..7c361bf26f9 100644
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -110,8 +110,8 @@ void CScheduler::scheduleEvery(CScheduler::Function f, std::chrono::milliseconds
     scheduleFromNow([=] { Repeat(*this, f, delta); }, delta);
 }
 
-size_t CScheduler::getQueueInfo(std::chrono::system_clock::time_point &first,
-                             std::chrono::system_clock::time_point &last) const
+size_t CScheduler::getQueueInfo(std::chrono::system_clock::time_point& first,
+                                std::chrono::system_clock::time_point& last) const
 {
     LOCK(newTaskMutex);
     size_t result = taskQueue.size();
@@ -122,13 +122,15 @@ size_t CScheduler::getQueueInfo(std::chrono::system_clock::time_point &first,
     return result;
 }
 
-bool CScheduler::AreThreadsServicingQueue() const {
+bool CScheduler::AreThreadsServicingQueue() const
+{
     LOCK(newTaskMutex);
     return nThreadsServicingQueue;
 }
 
 
-void SingleThreadedSchedulerClient::MaybeScheduleProcessQueue() {
+void SingleThreadedSchedulerClient::MaybeScheduleProcessQueue()
+{
     {
         LOCK(m_cs_callbacks_pending);
         // Try to avoid scheduling too many copies here, but if we
@@ -140,8 +142,9 @@ void SingleThreadedSchedulerClient::MaybeScheduleProcessQueue() {
     m_pscheduler->schedule(std::bind(&SingleThreadedSchedulerClient::ProcessQueue, this), std::chrono::system_clock::now());
 }
 
-void SingleThreadedSchedulerClient::ProcessQueue() {
-    std::function<void ()> callback;
+void SingleThreadedSchedulerClient::ProcessQueue()
+{
+    std::function<void()> callback;
     {
         LOCK(m_cs_callbacks_pending);
         if (m_are_callbacks_running) return;
@@ -157,7 +160,8 @@ void SingleThreadedSchedulerClient::ProcessQueue() {
     struct RAIICallbacksRunning {
         SingleThreadedSchedulerClient* instance;
         explicit RAIICallbacksRunning(SingleThreadedSchedulerClient* _instance) : instance(_instance) {}
-        ~RAIICallbacksRunning() {
+        ~RAIICallbacksRunning()
+        {
             {
                 LOCK(instance->m_cs_callbacks_pending);
                 instance->m_are_callbacks_running = false;
@@ -169,7 +173,8 @@ void SingleThreadedSchedulerClient::ProcessQueue() {
     callback();
 }
 
-void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void ()> func) {
+void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void()> func)
+{
     assert(m_pscheduler);
 
     {
@@ -179,7 +184,8 @@ void SingleThreadedSchedulerClient::AddToProcessQueue(std::function<void ()> fun
     MaybeScheduleProcessQueue();
 }
 
-void SingleThreadedSchedulerClient::EmptyQueue() {
+void SingleThreadedSchedulerClient::EmptyQueue()
+{
     assert(!m_pscheduler->AreThreadsServicingQueue());
     bool should_continue = true;
     while (should_continue) {
@@ -189,7 +195,8 @@ void SingleThreadedSchedulerClient::EmptyQueue() {
     }
 }
 
-size_t SingleThreadedSchedulerClient::CallbacksPending() {
+size_t SingleThreadedSchedulerClient::CallbacksPending()
+{
     LOCK(m_cs_callbacks_pending);
     return m_callbacks_pending.size();
 }
diff --git a/src/scheduler.h b/src/scheduler.h
index ffb2572f207..d7fe00d1b47 100644
--- a/src/scheduler.h
+++ b/src/scheduler.h
@@ -84,8 +84,8 @@ public:
      * Returns number of tasks waiting to be serviced,
      * and first and last task times
      */
-    size_t getQueueInfo(std::chrono::system_clock::time_point &first,
-                        std::chrono::system_clock::time_point &last) const;
+    size_t getQueueInfo(std::chrono::system_clock::time_point& first,
+                        std::chrono::system_clock::time_point& last) const;
 
     /** Returns true if there are threads actively running in serviceQueue() */
     bool AreThreadsServicingQueue() const;
@@ -110,19 +110,20 @@ private:
  * B() will be able to observe all of the effects of callback A() which executed
  * before it.
  */
-class SingleThreadedSchedulerClient {
+class SingleThreadedSchedulerClient
+{
 private:
-    CScheduler *m_pscheduler;
+    CScheduler* m_pscheduler;
 
     RecursiveMutex m_cs_callbacks_pending;
-    std::list<std::function<void ()>> m_callbacks_pending GUARDED_BY(m_cs_callbacks_pending);
+    std::list<std::function<void()>> m_callbacks_pending GUARDED_BY(m_cs_callbacks_pending);
     bool m_are_callbacks_running GUARDED_BY(m_cs_callbacks_pending) = false;
 
     void MaybeScheduleProcessQueue();
     void ProcessQueue();
 
 public:
-    explicit SingleThreadedSchedulerClient(CScheduler *pschedulerIn) : m_pscheduler(pschedulerIn) {}
+    explicit SingleThreadedSchedulerClient(CScheduler* pschedulerIn) : m_pscheduler(pschedulerIn) {}
 
     /**
      * Add a callback to be executed. Callbacks are executed serially
@@ -130,7 +131,7 @@ public:
      * Practically, this means that callbacks can behave as if they are executed
      * in order by a single thread.
      */
-    void AddToProcessQueue(std::function<void ()> func);
+    void AddToProcessQueue(std::function<void()> func);
 
     /**
      * Processes all remaining queue members on the calling thread, blocking until queue is empty