From c4012ff5f08c02840ef42216e1273b08e6bf5234 Mon Sep 17 00:00:00 2001 From: Sona Kurazyan Date: Wed, 21 Jul 2021 10:59:44 +0200 Subject: [PATCH] Make QFutureWatcher::isFinished() consistent with the watched QFuture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All the getters of QFutureWatcher are consistent with the getters of the corresponding QFuture, except for the isFinished() method, which returns 'true' only after the finished() signal is delivered. This behavior might be unintuitive for the users. In particular, isFinished() returns 'false', even if it's called immediately after waitForFinished(). [ChangeLog][QtCore][QFutureWatcher][Important Behavior Changes] The QFutureWatcher::isFinished() method now indicates if the related QFuture is finished, instead of indicating if the finished() signal was delivered. This makes it consistent with the future that is being watched. Fixes: QTBUG-91048 Change-Id: I6ae9b882b23e06198a82c95b026491bd480b3bf0 Reviewed-by: Edward Welbourne Reviewed-by: Qt CI Bot Reviewed-by: Mårten Nordheim (cherry picked from commit 53e4a50c6b3c7359b9afc24f30c9517abdf9561a) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/thread/qfuturewatcher.cpp | 8 +- src/corelib/thread/qfuturewatcher_p.h | 1 - .../qfuturewatcher/tst_qfuturewatcher.cpp | 75 ++++++++++++------- 3 files changed, 52 insertions(+), 32 deletions(-) diff --git a/src/corelib/thread/qfuturewatcher.cpp b/src/corelib/thread/qfuturewatcher.cpp index d59b6755a4b..530539ca4e0 100644 --- a/src/corelib/thread/qfuturewatcher.cpp +++ b/src/corelib/thread/qfuturewatcher.cpp @@ -329,8 +329,7 @@ bool QFutureWatcherBase::isStarted() const */ bool QFutureWatcherBase::isFinished() const { - Q_D(const QFutureWatcherBase); - return d->finished; + return futureInterface().isFinished(); } /*! \fn template bool QFutureWatcher::isRunning() const @@ -480,8 +479,7 @@ void QFutureWatcherBase::disconnectNotify(const QMetaMethod &signal) */ QFutureWatcherBasePrivate::QFutureWatcherBasePrivate() : maximumPendingResultsReady(QThread::idealThreadCount() * 2), - resultAtConnected(0), - finished(true) /* the initial m_future is a canceledResult(), with Finished set */ + resultAtConnected(0) { } /*! @@ -500,7 +498,6 @@ void QFutureWatcherBase::disconnectOutputInterface(bool pendingAssignment) if (pendingAssignment) { Q_D(QFutureWatcherBase); d->pendingResultsReady.storeRelaxed(0); - d->finished = false; /* May soon be amended, during connectOutputInterface() */ } futureInterface().d->disconnectOutputInterface(d_func()); @@ -532,7 +529,6 @@ void QFutureWatcherBasePrivate::sendCallOutEvent(QFutureCallOutEvent *event) emit q->started(); break; case QFutureCallOutEvent::Finished: - finished = true; emit q->finished(); break; case QFutureCallOutEvent::Canceled: diff --git a/src/corelib/thread/qfuturewatcher_p.h b/src/corelib/thread/qfuturewatcher_p.h index b086b88773d..f9a628e4fba 100644 --- a/src/corelib/thread/qfuturewatcher_p.h +++ b/src/corelib/thread/qfuturewatcher_p.h @@ -78,7 +78,6 @@ public: int maximumPendingResultsReady; QAtomicInt resultAtConnected; - bool finished; }; QT_END_NAMESPACE diff --git a/tests/auto/corelib/thread/qfuturewatcher/tst_qfuturewatcher.cpp b/tests/auto/corelib/thread/qfuturewatcher/tst_qfuturewatcher.cpp index 0d759eebd6f..929047c07e0 100644 --- a/tests/auto/corelib/thread/qfuturewatcher/tst_qfuturewatcher.cpp +++ b/tests/auto/corelib/thread/qfuturewatcher/tst_qfuturewatcher.cpp @@ -64,13 +64,13 @@ private slots: void suspendEvents(); void suspended(); void suspendedEventsOrder(); - void finishedState(); void throttling(); void incrementalMapResults(); void incrementalFilterResults(); void qfutureSynchronizer(); void warnRace(); void matchFlags(); + void checkStateConsistency(); }; void sleeper() @@ -960,30 +960,6 @@ void tst_QFutureWatcher::suspendedEventsOrder() iface.reportFinished(); } -// Test that the finished state for the watcher gets -// set when the finished event is delivered. -// This means it will lag the finished state for the future, -// but makes it more useful. -void tst_QFutureWatcher::finishedState() -{ - QFutureInterface iface; - iface.reportStarted(); - QFuture future = iface.future(); - QFutureWatcher watcher; - QSignalSpy startedSpy(&watcher, &QFutureWatcher::started); - QSignalSpy finishedSpy(&watcher, &QFutureWatcher::finished); - - watcher.setFuture(future); - QVERIFY(startedSpy.wait()); - - iface.reportFinished(); - QVERIFY(future.isFinished()); - QVERIFY(!watcher.isFinished()); - - QVERIFY(finishedSpy.wait()); - QVERIFY(watcher.isFinished()); -} - /* Verify that throttling kicks in if you report a lot of results, and that it clears when the result events are processed. @@ -1178,6 +1154,55 @@ void tst_QFutureWatcher::matchFlags() QCOMPARE(watcher.isFinished(), future.isFinished()); } +void tst_QFutureWatcher::checkStateConsistency() +{ +#define CHECK_FAIL(state) \ + do { \ + if (QTest::currentTestFailed()) \ + QFAIL("checkState() failed, QFutureWatcher has inconistent state after " state "!"); \ + } while (false) + + QFutureWatcher futureWatcher; + + auto checkState = [&futureWatcher] { + QCOMPARE(futureWatcher.isStarted(), futureWatcher.future().isStarted()); + QCOMPARE(futureWatcher.isRunning(), futureWatcher.future().isRunning()); + QCOMPARE(futureWatcher.isCanceled(), futureWatcher.future().isCanceled()); + QCOMPARE(futureWatcher.isSuspended(), futureWatcher.future().isSuspended()); + QCOMPARE(futureWatcher.isSuspending(), futureWatcher.future().isSuspending()); + QCOMPARE(futureWatcher.isFinished(), futureWatcher.future().isFinished()); + }; + + checkState(); + CHECK_FAIL("default-constructing"); + + QFutureInterface fi; + futureWatcher.setFuture(fi.future()); + checkState(); + CHECK_FAIL("setting future"); + + fi.reportStarted(); + checkState(); + CHECK_FAIL("starting"); + + fi.future().suspend(); + checkState(); + CHECK_FAIL("suspending"); + + fi.reportSuspended(); + checkState(); + CHECK_FAIL("suspended"); + + fi.reportCanceled(); + checkState(); + CHECK_FAIL("canceling"); + + fi.reportFinished(); + checkState(); + CHECK_FAIL("finishing"); + +#undef CHECK_FAIL +} QTEST_MAIN(tst_QFutureWatcher) #include "tst_qfuturewatcher.moc"