From 9efe2a8603cd9023bfebb96a3a7ca4a65002669d Mon Sep 17 00:00:00 2001
From: Sona Kurazyan <sona.kurazyan@qt.io>
Date: Mon, 24 Aug 2020 16:57:18 +0200
Subject: [PATCH] Fix QFuture::waitForFinished to wait until QFuture is started
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently QFuture::waitForFinished() exits as soon as the future is not
in the running state. If the user calls it before
QPromise::reportStarted() is called, it will exit immediately, because
nothing is running yet. Fix the behavior to wait for the finished state.

[ChangeLog][Important Behavior Changes][QtCore] Fixed the behavior of
QFuture::waitForFinished() to wait until the future is actually in the
finished state, instead of exiting as soon as it is not in the running
state. This prevents waitForFinished() from exiting immediately, if at
the moment of calling it the future is not started yet.

Task-number: QTBUG-84867
Change-Id: I12f5e95d8200cfffa5653b6aa566a625f8320ca8
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/corelib/thread/qfuture.qdoc               |  2 +-
 src/corelib/thread/qfutureinterface.cpp       |  4 +--
 src/corelib/thread/qfuturewatcher.cpp         |  2 +-
 .../corelib/thread/qfuture/tst_qfuture.cpp    | 25 +++++++++++++++++++
 .../thread/qpromise/snippet_qpromise.cpp      |  5 +---
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/corelib/thread/qfuture.qdoc b/src/corelib/thread/qfuture.qdoc
index b20875c423d..8235fbf1907 100644
--- a/src/corelib/thread/qfuture.qdoc
+++ b/src/corelib/thread/qfuture.qdoc
@@ -404,7 +404,7 @@
 /*! \fn template <typename T> void QFuture<T>::waitForFinished()
 
     Waits for the asynchronous computation to finish (including cancel()ed
-    computations).
+    computations), i.e. until isFinished() returns \c true.
 */
 
 /*! \fn template <typename T> T QFuture<T>::result() const
diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp
index a625e05aac3..600dd9bd3f1 100644
--- a/src/corelib/thread/qfutureinterface.cpp
+++ b/src/corelib/thread/qfutureinterface.cpp
@@ -422,7 +422,7 @@ void QFutureInterfaceBase::waitForResult(int resultIndex)
 void QFutureInterfaceBase::waitForFinished()
 {
     QMutexLocker lock(&d->m_mutex);
-    const bool alreadyFinished = !isRunningOrPending();
+    const bool alreadyFinished = isFinished();
     lock.unlock();
 
     if (!alreadyFinished) {
@@ -430,7 +430,7 @@ void QFutureInterfaceBase::waitForFinished()
 
         lock.relock();
 
-        while (isRunningOrPending())
+        while (!isFinished())
             d->waitCondition.wait(&d->m_mutex);
     }
 
diff --git a/src/corelib/thread/qfuturewatcher.cpp b/src/corelib/thread/qfuturewatcher.cpp
index 75f5e297e2f..d59b6755a4b 100644
--- a/src/corelib/thread/qfuturewatcher.cpp
+++ b/src/corelib/thread/qfuturewatcher.cpp
@@ -417,7 +417,7 @@ bool QFutureWatcherBase::isSuspended() const
 /*! \fn template <typename T> void QFutureWatcher<T>::waitForFinished()
 
     Waits for the asynchronous computation to finish (including cancel()ed
-    computations).
+    computations), i.e. until isFinished() returns \c true.
 */
 void QFutureWatcherBase::waitForFinished()
 {
diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
index c1b562e7f49..e6f6827f3f7 100644
--- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
+++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
@@ -144,6 +144,7 @@ private slots:
     void takeResultWorksForTypesWithoutDefaultCtor();
     void canceledFutureIsNotValid();
     void signalConnect();
+    void waitForFinished();
 
 private:
     using size_type = std::vector<int>::size_type;
@@ -3076,5 +3077,29 @@ void tst_QFuture::signalConnect()
     }
 }
 
+void tst_QFuture::waitForFinished()
+{
+    QFutureInterface<void> fi;
+    auto future = fi.future();
+
+    QScopedPointer<QThread> waitingThread (QThread::create([&] {
+        future.waitForFinished();
+    }));
+
+    waitingThread->start();
+
+    QVERIFY(!waitingThread->wait(200));
+    QVERIFY(!waitingThread->isFinished());
+
+    fi.reportStarted();
+    QVERIFY(!waitingThread->wait(200));
+    QVERIFY(!waitingThread->isFinished());
+
+    fi.reportFinished();
+
+    QVERIFY(waitingThread->wait());
+    QVERIFY(waitingThread->isFinished());
+}
+
 QTEST_MAIN(tst_QFuture)
 #include "tst_qfuture.moc"
diff --git a/tests/auto/corelib/thread/qpromise/snippet_qpromise.cpp b/tests/auto/corelib/thread/qpromise/snippet_qpromise.cpp
index 2975afca0f9..7180baccca2 100644
--- a/tests/auto/corelib/thread/qpromise/snippet_qpromise.cpp
+++ b/tests/auto/corelib/thread/qpromise/snippet_qpromise.cpp
@@ -75,11 +75,8 @@ void snippet_QPromise::basicExample()
     QPromise<int> promise;
     QFuture<int> future = promise.future();
 
-    // Note: calling reportStarted() prior to thread creation to enforce order
-    // of calls: first promise.reportStarted() then future.waitForFinished()
-    promise.reportStarted();  // notifies QFuture that the computation is started
-
     QScopedPointer<QThread> thread(QThread::create([] (QPromise<int> promise) {
+        promise.reportStarted();   // notifies QFuture that the computation is started
         promise.addResult(42);
         promise.reportFinished();  // notifies QFuture that the computation is finished
     }, std::move(promise)));