QThread: fix a race condition when destroying or restarting thread from finished()

Since we do not keep the mutex locked in QThreadPrivate::finish,
We could have races if the thread is destroyed or restarted from
another thread while we are still in that function

This solve tst_QCoreApplication::deliverInDefinedOrder on mac

Regression since a43583e0221311b7fe666726a

Reviewed-by: Brad

4 files changed, 22 insertions, 3 deletions

diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp
index 69b70cb..71a4896 100644
--- a/src/corelib/thread/qthread.cpp
+++ b/src/corelib/thread/qthread.cpp
@@ -173,7 +173,8 @@ void QAdoptedThread::run()
 */
 
 QThreadPrivate::QThreadPrivate(QThreadData *d)
-    : QObjectPrivate(), running(false), finished(false), terminated(false), exited(false), returnCode(-1),
+    : QObjectPrivate(), running(false), finished(false), terminated(false),
+      isInFinish(false), exited(false), returnCode(-1),
       stackSize(0), priority(QThread::InheritPriority), data(d)
 {
 #if defined (Q_OS_UNIX)
@@ -403,6 +404,11 @@ QThread::~QThread()
     Q_D(QThread);
     {
         QMutexLocker locker(&d->mutex);
+        if (d->isInFinish) {
+            locker.unlock();
+            wait();
+            locker.relock();
+        }
         if (d->running && !d->finished)
             qWarning("QThread: Destroyed while thread is still running");
 
diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h
index d816aef..51c6991 100644
--- a/src/corelib/thread/qthread_p.h
+++ b/src/corelib/thread/qthread_p.h
@@ -126,6 +126,7 @@ public:
     bool running;
     bool finished;
     bool terminated;
+    bool isInFinish; //when in QThreadPrivate::finish
 
     bool exited;
     int returnCode;
diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp
index 3a3c2bb..b674ab8 100644
--- a/src/corelib/thread/qthread_unix.cpp
+++ b/src/corelib/thread/qthread_unix.cpp
@@ -341,7 +341,7 @@ void QThreadPrivate::finish(void *arg)
     QMutexLocker locker(&d->mutex);
 #endif
 
-
+    d->isInFinish = true;
     d->priority = QThread::InheritPriority;
     bool terminated = d->terminated;
     void *data = &d->data->tls;
@@ -371,6 +371,7 @@ void QThreadPrivate::finish(void *arg)
     d->running = false;
     d->finished = true;
 
+    d->isInFinish = false;
     d->thread_done.wakeAll();
 }
 
@@ -548,6 +549,10 @@ void QThread::start(Priority priority)
 {
     Q_D(QThread);
     QMutexLocker locker(&d->mutex);
+
+    if (d->isInFinish)
+        d->thread_done.wait(locker.mutex());
+
     if (d->running)
         return;
 
diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp
index 3706da8..74fedd3 100644
--- a/src/corelib/thread/qthread_win.cpp
+++ b/src/corelib/thread/qthread_win.cpp
@@ -324,6 +324,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway)
     QThreadPrivate *d = thr->d_func();
 
     QMutexLocker locker(lockAnyway ? &d->mutex : 0);
+    d->isInFinish = true;
     d->priority = QThread::InheritPriority;
     bool terminated = d->terminated;
     void **tls_data = reinterpret_cast<void **>(&d->data->tls);
@@ -348,7 +349,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway)
 
     d->running = false;
     d->finished = true;
-
+    d->isInFinish = false;
 
     if (!d->waiters) {
         CloseHandle(d->handle);
@@ -405,6 +406,12 @@ void QThread::start(Priority priority)
     Q_D(QThread);
     QMutexLocker locker(&d->mutex);
 
+    if (d->isInFinish) {
+        locker.unlock();
+        wait();
+        locker.relock();
+    }
+
     if (d->running)
         return;