From 83481c54e0421f5fce2018d1fd5f07b8f9be4f69 Mon Sep 17 00:00:00 2001
From: ericek111 <erik.brocko@letemsvetemapplem.eu>
Date: Mon, 30 May 2022 15:36:50 +0200
Subject: [PATCH] prevent race condition in isIdle

---
 libs/MyThreadPool/include/MyThreadPool.h      |  8 +++++--
 libs/MyThreadPool/include/SafePriorityQueue.h | 23 ++++++++++++++++---
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/libs/MyThreadPool/include/MyThreadPool.h b/libs/MyThreadPool/include/MyThreadPool.h
index 36d7705..cc2732a 100644
--- a/libs/MyThreadPool/include/MyThreadPool.h
+++ b/libs/MyThreadPool/include/MyThreadPool.h
@@ -119,9 +119,13 @@ private:
                 return;
             }
 
-            auto callable = std::move(queue.pop());
-            if (callable.has_value()) {
+            // Prevent possible race condition in isIdle().
+            // Perhaps can be reworked, e. g. by having each worker track its progress individually?
+            auto callable = std::move(queue.pop([&] {
                 threadsWorking++;
+            }));
+
+            if (callable.has_value()) {
                 callable.value()();
                 threadsWorking--;
                 poolCond.notify_all();
diff --git a/libs/MyThreadPool/include/SafePriorityQueue.h b/libs/MyThreadPool/include/SafePriorityQueue.h
index 1bf967f..8a2d8ce 100644
--- a/libs/MyThreadPool/include/SafePriorityQueue.h
+++ b/libs/MyThreadPool/include/SafePriorityQueue.h
@@ -17,6 +17,13 @@ template <typename T>
 class SafePriorityQueue {
 
 public:
+    /*
+     * Default value for functor-taking methods.
+     */
+    struct no_op {
+        constexpr void operator()() const { }
+    };
+
     void push(int priority, T& obj)
     {
         push(priority, std::forward<T>(obj));
@@ -41,7 +48,15 @@ public:
         queueLength++;
     }
 
-    std::optional<T> pop()
+    /**
+     * Pop (and return) the top-most item in the stack.
+     * @tparam F
+     * @param callback Will be executed after (and only after) an element is popped off the queue,
+     *                 but before the mutex is released -- good for count-keeping.
+     * @return If the queue is empty, std::nullopt is returned.
+     */
+    template <typename F = no_op>
+    std::optional<T> pop(F&& callback = no_op())
     {
         std::unique_lock<std::shared_mutex> lock(mtx);
 
@@ -51,10 +66,12 @@ public:
 
         queueLength--;
 
-        auto ret = std::move(queue.front());
+        auto ret = std::move(queue.front().second);
         queue.pop_front();
 
-        return ret.second;
+        callback();
+
+        return ret;
     }
 
     std::optional<T> peek() const