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.

Pick-to: 5.15 6.2 6.4
Fixes: QTBUG-108860
Change-Id: I2afc77955b95de8aa5fb88048cd9feb217f83b4f
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 38 insertions, 29 deletions

diff --git a/examples/corelib/threads/doc/src/waitconditions.qdoc b/examples/corelib/threads/doc/src/waitconditions.qdoc
index 90f911aab3..7715bd8ab6 100644
--- a/examples/corelib/threads/doc/src/waitconditions.qdoc
+++ b/examples/corelib/threads/doc/src/waitconditions.qdoc
@@ -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.
diff --git a/examples/corelib/threads/waitconditions/waitconditions.cpp b/examples/corelib/threads/waitconditions/waitconditions.cpp
index 72b9ed39a5..36f02f7896 100644
--- a/examples/corelib/threads/waitconditions/waitconditions.cpp
+++ b/examples/corelib/threads/waitconditions/waitconditions.cpp
@@ -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"