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
Pick-to: dev 6.0
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>

4 files changed, 36 insertions, 27 deletions

diff --git a/src/corelib/thread/qfuture_impl.h b/src/corelib/thread/qfuture_impl.h
index 2d3ea148e9..4998fa79aa 100644
--- a/src/corelib/thread/qfuture_impl.h
+++ b/src/corelib/thread/qfuture_impl.h
@@ -464,18 +464,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
@@ -498,12 +500,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.
@@ -584,12 +588,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));
@@ -664,12 +668,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()) {
diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp
index 57ddc7407e..7a8ba08103 100644
--- a/src/corelib/thread/qfutureinterface.cpp
+++ b/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);
     }
 }
 
diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h
index 8467047809..2432503cf7 100644
--- a/src/corelib/thread/qfutureinterface.h
+++ b/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)); }
 
diff --git a/src/corelib/thread/qfutureinterface_p.h b/src/corelib/thread/qfutureinterface_p.h
index 083d6d3962..df71d3d4b6 100644
--- a/src/corelib/thread/qfutureinterface_p.h
+++ b/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;