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().
This commit is contained in:
Igor Kushnir 2021-03-16 12:50:53 +02:00 committed by Luis Ángel San Martín
parent 57eb8d0171
commit d869e1230b

View File

@ -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;