Wait conditions example: code tidies

In no particular order:

* Clean up #includes.

* Document what is protected by the mutex.

* Use explicit, nullptr.

* Use lock managers, not manual calls to lock/unlock.

* Unlock the mutex before notifying the condition variables.

* Condition variables are always meant to be used in a while loop, and
  never with a plain if, because of spurious wakeups.

* Don't lock a mutex just to protect a plain integer. We have atomics
  for that use case.

* Remove an unneeded signal, therefore also the need of using
  Q_OBJECT and the inclusion of the moc-generated file.

Fixes: QTBUG-108860
Change-Id: I2afc77955b95de8aa5fb88048cd9feb217f83b4f
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 3a449bbb69c9a3c3a5bc6a052f2de98ab79be7e9)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Giuseppe D'Angelo 2022-02-11 18:26:46 +01:00 committed by Qt Cherry-pick Bot
parent 77490d15d7
commit ba13715e5c
2 changed files with 38 additions and 29 deletions

View File

@ -75,13 +75,16 @@
that the condition \c bufferNotEmpty is true, since \c
numUsedBytes is necessarily greater than 0.
We guard all accesses to the \c numUsedBytes variable with a
mutex. In addition, the QWaitCondition::wait() function accepts a
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:
@ -109,7 +112,7 @@
thread is the only one that can do anything; the consumer is
blocked waiting for the \c bufferNotEmpty condition to be
signalled (\c numUsedBytes is 0). Once the producer has put one
byte in the buffer, \c numUsedBytes is \c BufferSize - 1 and the
byte in the buffer, \c numUsedBytes is strictly greater than 0, and the
\c bufferNotEmpty condition is signalled. At that point, two
things can happen: Either the consumer thread takes over and
reads that byte, or the producer gets to produce a second byte.

View File

@ -1,21 +1,30 @@
// Copyright (C) 2016 The Qt Company Ltd.
// 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 <QtCore>
#include <QAtomicInt>
#include <QCoreApplication>
#include <QMutex>
#include <QMutexLocker>
#include <QObject>
#include <QRandomGenerator>
#include <QThread>
#include <QWaitCondition>
#include <stdio.h>
#include <stdlib.h>
//! [0]
const int DataSize = 100000;
constexpr int DataSize = 100000;
constexpr int BufferSize = 8192;
const int BufferSize = 8192;
QMutex mutex; // protects the buffer
char buffer[BufferSize];
QWaitCondition bufferNotEmpty;
QWaitCondition bufferNotFull;
QMutex mutex;
int numUsedBytes = 0;
QAtomicInt numUsedBytes;
//! [0]
//! [1]
@ -23,24 +32,25 @@ class Producer : public QThread
//! [1] //! [2]
{
public:
Producer(QObject *parent = NULL) : QThread(parent)
explicit Producer(QObject *parent = nullptr)
: QThread(parent)
{
}
private:
void run() override
{
for (int i = 0; i < DataSize; ++i) {
mutex.lock();
if (numUsedBytes == BufferSize)
bufferNotFull.wait(&mutex);
mutex.unlock();
{
const QMutexLocker locker(&mutex);
while (numUsedBytes.loadAcquire() == BufferSize)
bufferNotFull.wait(&mutex);
}
buffer[i % BufferSize] = "ACGT"[QRandomGenerator::global()->bounded(4)];
mutex.lock();
++numUsedBytes;
numUsedBytes.fetchAndAddRelease(1);
bufferNotEmpty.wakeAll();
mutex.unlock();
}
}
};
@ -50,32 +60,29 @@ public:
class Consumer : public QThread
//! [3] //! [4]
{
Q_OBJECT
public:
Consumer(QObject *parent = NULL) : QThread(parent)
explicit Consumer(QObject *parent = nullptr)
: QThread(parent)
{
}
private:
void run() override
{
for (int i = 0; i < DataSize; ++i) {
mutex.lock();
if (numUsedBytes == 0)
bufferNotEmpty.wait(&mutex);
mutex.unlock();
{
const QMutexLocker locker(&mutex);
while (numUsedBytes.loadAcquire() == 0)
bufferNotEmpty.wait(&mutex);
}
fprintf(stderr, "%c", buffer[i % BufferSize]);
mutex.lock();
--numUsedBytes;
numUsedBytes.fetchAndAddRelease(-1);
bufferNotFull.wakeAll();
mutex.unlock();
}
fprintf(stderr, "\n");
}
signals:
void stringConsumed(const QString &text);
};
//! [4]
@ -95,4 +102,3 @@ int main(int argc, char *argv[])
}
//! [6]
#include "waitconditions.moc"