Wait conditions example: fix an incorrect condition variable usage
3a449bbb69c9a3c3a5bc6a052f2de98ab79be7e9 amended the code to remove acquiring a lock when waking up a condition variable. It is fine to not have a lock associated when waking a condition variable; what I misunderstood was the scope of the lock, which (and this underlines the importance of commenting _what exactly_ a lock protects, for each and ever lock) protected both the buffer as well as the counter of the buffer. This made my reasoning flawed: it is necessary to keep the lock while notifying, otherwise the counterpart could verify the condition isn't satisfied and wait (e.g. see numUsedBytes==0), missing the wake from the other thread (which could arrive between the check and the wait). Amends the previous commit. Change-Id: If7db2d045331f1b33b976fb6bf6aa9117c41678f Pick-to: 5.15 6.2 6.4 6.5 Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
This commit is contained in:
parent
41c10d4db8
commit
fddeec60cb
@ -75,16 +75,13 @@
|
||||
that the condition \c bufferNotEmpty is true, since \c
|
||||
numUsedBytes is necessarily greater than 0.
|
||||
|
||||
The QWaitCondition::wait() function accepts a
|
||||
We guard all accesses to the \c numUsedBytes variable with a
|
||||
mutex. In addition, the QWaitCondition::wait() function accepts a
|
||||
mutex as its argument. This mutex is unlocked before the thread
|
||||
is put to sleep and locked when the thread wakes up. Furthermore,
|
||||
the transition from the locked state to the wait state is atomic,
|
||||
to prevent race conditions from occurring.
|
||||
|
||||
Accesses to the \c numUsedBytes variable do not need mutex
|
||||
protection, as that variable is a QAtomicInt; atomic variables
|
||||
do not participate in data races.
|
||||
|
||||
\section1 Consumer Class
|
||||
|
||||
Let's turn to the \c Consumer class:
|
||||
|
@ -2,7 +2,6 @@
|
||||
// Copyright (C) 2022 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
|
||||
// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
|
||||
|
||||
#include <QAtomicInt>
|
||||
#include <QCoreApplication>
|
||||
#include <QMutex>
|
||||
#include <QMutexLocker>
|
||||
@ -18,13 +17,12 @@
|
||||
constexpr int DataSize = 100000;
|
||||
constexpr int BufferSize = 8192;
|
||||
|
||||
QMutex mutex; // protects the buffer
|
||||
QMutex mutex; // protects the buffer and the counter
|
||||
char buffer[BufferSize];
|
||||
int numUsedBytes;
|
||||
|
||||
QWaitCondition bufferNotEmpty;
|
||||
QWaitCondition bufferNotFull;
|
||||
|
||||
QAtomicInt numUsedBytes;
|
||||
//! [0]
|
||||
|
||||
//! [1]
|
||||
@ -43,14 +41,17 @@ private:
|
||||
for (int i = 0; i < DataSize; ++i) {
|
||||
{
|
||||
const QMutexLocker locker(&mutex);
|
||||
while (numUsedBytes.loadAcquire() == BufferSize)
|
||||
while (numUsedBytes == BufferSize)
|
||||
bufferNotFull.wait(&mutex);
|
||||
}
|
||||
|
||||
buffer[i % BufferSize] = "ACGT"[QRandomGenerator::global()->bounded(4)];
|
||||
|
||||
numUsedBytes.fetchAndAddRelease(1);
|
||||
bufferNotEmpty.wakeAll();
|
||||
{
|
||||
const QMutexLocker locker(&mutex);
|
||||
++numUsedBytes;
|
||||
bufferNotEmpty.wakeAll();
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
@ -72,14 +73,17 @@ private:
|
||||
for (int i = 0; i < DataSize; ++i) {
|
||||
{
|
||||
const QMutexLocker locker(&mutex);
|
||||
while (numUsedBytes.loadAcquire() == 0)
|
||||
while (numUsedBytes == 0)
|
||||
bufferNotEmpty.wait(&mutex);
|
||||
}
|
||||
|
||||
fprintf(stderr, "%c", buffer[i % BufferSize]);
|
||||
|
||||
numUsedBytes.fetchAndAddRelease(-1);
|
||||
bufferNotFull.wakeAll();
|
||||
{
|
||||
const QMutexLocker locker(&mutex);
|
||||
--numUsedBytes;
|
||||
bufferNotFull.wakeAll();
|
||||
}
|
||||
}
|
||||
fprintf(stderr, "\n");
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user