From d869e1230b6bd951eca48fd247557adc083bd249 Mon Sep 17 00:00:00 2001
From: Igor Kushnir <igorkuo@gmail.com>
Date: Tue, 16 Mar 2021 12:50:53 +0200
Subject: [PATCH] ConcurrentQueue::enqueue: increment jobsLeft before adding a
 job

ConcurrentQueueTest::randomCalls() built in Debug mode often crashes
when concurrent_queue_test.cpp is modified and ConcurrentQueueTest is
launched from Qt Creator so that the test is built and immediately run.
The crash is an assertion failure that occurs most of the time under
the described circumstances at different test data rows:
void YACReader::ConcurrentQueue::finalizeJobs(int): Assertion `jobsLeft >= count' failed.

The assertion fails because ConcurrentQueue::enqueue() adds a job into
the queue first and then increments jobsLeft. If the job is immediately
picked up and executed very fast, ConcurrentQueue::nextJob() can try to
finalize it before enqueue() increments jobsLeft.

Simply reordering the modifications of jobsLeft and _queue in enqueue()
ensures that jobsLeft is always non-negative and eliminates the
assertion failures. Note that ConcurrentQueue::finalizeJobs() is the
only other function that modifies (decreases) jobsLeft. finalizeJobs()
is always called *after* the queue's size is reduced. So the following
invariant is now maintained at all times and documented:
jobsLeft >= _queue.size().
---
 common/concurrent_queue.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/common/concurrent_queue.h b/common/concurrent_queue.h
index 5be10b0d..d862bf3d 100644
--- a/common/concurrent_queue.h
+++ b/common/concurrent_queue.h
@@ -34,13 +34,13 @@ public:
     void enqueue(std::function<void(void)> job)
     {
         {
-            std::lock_guard<std::mutex> lock(queueMutex);
-            _queue.emplace(job);
+            std::lock_guard<std::mutex> lock(jobsLeftMutex);
+            ++jobsLeft;
         }
 
         {
-            std::lock_guard<std::mutex> lock(jobsLeftMutex);
-            ++jobsLeft;
+            std::lock_guard<std::mutex> lock(queueMutex);
+            _queue.emplace(job);
         }
 
         jobAvailableVar.notify_one();
@@ -78,7 +78,7 @@ public:
 private:
     std::vector<std::thread> threads;
     std::queue<std::function<void(void)>> _queue;
-    int jobsLeft;
+    int jobsLeft; //!< @invariant jobsLeft >= _queue.size()
     bool bailout;
     std::condition_variable jobAvailableVar;
     std::condition_variable _waitVar;