Fix memory leaks in QFuture's continuations

There were two issues:

- Some of the continuations were allocating memory for the
continuation's context dynamically, but deleting the allocated memory
only if they were actually invoked. Since the continuations may not be
invoked at all, this could cause memory leaks. Fixed by postponing the
allocations to the point when the continuations need to be invoked.

- In other cases the parent future is captured by copy in the
continuation's lambda, which is then saved in the parent. This causes
the following problem: the data of the ref-counted parent will be
deleted as soon as its last copy gets deleted. But the saved
continuation will prevent it from being deleted, since it holds a copy
of parent. To break the circular dependency, instead of capturing the
parent inside the lambda, we can pass the parent's data directly to
continuation when calling it.

Fixes: QTBUG-87289
Change-Id: If340520b68f6e960bc80953ca18b796173d34f7b
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Karsten Heimrich <karsten.heimrich@qt.io>
(cherry picked from commit 5d26d40a5596be048be87f309df9264bac741be9)
Reviewed-by: Jarek Kobus <jaroslaw.kobus@qt.io>

src/corelib/thread/qfuture_impl.h

@@ -465,18 +465,20 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
         }
     }
 
-    Continuation<Function, ResultType, ParentResultType> *continuationJob = nullptr;
-    if (launchAsync) {
-        continuationJob = new AsyncContinuation<Function, ResultType, ParentResultType>(
-                std::forward<F>(func), *f, p, pool);
-    } else {
-        continuationJob = new SyncContinuation<Function, ResultType, ParentResultType>(
-                std::forward<F>(func), *f, p);
-    }
-
     p.setLaunchAsync(launchAsync);
 
-    auto continuation = [continuationJob, launchAsync]() mutable {
+    auto continuation = [func = std::forward<F>(func), p, pool,
+                         launchAsync](const QFutureInterfaceBase &parentData) mutable {
+        const auto parent = QFutureInterface<ParentResultType>(parentData).future();
+        Continuation<Function, ResultType, ParentResultType> *continuationJob = nullptr;
+        if (launchAsync) {
+            continuationJob = new AsyncContinuation<Function, ResultType, ParentResultType>(
+                    std::forward<Function>(func), parent, p, pool);
+        } else {
+            continuationJob = new SyncContinuation<Function, ResultType, ParentResultType>(
+                    std::forward<Function>(func), parent, p);
+        }
+
         bool isLaunched = continuationJob->execute();
         // If continuation is successfully launched, AsyncContinuation will be deleted
         // by the QThreadPool which has started it. Synchronous continuation will be
@@ -499,12 +501,14 @@ void Continuation<Function, ResultType, ParentResultType>::create(F &&func,
 {
     Q_ASSERT(f);
 
-    auto continuationJob = new AsyncContinuation<Function, ResultType, ParentResultType>(
-            std::forward<F>(func), *f, p, pool);
     p.setLaunchAsync(true);
     p.setThreadPool(pool);
 
-    auto continuation = [continuationJob]() mutable {
+    auto continuation = [func = std::forward<F>(func), p,
+                         pool](const QFutureInterfaceBase &parentData) mutable {
+        const auto parent = QFutureInterface<ParentResultType>(parentData).future();
+        auto continuationJob = new AsyncContinuation<Function, ResultType, ParentResultType>(
+                std::forward<Function>(func), parent, p, pool);
         bool isLaunched = continuationJob->execute();
         // If continuation is successfully launched, AsyncContinuation will be deleted
         // by the QThreadPool which has started it.
@@ -585,12 +589,12 @@ void FailureHandler<Function, ResultType>::create(F &&function, QFuture<ResultTy
 {
     Q_ASSERT(future);
 
-    FailureHandler<Function, ResultType> *failureHandler =
-            new FailureHandler<Function, ResultType>(std::forward<F>(function), *future, promise);
-
-    auto failureContinuation = [failureHandler]() mutable {
-        failureHandler->run();
-        delete failureHandler;
+    auto failureContinuation = [function = std::forward<F>(function),
+                                promise](const QFutureInterfaceBase &parentData) mutable {
+        const auto parent = QFutureInterface<ResultType>(parentData).future();
+        FailureHandler<Function, ResultType> failureHandler(std::forward<Function>(function),
+                                                            parent, promise);
+        failureHandler.run();
     };
 
     future->d.setContinuation(std::move(failureContinuation));
@@ -665,12 +669,14 @@ class CanceledHandler
 public:
     template<class F = Function>
     static QFuture<ResultType> create(F &&handler, QFuture<ResultType> *future,
-                                      QFutureInterface<ResultType> promise)
+                                      QFutureInterface<ResultType> &promise)
     {
         Q_ASSERT(future);
 
-        auto canceledContinuation = [parentFuture = *future, promise,
-                                     handler = std::forward<F>(handler)]() mutable {
+        auto canceledContinuation = [promise, handler = std::forward<F>(handler)](
+                                            const QFutureInterfaceBase &parentData) mutable {
+            auto parentFuture = QFutureInterface<ResultType>(parentData).future();
+
             promise.reportStarted();
 
             if (parentFuture.isCanceled()) {

src/corelib/thread/qfutureinterface.cpp

@@ -744,14 +744,14 @@ void QFutureInterfaceBasePrivate::setState(QFutureInterfaceBase::State newState)
     state.storeRelaxed(newState);
 }
 
-void QFutureInterfaceBase::setContinuation(std::function<void()> func)
+void QFutureInterfaceBase::setContinuation(std::function<void(const QFutureInterfaceBase &)> func)
 {
     QMutexLocker lock(&d->continuationMutex);
     // If the state is ready, run continuation immediately,
     // otherwise save it for later.
     if (isFinished()) {
         lock.unlock();
-        func();
+        func(*this);
     } else {
         d->continuation = std::move(func);
     }
@@ -762,7 +762,7 @@ void QFutureInterfaceBase::runContinuation() const
     QMutexLocker lock(&d->continuationMutex);
     if (d->continuation) {
         lock.unlock();
-        d->continuation();
+        d->continuation(*this);
     }
 }
 

src/corelib/thread/qfutureinterface.h

@@ -192,7 +192,7 @@ private:
 #endif
 
 protected:
-    void setContinuation(std::function<void()> func);
+    void setContinuation(std::function<void(const QFutureInterfaceBase &)> func);
     void runContinuation() const;
 
     void setLaunchAsync(bool value);
@@ -215,6 +215,7 @@ public:
     {
         refT();
     }
+    QFutureInterface(const QFutureInterfaceBase &dd) : QFutureInterfaceBase(dd) { refT(); }
     ~QFutureInterface()
     {
         if (!derefT())
@@ -424,6 +425,8 @@ public:
         : QFutureInterfaceBase(initialState)
     { }
 
+    QFutureInterface(const QFutureInterfaceBase &dd) : QFutureInterfaceBase(dd) { }
+
     static QFutureInterface<void> canceledResult()
     { return QFutureInterface(State(Started | Finished | Canceled)); }
 

src/corelib/thread/qfutureinterface_p.h

@@ -195,7 +195,7 @@ public:
     void setState(QFutureInterfaceBase::State state);
 
     // Wrapper for continuation
-    std::function<void()> continuation;
+    std::function<void(const QFutureInterfaceBase &)> continuation;
     QBasicMutex continuationMutex;
 
     bool launchAsync = false;