QFuture: Gracefully handle a destroyed context in continuations

This patch relaxes the requirements on the context object of
continuations. Instead of having to stay alive during execution of the
whole chain, it now only has to stay alive during setup of the chain.
If the context object is destroyed before the chain finishes, the
respective future is canceled.

This patch works by using QFutureCallOutInterface and signals instead
of direct invocation of the continuation by the parent future, similar
to how QFutureWatcher is implemented.
If a continuation is used with a context object, a QBasicFutureWatcher
is connected to the QFuture via a QFutureCallOutInterface. When the
future finishes, QBasicFutureWatcher::finished() triggers the
continuation with a signal/slot connection.
This way, we require the context object to stay alive only during setup;
the required synchronization is guaranteed by the existing event and
signal-slot mechanisms. The continuation itself does not need to know
about the context object anymore.

[ChangeLog][QtCore][QFuture] Added support for context objects of
continuations being destroyed before the continuation finishes. In
these cases the future is cancelled immediately.

Fixes: QTBUG-112958
Change-Id: Ie0ef3470b2a0ccfa789d2ae7604b92e509c14591
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

9 files changed, 275 insertions, 63 deletions

diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt
index 42364f6f4a..6b9fbfe77c 100644
--- a/src/corelib/CMakeLists.txt
+++ b/src/corelib/CMakeLists.txt
@@ -689,6 +689,7 @@ qt_internal_extend_target(Core CONDITION QT_FEATURE_thread AND UNIX AND NOT APPL
 qt_internal_extend_target(Core CONDITION QT_FEATURE_future
     SOURCES
         thread/qexception.cpp thread/qexception.h
+        thread/qbasicfuturewatcher.cpp thread/qbasicfuturewatcher.h
         thread/qfuture.h
         thread/qfuture_impl.h
         thread/qfutureinterface.cpp thread/qfutureinterface.h thread/qfutureinterface_p.h
diff --git a/src/corelib/doc/snippets/code/src_corelib_thread_qfuture.cpp b/src/corelib/doc/snippets/code/src_corelib_thread_qfuture.cpp
index bbd61d6dd3..500d7cc7a5 100644
--- a/src/corelib/doc/snippets/code/src_corelib_thread_qfuture.cpp
+++ b/src/corelib/doc/snippets/code/src_corelib_thread_qfuture.cpp
@@ -427,3 +427,16 @@ const bool started = f.isStarted(); // started == true
 const bool running = f.isRunning(); // running == false
 const bool finished = f.isFinished(); // finished == true
 //! [36]
+
+//! [37]
+QObject *context = ...;
+auto future = ...;
+auto continuation = future.then(context, [context](Result result) {
+                               // ...
+                           }).onCanceled([context = QPointer(context)] {
+                               if (!context)
+                                   return;  // context was destroyed already
+                               // handle cancellation
+                           });
+
+//! [37]
diff --git a/src/corelib/thread/qbasicfuturewatcher.cpp b/src/corelib/thread/qbasicfuturewatcher.cpp
new file mode 100644
index 0000000000..2602995e8f
--- /dev/null
+++ b/src/corelib/thread/qbasicfuturewatcher.cpp
@@ -0,0 +1,80 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#include "qbasicfuturewatcher.h"
+#include "qcoreapplication.h"
+#include "qfutureinterface.h"
+#include "qfutureinterface_p.h"
+
+#include <QtCore/private/qobject_p.h>
+
+QT_BEGIN_NAMESPACE
+
+namespace QtPrivate {
+
+class QBasicFutureWatcherPrivate : public QObjectPrivate, QFutureCallOutInterface
+{
+public:
+    Q_DECLARE_PUBLIC(QBasicFutureWatcher)
+
+    QFutureInterfaceBase future;
+
+    void postCallOutEvent(const QFutureCallOutEvent &event) override;
+    void callOutInterfaceDisconnected() override;
+};
+
+void QBasicFutureWatcherPrivate::postCallOutEvent(const QFutureCallOutEvent &event)
+{
+    Q_Q(QBasicFutureWatcher);
+    if (q->thread() == QThread::currentThread()) {
+        // If we are in the same thread, don't queue up anything.
+        std::unique_ptr<QFutureCallOutEvent> clonedEvent(event.clone());
+        QCoreApplication::sendEvent(q, clonedEvent.get());
+    } else {
+        QCoreApplication::postEvent(q, event.clone());
+    }
+}
+
+void QBasicFutureWatcherPrivate::callOutInterfaceDisconnected()
+{
+    Q_Q(QBasicFutureWatcher);
+    QCoreApplication::removePostedEvents(q, QEvent::FutureCallOut);
+}
+
+/*
+ * QBasicFutureWatcher is a more lightweight version of QFutureWatcher for internal use
+ */
+QBasicFutureWatcher::QBasicFutureWatcher(QObject *parent)
+    : QObject(*new QBasicFutureWatcherPrivate, parent)
+{
+}
+
+QBasicFutureWatcher::~QBasicFutureWatcher()
+{
+    Q_D(QBasicFutureWatcher);
+    d->future.d->disconnectOutputInterface(d);
+}
+
+void QBasicFutureWatcher::setFuture(QFutureInterfaceBase &fi)
+{
+    Q_D(QBasicFutureWatcher);
+    d->future = fi;
+    d->future.d->connectOutputInterface(d);
+}
+
+bool QtPrivate::QBasicFutureWatcher::event(QEvent *event)
+{
+    if (event->type() == QEvent::FutureCallOut) {
+        QFutureCallOutEvent *callOutEvent = static_cast<QFutureCallOutEvent *>(event);
+        if (callOutEvent->callOutType == QFutureCallOutEvent::Finished)
+            emit finished();
+        return true;
+    }
+    return QObject::event(event);
+}
+
+} // namespace QtPrivate
+
+QT_END_NAMESPACE
+
+#include "moc_qbasicfuturewatcher.cpp"
diff --git a/src/corelib/thread/qbasicfuturewatcher.h b/src/corelib/thread/qbasicfuturewatcher.h
new file mode 100644
index 0000000000..49db7284e7
--- /dev/null
+++ b/src/corelib/thread/qbasicfuturewatcher.h
@@ -0,0 +1,39 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#ifndef QBASICFUTUREWATCHER_H
+#define QBASICFUTUREWATCHER_H
+
+#include <QtCore/qobject.h>
+
+QT_REQUIRE_CONFIG(future);
+
+QT_BEGIN_NAMESPACE
+
+class QFutureInterfaceBase;
+
+namespace QtPrivate {
+
+class QBasicFutureWatcherPrivate;
+
+class Q_CORE_EXPORT QBasicFutureWatcher : public QObject
+{
+    Q_OBJECT
+    Q_DECLARE_PRIVATE(QBasicFutureWatcher)
+public:
+    explicit QBasicFutureWatcher(QObject *parent = nullptr);
+    ~QBasicFutureWatcher() override;
+
+    void setFuture(QFutureInterfaceBase &fi);
+
+    bool event(QEvent *event) override;
+
+Q_SIGNALS:
+    void finished();
+};
+
+}
+
+QT_END_NAMESPACE
+
+#endif // QBASICFUTUREWATCHER_H
diff --git a/src/corelib/thread/qfuture.qdoc b/src/corelib/thread/qfuture.qdoc
index 7d0f639417..79bd121aee 100644
--- a/src/corelib/thread/qfuture.qdoc
+++ b/src/corelib/thread/qfuture.qdoc
@@ -1298,8 +1298,18 @@
     is attached. In this case it will be resolved in the current thread. Therefore, when
     in doubt, pass the context explicitly.
 
+    \target context_lifetime
+    If the \a context is destroyed before the chain has finished, the future is canceled.
+    This implies that a cancellation handler might be invoked when the \a context is not valid
+    anymore. To guard against this, capture the \a context as a QPointer:
+
+    \snippet code/src_corelib_thread_qfuture.cpp 37
+
+    When the context object is destroyed, cancellation happens immediately. Previous futures in the
+    chain are \e {not} cancelled and keep running until they are finished.
+
     \note When calling this method, it should be guaranteed that the \a context stays alive
-    throughout the execution of the chain.
+    during setup of the chain.
 
     \sa onFailed(), onCanceled()
 */
@@ -1364,8 +1374,11 @@
     be invoked from a non-gui thread. So \c this is provided as a context to \c .onFailed(),
     to make sure that it will be invoked in the main thread.
 
+    If the \a context is destroyed before the chain has finished, the future is canceled.
+    See \l {context_lifetime}{then()} for details.
+
     \note When calling this method, it should be guaranteed that the \a context stays alive
-    throughout the execution of the chain.
+    during setup of the chain.
 
     See the documentation of the other overload for more details about \a handler.
 
@@ -1427,8 +1440,11 @@
     invoked in the thread of the \a context object. This can be useful if the cancellation
     needs to be handled in a specific thread.
 
+    If the \a context is destroyed before the chain has finished, the future is canceled.
+    See \l {context_lifetime}{then()} for details.
+
     \note When calling this method, it should be guaranteed that the \a context stays alive
-    throughout the execution of the chain.
+    during setup of the chain.
 
     See the documentation of the other overload for more details about \a handler.
 
diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h
index 47a07801c7..53f2326cde 100644
--- a/src/corelib/thread/qfuture_impl.h
+++ b/src/corelib/thread/qfuture_impl.h
@@ -11,6 +11,7 @@
 #endif
 
 #include <QtCore/qglobal.h>
+#include <QtCore/qbasicfuturewatcher.h>
 #include <QtCore/qfutureinterface.h>
 #include <QtCore/qthreadpool.h>
 #include <QtCore/qexception.h>
@@ -597,21 +598,27 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
                                                                   QObject *context)
 {
     Q_ASSERT(f);
-
-    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 = QPromise(fi), parent]() mutable {
-                    SyncContinuation<Function, ResultType, ParentResultType> continuationJob(
-                            std::forward<Function>(func), parent, std::move(promise));
-                    continuationJob.execute();
-                });
+    Q_ASSERT(context);
+
+    // When the context object is destroyed, the signal-slot connection is broken and the
+    // continuation callback is destroyed. The promise that is created in the capture list is
+    // destroyed and, if it is not yet finished, cancelled.
+    auto continuation = [func = std::forward<F>(func), parent = *f,
+                         promise = QPromise(fi)]() mutable {
+        SyncContinuation<Function, ResultType, ParentResultType> continuationJob(
+                std::forward<Function>(func), parent, std::move(promise));
+        continuationJob.execute();
     };
-    f->d.setContinuation(ContinuationWrapper(std::move(continuation)), fi.d);
+
+    auto *watcher = new QBasicFutureWatcher;
+    watcher->moveToThread(context->thread());
+    QObject::connect(watcher, &QBasicFutureWatcher::finished,
+                     context, std::move(continuation));
+    QObject::connect(watcher, &QBasicFutureWatcher::finished,
+                     watcher, &QObject::deleteLater);
+    QObject::connect(context, &QObject::destroyed,
+                     watcher, &QObject::deleteLater);
+    watcher->setFuture(f->d);
 }
 
 template<typename Function, typename ResultType, typename ParentResultType>
@@ -695,22 +702,20 @@ void FailureHandler<Function, ResultType>::create(F &&function, QFuture<ResultTy
                                                   QObject *context)
 {
     Q_ASSERT(future);
+    Q_ASSERT(context);
+    auto failureContinuation = [function = std::forward<F>(function),
+                                parent = *future, promise = QPromise(fi)]() mutable {
+        FailureHandler<Function, ResultType> failureHandler(
+                std::forward<Function>(function), parent, std::move(promise));
+        failureHandler.run();
+    };
 
-    auto failureContinuation =
-            [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 = QPromise(fi), parent]() mutable {
-                    FailureHandler<Function, ResultType> failureHandler(
-                                std::forward<Function>(function), parent, std::move(promise));
-                    failureHandler.run();
-                });
-            };
-
-    future->d.setContinuation(ContinuationWrapper(std::move(failureContinuation)));
+    auto *watcher = new QBasicFutureWatcher;
+    watcher->moveToThread(context->thread());
+    QObject::connect(watcher, &QBasicFutureWatcher::finished, context, std::move(failureContinuation));
+    QObject::connect(watcher, &QBasicFutureWatcher::finished, watcher, &QObject::deleteLater);
+    QObject::connect(context, &QObject::destroyed, watcher, &QObject::deleteLater);
+    watcher->setFuture(future->d);
 }
 
 template<class Function, class ResultType>
@@ -796,19 +801,19 @@ public:
                        QObject *context)
     {
         Q_ASSERT(future);
-        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 = QPromise(fi), parentFuture,
-                                      handler = std::forward<F>(handler)]() mutable {
-                run(std::forward<F>(handler), parentFuture, std::move(promise));
-            });
+        Q_ASSERT(context);
+        auto canceledContinuation = [handler = std::forward<F>(handler),
+                                     parentFuture = *future, promise = QPromise(fi)]() mutable {
+            run(std::forward<F>(handler), parentFuture, std::move(promise));
         };
 
-        future->d.setContinuation(ContinuationWrapper(std::move(canceledContinuation)));
+        auto *watcher = new QBasicFutureWatcher;
+        watcher->moveToThread(context->thread());
+        QObject::connect(watcher, &QBasicFutureWatcher::finished,
+                         context, std::move(canceledContinuation));
+        QObject::connect(watcher, &QBasicFutureWatcher::finished, watcher, &QObject::deleteLater);
+        QObject::connect(context, &QObject::destroyed, watcher, &QObject::deleteLater);
+        watcher->setFuture(future->d);
     }
 
     template<class F = Function>
diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp
index 7784ec0939..5556551447 100644
--- a/src/corelib/thread/qfutureinterface.cpp
+++ b/src/corelib/thread/qfutureinterface.cpp
@@ -7,6 +7,7 @@
 
 #include <QtCore/qatomic.h>
 #include <QtCore/qthread.h>
+#include <QtCore/qvarlengtharray.h>
 #include <QtCore/private/qsimd_p.h> // for qYieldCpu()
 #include <private/qthreadpool_p.h>
 
@@ -751,23 +752,25 @@ void QFutureInterfaceBasePrivate::connectOutputInterface(QFutureCallOutInterface
 {
     QMutexLocker locker(&m_mutex);
 
+    QVarLengthArray<std::unique_ptr<QFutureCallOutEvent>, 3> events;
+
     const auto currentState = state.loadRelaxed();
     if (currentState & QFutureInterfaceBase::Started) {
-        interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Started));
+        events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Started));
         if (m_progress) {
-            interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
-                                                            m_progress->minimum,
-                                                            m_progress->maximum));
-            interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Progress,
-                                                            m_progressValue,
-                                                            m_progress->text));
+            events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
+                                                        m_progress->minimum,
+                                                        m_progress->maximum));
+            events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Progress,
+                                                        m_progressValue,
+                                                        m_progress->text));
         } else {
-            interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
-                                                            0,
-                                                            0));
-            interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Progress,
-                                                            m_progressValue,
-                                                            QString()));
+            events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::ProgressRange,
+                                                        0,
+                                                        0));
+            events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Progress,
+                                                        m_progressValue,
+                                                        QString()));
         }
     }
 
@@ -776,25 +779,29 @@ void QFutureInterfaceBasePrivate::connectOutputInterface(QFutureCallOutInterface
         while (it != data.m_results.end()) {
             const int begin = it.resultIndex();
             const int end = begin + it.batchSize();
-            interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::ResultsReady,
-                                                            begin,
-                                                            end));
+            events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::ResultsReady,
+                                                        begin,
+                                                        end));
             it.batchedAdvance();
         }
     }
 
     if (currentState & QFutureInterfaceBase::Suspended)
-        interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Suspended));
+        events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Suspended));
     else if (currentState & QFutureInterfaceBase::Suspending)
-        interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Suspending));
+        events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Suspending));
 
     if (currentState & QFutureInterfaceBase::Canceled)
-        interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Canceled));
+        events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Canceled));
 
     if (currentState & QFutureInterfaceBase::Finished)
-        interface->postCallOutEvent(QFutureCallOutEvent(QFutureCallOutEvent::Finished));
+        events.emplace_back(new QFutureCallOutEvent(QFutureCallOutEvent::Finished));
 
     outputConnections.append(interface);
+
+    locker.unlock();
+    for (auto &&event : events)
+        interface->postCallOutEvent(*event);
 }
 
 void QFutureInterfaceBasePrivate::disconnectOutputInterface(QFutureCallOutInterface *interface)
diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h
index ecb0f5b7ea..4db3ebb859 100644
--- a/src/corelib/thread/qfutureinterface.h
+++ b/src/corelib/thread/qfutureinterface.h
@@ -38,6 +38,8 @@ class CanceledHandler;
 template<class Function, class ResultType>
 class FailureHandler;
 #endif
+
+class QBasicFutureWatcher;
 }
 
 class Q_CORE_EXPORT QFutureInterfaceBase
@@ -176,6 +178,8 @@ private:
     friend class QtPrivate::FailureHandler;
 #endif
 
+    friend class QtPrivate::QBasicFutureWatcher;
+
     template<class T>
     friend class QPromise;
 
diff --git a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
index 08c417019e..79ca38209e 100644
--- a/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
+++ b/tests/auto/corelib/thread/qfuture/tst_qfuture.cpp
@@ -11,6 +11,7 @@
 #include <QVarLengthArray>
 #include <QSet>
 #include <QList>
+#include <private/qobject_p.h>
 
 #include <QTest>
 #include <qfuture.h>
@@ -139,6 +140,18 @@ private:
     std::function<void ()> m_fn;
 };
 
+// Emulates QWidget behavior by deleting its children early in the destructor
+// instead of leaving it to ~QObject()
+class FakeQWidget : public QObject
+{
+    Q_OBJECT
+public:
+    ~FakeQWidget() override {
+        auto *d = QObjectPrivate::get(this);
+        d->deleteChildren();
+    }
+};
+
 using UniquePtr = std::unique_ptr<int>;
 
 class tst_QFuture: public QObject
@@ -3264,6 +3277,40 @@ void tst_QFuture::continuationsWithContext()
         QCOMPARE(future.result(), 2);
     }
 
+    // Cancellation when the context object is destroyed
+    {
+        // Use something like QWidget which deletes its children early, i.e.
+        // before ~QObject() runs. This behavior can lead to side-effects
+        // like QPointers to the parent not being set to nullptr during child
+        // object destruction.
+        QPointer shortLivedContext = new FakeQWidget();
+        shortLivedContext->moveToThread(&thread);
+
+        QPromise<int> promise;
+        auto future = promise.future()
+                              .then(shortLivedContext, [&](int val) {
+                                  if (QThread::currentThread() != &thread)
+                                      return 0;
+                                  return val + 1000;
+                              })
+                              .onCanceled([&, ptr=QPointer(shortLivedContext)] {
+                                  if (QThread::currentThread() != &thread)
+                                      return 0;
+                                  if (ptr)
+                                      return 1;
+                                  return 2;
+                              });
+        promise.start();
+
+        QMetaObject::invokeMethod(shortLivedContext, [&]() {
+            delete shortLivedContext;
+        }, Qt::BlockingQueuedConnection);
+
+        promise.finish();
+
+        QCOMPARE(future.result(), 2);
+    }
+
 #ifndef QT_NO_EXCEPTIONS
     // .onFaled()
     {