QThread: avoid unlock/lock/unlock in ~QThread if state is Finishing

This is a corner-case scenario but valid because we tell users they can
destroy the QThread object right after finished() has been emitted. But
emitting finished() does not mean the launched thread has actually
exited: it may still be in Finishing state for an arbitrarily long time.
Completely aside from what else may run from other libraries, we only
destroy QThreadStorage and the thread's event dispatcher after
finished() has been emitted.

This commit avoids the unnecessary mutex unlocking in the destructor,
then QThread::wait() locking again, only to unlock yet again so that it
can perform the necessary low-level wait calls. The same for the return
path: wait() locked again to check the state, then unlocked, only for
the destructor to lock again. Now, QThreadPrivate::wait() is responsible
for returning with a locked mutex.

Pick-to: 6.8
Change-Id: I87adffb89f275accea18fffd6b4293861ea7cf39
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

src/corelib/thread/qthread.cpp

@@ -483,11 +483,8 @@ QThread::~QThread()
     Q_D(QThread);
     {
         QMutexLocker locker(&d->mutex);
-        if (d->threadState == QThreadPrivate::Finishing) {
-            locker.unlock();
-            wait();
-            locker.relock();
-        }
+        if (d->threadState == QThreadPrivate::Finishing)
+            d->wait(locker, QDeadlineTimer::Forever);
         if (d->threadState == QThreadPrivate::Running && !d->data->isAdopted)
             qFatal("QThread: Destroyed while thread '%ls' is still running", qUtf16Printable(objectName()));
 

src/corelib/thread/qthread_p.h

@@ -204,6 +204,8 @@ public:
     uint stackSize = 0;
     std::underlying_type_t<QThread::Priority> priority = QThread::InheritPriority;
 
+    bool wait(QMutexLocker<QMutex> &locker, QDeadlineTimer deadline);
+
 #ifdef Q_OS_UNIX
     QWaitCondition thread_done;
 

src/corelib/thread/qthread_unix.cpp

@@ -822,6 +822,17 @@ bool QThread::wait(QDeadlineTimer deadline)
 
     if (d->threadState == QThreadPrivate::NotStarted)
         return true;
+    if (d->threadState == QThreadPrivate::Finished)
+        return true;
+
+    return d->wait(locker, deadline);
+}
+
+bool QThreadPrivate::wait(QMutexLocker<QMutex> &locker, QDeadlineTimer deadline)
+{
+    Q_ASSERT(threadState != QThreadPrivate::Finished);
+    Q_ASSERT(locker.isLocked());
+    QThreadPrivate *d = this;
 
     while (d->threadState != QThreadPrivate::Finished) {
         if (!d->thread_done.wait(locker.mutex(), deadline))

src/corelib/thread/qthread_win.cpp

@@ -488,6 +488,14 @@ bool QThread::wait(QDeadlineTimer deadline)
     }
     if (d->threadState == QThreadPrivate::NotStarted || d->threadState == QThreadPrivate::Finished)
         return true;
+    return d->wait(locker, deadline);
+}
+
+bool QThreadPrivate::wait(QMutexLocker<QMutex> &locker, QDeadlineTimer deadline)
+{
+    Q_ASSERT(threadState != QThreadPrivate::Finished);
+    Q_ASSERT(locker.isLocked());
+    QThreadPrivate *d = this;
 
     ++d->waiters;
     locker.mutex()->unlock();