From a11a2ac1324df0125fb6e83a8c5d51b24c925c88 Mon Sep 17 00:00:00 2001
From: Sona Kurazyan <sona.kurazyan@qt.io>
Date: Mon, 26 Dec 2022 21:02:45 +0100
Subject: [PATCH] Fix crash when cancelling a QFuture that has continuation
 with context
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 24dedaeaa1a94bfe9ade2da2a2c9aa112241b07a).
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
Change-Id: Ief4b37f31e652988d13b03499505ac65c7889226
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit 9e61cc4f72a4fe86fa97f64238f1e3d940a7746b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/thread/qfuture_impl.h                 | 12 ++++++------
 tests/auto/corelib/thread/qfuture/tst_qfuture.cpp |  9 +++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h
index 9a60348e3de..58169bd05bf 100644
--- a/src/corelib/thread/qfuture_impl.h
+++ b/src/corelib/thread/qfuture_impl.h
@@ -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));
             });
diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
index 217dfddd2fe..9234204bdcb 100644
--- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
+++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
@@ -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()