QThread::create(): request interruption and join on destruction

If one destroys a running QThread, so far the behavior has been to crash
(à la std::thread) -- assuming the thread hasn't already signalled that
it has finished. This behavior is hostile to solutions such as using
QThread::create(), which always require a wait() before destroying the
thread object.

We can use the opportunity to change the behavior without breaking any
valid code. Instead of crashing, inside QThread's destructor we can ask
the new thread to quit, and then join it (à la std::jthread). This
simplifies the implementation of long-living runnables and the code that
manages them.

Deploying this solution for the whole QThread class may not be entirely
painless. While no correct code would work differently with the proposed
changes, incorrect code that deletes a running thread would no longer
crash "loudly" -- instead, it might deadlock "quietly", have memory
corruptions, etc.

Hence I'm limiting this approach to only the threads created by
QThread::create(), at least for the time being. This also side-steps
perhaps the biggest problem of generalizing the approach, which is that
placing such interrupt+join logic into~QThread's destructor would cause
it to be run _after_ a QThread subclass' own destructor has run,
destroying the subclass' data members too early. This might create
an antipattern if one chooses to subclass QThread. With create(), a
subclass in question exists, and it indeed has NSDMs, but it's entirely
under our control (in fact, I'm placing the logic just in its dtor).

[ChangeLog][QtCore][QThread] Destroying a QThread object created by
QThread::create() while the thread that it manages is still running will
now automatically ask that thread to quit, and will wait until the
thread has finished. Before, this resulted in a program crash. See the
documentation of QThread::~QThread() for more details.

Change-Id: Ib268b13da422e277ee3ed6f6c7b2ecc8cea5750c
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

1 files changed, 62 insertions, 0 deletions

diff --git a/tests/auto/corelib/thread/qthread/tst_qthread.cpp b/tests/auto/corelib/thread/qthread/tst_qthread.cpp
index d4950cd42b..b9c5eeb243 100644
--- a/tests/auto/corelib/thread/qthread/tst_qthread.cpp
+++ b/tests/auto/corelib/thread/qthread/tst_qthread.cpp
@@ -28,6 +28,7 @@
 
 #include <QTest>
 #include <QTestEventLoop>
+#include <QSignalSpy>
 #include <QSemaphore>
 #include <QAbstractEventDispatcher>
 #include <QWinEventNotifier>
@@ -111,6 +112,7 @@ private slots:
     void quitLock();
 
     void create();
+    void createDestruction();
     void threadIdReuse();
 };
 
@@ -1591,6 +1593,66 @@ void tst_QThread::create()
 #endif // QT_CONFIG(cxx11_future)
 }
 
+void tst_QThread::createDestruction()
+{
+    for (int delay : {0, 10, 20}) {
+        auto checkForInterruptions = []() {
+            for (;;) {
+                if (QThread::currentThread()->isInterruptionRequested())
+                    return;
+                QThread::msleep(1);
+            }
+        };
+
+        QScopedPointer<QThread> thread(QThread::create(checkForInterruptions));
+        QSignalSpy finishedSpy(thread.get(), &QThread::finished);
+        QVERIFY(finishedSpy.isValid());
+
+        thread->start();
+        if (delay)
+            QThread::msleep(delay);
+        thread.reset();
+
+        QCOMPARE(finishedSpy.size(), 1);
+    }
+
+    for (int delay : {0, 10, 20}) {
+        auto runEventLoop = []() {
+            QEventLoop loop;
+            loop.exec();
+        };
+
+        QScopedPointer<QThread> thread(QThread::create(runEventLoop));
+        QSignalSpy finishedSpy(thread.get(), &QThread::finished);
+        QVERIFY(finishedSpy.isValid());
+
+        thread->start();
+        if (delay)
+            QThread::msleep(delay);
+        thread.reset();
+
+        QCOMPARE(finishedSpy.size(), 1);
+    }
+
+    for (int delay : {0, 10, 20}) {
+        auto runEventLoop = [delay]() {
+            if (delay)
+                QThread::msleep(delay);
+            QEventLoop loop;
+            loop.exec();
+        };
+
+        QScopedPointer<QThread> thread(QThread::create(runEventLoop));
+        QSignalSpy finishedSpy(thread.get(), &QThread::finished);
+        QVERIFY(finishedSpy.isValid());
+
+        thread->start();
+        thread.reset();
+
+        QCOMPARE(finishedSpy.size(), 1);
+    }
+}
+
 class StopableJob : public QObject
 {
     Q_OBJECT