Fix crash when cancelling a QFuture that has continuation with context

To support cancellation of continuations attached via the parent future,
we store a pointer to continuation future's data in parent. This
requires preserving the lifetime of continuation future's data while the
parent is still alive (see 24dedaeaa1).
This is achieved by capturing the promise in the continuation's lambda,
which is only cleaned up after the parent's data is destroyed. This is
already the case for continuations without context, but was overlooked
for continuations with context: they transfer the ownership of the
continuation promise to lambda passed to QMetaObject::invokeMethod(),
which destroys the lambda's context after it's run. As a result, the
continuation's promise (and data, if there are no other copies of it)
is also destroyed, leaving the parent pointing to deleted continuation
data.

To fix this, capture a copy of continuation future's ref-counted data in
the continuation's lambda. This will guarantee that the continuation
data remains alive until the parent is destroyed and the continuation
is cleaned up.

Fixes: QTBUG-108790
Pick-to: 6.5 6.4 6.2
Change-Id: Ief4b37f31e652988d13b03499505ac65c7889226
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Sona Kurazyan 2022-12-26 21:02:45 +01:00 committed by Sona Kurazyan
parent f9aaa3e163
commit 9e61cc4f72
2 changed files with 15 additions and 6 deletions

View File

@ -592,14 +592,14 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
{
Q_ASSERT(f);
auto continuation = [func = std::forward<F>(func), promise = QPromise(fi),
auto continuation = [func = std::forward<F>(func), fi,
context = QPointer<QObject>(context)](
const QFutureInterfaceBase &parentData) mutable {
Q_ASSERT(context);
const auto parent = QFutureInterface<ParentResultType>(parentData).future();
QMetaObject::invokeMethod(
context,
[func = std::forward<F>(func), promise = std::move(promise), parent]() mutable {
[func = std::forward<F>(func), promise = QPromise(fi), parent]() mutable {
SyncContinuation<Function, ResultType, ParentResultType> continuationJob(
std::forward<Function>(func), parent, std::move(promise));
continuationJob.execute();
@ -691,13 +691,13 @@ void FailureHandler<Function, ResultType>::create(F &&function, QFuture<ResultTy
Q_ASSERT(future);
auto failureContinuation =
[function = std::forward<F>(function), promise = QPromise(fi),
[function = std::forward<F>(function), fi,
context = QPointer<QObject>(context)](const QFutureInterfaceBase &parentData) mutable {
Q_ASSERT(context);
const auto parent = QFutureInterface<ResultType>(parentData).future();
QMetaObject::invokeMethod(context,
[function = std::forward<F>(function),
promise = std::move(promise), parent]() mutable {
promise = QPromise(fi), parent]() mutable {
FailureHandler<Function, ResultType> failureHandler(
std::forward<Function>(function), parent, std::move(promise));
failureHandler.run();
@ -790,13 +790,13 @@ public:
QObject *context)
{
Q_ASSERT(future);
auto canceledContinuation = [promise = QPromise(fi), handler = std::forward<F>(handler),
auto canceledContinuation = [fi, handler = std::forward<F>(handler),
context = QPointer<QObject>(context)](
const QFutureInterfaceBase &parentData) mutable {
Q_ASSERT(context);
auto parentFuture = QFutureInterface<ResultType>(parentData).future();
QMetaObject::invokeMethod(context,
[promise = std::move(promise), parentFuture,
[promise = QPromise(fi), parentFuture,
handler = std::forward<F>(handler)]() mutable {
run(std::forward<F>(handler), parentFuture, std::move(promise));
});

View File

@ -3187,6 +3187,15 @@ void tst_QFuture::cancelContinuations()
QVERIFY(watcher2.isFinished());
QVERIFY(watcher2.isCanceled());
}
// Cancel continuations with context (QTBUG-108790)
{
// This test should pass with ASan
auto future = QtConcurrent::run([] {});
future.then(this, [] {});
future.waitForFinished();
future.cancel();
}
}
void tst_QFuture::continuationsWithContext()