diff options
| -rw-r--r-- | src/corelib/compat/removed_api.cpp | 18 | ||||
| -rw-r--r-- | src/corelib/thread/qfutureinterface.cpp | 55 | ||||
| -rw-r--r-- | src/corelib/thread/qfutureinterface.h | 2 | ||||
| -rw-r--r-- | src/corelib/thread/qfutureinterface_p.h | 5 | ||||
| -rw-r--r-- | src/corelib/thread/qpromise.h | 1 | ||||
| -rw-r--r-- | tests/auto/corelib/thread/qfuture/tst_qfuture.cpp | 71 |
6 files changed, 112 insertions, 40 deletions
diff --git a/src/corelib/compat/removed_api.cpp b/src/corelib/compat/removed_api.cpp index f3f741d830..213b07f655 100644 --- a/src/corelib/compat/removed_api.cpp +++ b/src/corelib/compat/removed_api.cpp @@ -173,24 +173,6 @@ QCalendar::QCalendar(QLatin1StringView name) #include "qcollator.h" // inline function compare(ptr, n, ptr, n) (for MSVC) -#if QT_CONFIG(future) - -#include "qfutureinterface.h" -#include "private/qfutureinterface_p.h" - -void QFutureInterfaceBase::cleanContinuation() -{ - if (!d) - return; - - // This was called when the associated QPromise was being destroyed, - // but isn't used anymore. - QMutexLocker lock(&d->continuationMutex); - d->continuation = nullptr; -} - -#endif // QT_CONFIG(future) - #include "qhashfunctions.h" size_t qHash(const QByteArray &key, size_t seed) noexcept diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp index a82b7af873..b62ceaf91d 100644 --- a/src/corelib/thread/qfutureinterface.cpp +++ b/src/corelib/thread/qfutureinterface.cpp @@ -105,6 +105,13 @@ void QFutureInterfaceBase::cancel(QFutureInterfaceBase::CancelMode mode) break; } + // Cancel the continuations chain + QFutureInterfaceBasePrivate *next = d->continuationData; + while (next) { + next->continuationState = QFutureInterfaceBasePrivate::Canceled; + next = next->continuationData; + } + d->waitCondition.wakeAll(); d->pausedWaitCondition.wakeAll(); @@ -816,45 +823,55 @@ void QFutureInterfaceBase::setContinuation(std::function<void(const QFutureInter { QMutexLocker lock(&d->continuationMutex); - if (continuationFutureData) - continuationFutureData->parentData = d; - // If the state is ready, run continuation immediately, // otherwise save it for later. if (isFinished()) { lock.unlock(); func(*this); - } else { + lock.relock(); + } + // Unless the continuation has been cleaned earlier, we have to + // store the move-only continuation, to guarantee that the associated + // future's data stays alive. + if (d->continuationState != QFutureInterfaceBasePrivate::Cleaned) { d->continuation = std::move(func); + d->continuationData = continuationFutureData; } } +void QFutureInterfaceBase::cleanContinuation() +{ + if (!d) + return; + + QMutexLocker lock(&d->continuationMutex); + d->continuation = nullptr; + d->continuationState = QFutureInterfaceBasePrivate::Cleaned; +} + void QFutureInterfaceBase::runContinuation() const { QMutexLocker lock(&d->continuationMutex); if (d->continuation) { - auto fn = std::exchange(d->continuation, nullptr); + // Save the continuation in a local function, to avoid calling + // a null std::function below, in case cleanContinuation() is + // called from some other thread right after unlock() below. + auto fn = std::move(d->continuation); lock.unlock(); fn(*this); + + lock.relock(); + // Unless the continuation has been cleaned earlier, we have to + // store the move-only continuation, to guarantee that the associated + // future's data stays alive. + if (d->continuationState != QFutureInterfaceBasePrivate::Cleaned) + d->continuation = std::move(fn); } } bool QFutureInterfaceBase::isChainCanceled() const { - if (isCanceled()) - return true; - - auto parent = d->parentData; - while (parent) { - // If the future is in Canceled state because it had an exception, we want to - // continue checking the chain of parents for cancellation, otherwise if the exception - // is handled inside the chain, it won't be interrupted even though cancellation has - // been requested. - if ((parent->state.loadRelaxed() & Canceled) && !parent->hasException) - return true; - parent = parent->parentData; - } - return false; + return isCanceled() || d->continuationState == QFutureInterfaceBasePrivate::Canceled; } void QFutureInterfaceBase::setLaunchAsync(bool value) diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h index 820faa9ec2..eb251c7d14 100644 --- a/src/corelib/thread/qfutureinterface.h +++ b/src/corelib/thread/qfutureinterface.h @@ -183,9 +183,7 @@ protected: void setContinuation(std::function<void(const QFutureInterfaceBase &)> func); void setContinuation(std::function<void(const QFutureInterfaceBase &)> func, QFutureInterfaceBasePrivate *continuationFutureData); -#if QT_CORE_REMOVED_SINCE(6, 4) void cleanContinuation(); -#endif void runContinuation() const; void setLaunchAsync(bool value); diff --git a/src/corelib/thread/qfutureinterface_p.h b/src/corelib/thread/qfutureinterface_p.h index ec3517bab3..6258e61de7 100644 --- a/src/corelib/thread/qfutureinterface_p.h +++ b/src/corelib/thread/qfutureinterface_p.h @@ -141,7 +141,10 @@ public: QThreadPool *m_pool = nullptr; // Wrapper for continuation std::function<void(const QFutureInterfaceBase &)> continuation; - QFutureInterfaceBasePrivate *parentData = nullptr; + QFutureInterfaceBasePrivate *continuationData = nullptr; + + enum ContinuationState : quint8 { Default, Canceled, Cleaned }; + std::atomic<ContinuationState> continuationState { Default }; RefCount refCount = 1; QAtomicInt state; // reads and writes can happen unprotected, both must be atomic diff --git a/src/corelib/thread/qpromise.h b/src/corelib/thread/qpromise.h index d0f6e6ae66..43c30582be 100644 --- a/src/corelib/thread/qpromise.h +++ b/src/corelib/thread/qpromise.h @@ -41,6 +41,7 @@ public: d.cancelAndFinish(); // cancel and finalize the state d.runContinuation(); } + d.cleanContinuation(); } // Core QPromise APIs diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp index 638dc380f7..81120c6f67 100644 --- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp +++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp @@ -2240,6 +2240,26 @@ void tst_QFuture::then() QVERIFY(threadId1 != QThread::currentThreadId()); QVERIFY(threadId2 != QThread::currentThreadId()); } + + // QTBUG-106083 & QTBUG-105182 + { + QThread thread; + thread.start(); + + QObject context; + context.moveToThread(&thread); + + auto future = QtConcurrent::run([] { + return 42; + }).then([] (int result) { + return result + 1; + }).then(&context, [] (int result) { + return result + 1; + }); + QCOMPARE(future.result(), 44); + thread.quit(); + thread.wait(); + } } template<class Type, class Callable> @@ -3116,6 +3136,57 @@ void tst_QFuture::cancelContinuations() QCOMPARE(checkpoint, 3); } #endif // QT_NO_EXCEPTIONS + + // Check notifications from QFutureWatcher + { + QPromise<void> p; + auto f = p.future(); + + auto f1 = f.then([] {}); + auto f2 = f1.then([] {}); + + QFutureWatcher<void> watcher1, watcher2; + int state = 0; + QObject::connect(&watcher1, &QFutureWatcher<void>::started, [&] { + QCOMPARE(state, 0); + ++state; + }); + QObject::connect(&watcher1, &QFutureWatcher<void>::canceled, [&] { + QCOMPARE(state, 1); + ++state; + }); + QObject::connect(&watcher1, &QFutureWatcher<void>::finished, [&] { + QCOMPARE(state, 2); + ++state; + }); + QObject::connect(&watcher2, &QFutureWatcher<void>::started, [&] { + QCOMPARE(state, 3); + ++state; + }); + QObject::connect(&watcher2, &QFutureWatcher<void>::canceled, [&] { + QCOMPARE(state, 4); + ++state; + }); + QObject::connect(&watcher2, &QFutureWatcher<int>::finished, [&] { + QCOMPARE(state, 5); + ++state; + }); + + watcher1.setFuture(f1); + watcher2.setFuture(f2); + + p.start(); + f.cancel(); + p.finish(); + + qApp->processEvents(); + + QCOMPARE(state, 6); + QVERIFY(watcher1.isFinished()); + QVERIFY(watcher1.isCanceled()); + QVERIFY(watcher2.isFinished()); + QVERIFY(watcher2.isCanceled()); + } } void tst_QFuture::continuationsWithContext() |