From a8794503ebdefec53a7a42479c477d04f0efa067 Mon Sep 17 00:00:00 2001
From: Sona Kurazyan <sona.kurazyan@qt.io>
Date: Mon, 17 Jan 2022 14:45:32 +0100
Subject: Fix memory leaks when capturing a QFuture in its continuation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Capturing a QFuture in the continuations attached to it results in
memory leaks. QFuture's ref-counted data can only be deleted when the
last copy referencing the data gets deleted. The saved continuation
that keeps a copy of the future (as in case of the lambda capture) will
prevent the data from being deleted. So we need to manually clean the
continuation after it is run. But this doesn't solve the problem if the
continuation isn't run. In that case, clean the continuation in the
destructor of the associated QPromise.

To avoid similar leaks, internally we should always create futures via
QPromise, instead of the ref-counted QFutureInterface, so that the
continuation is always cleaned in the destructor. Currently QFuture
continuations and QtFuture::when* methods use QFutureInterface directly,
which will be fixed by the follow-up commits.

Fixes: QTBUG-99534
Pick-to: 6.3 6.2
Change-Id: Ic13e7dffd8cb25bd6b87e5416fe4d1a97af74c9b
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
---
 src/corelib/thread/qfutureinterface.cpp | 15 ++++++++++++++-
 src/corelib/thread/qfutureinterface.h   |  4 ++++
 src/corelib/thread/qpromise.h           |  8 ++++++--
 3 files changed, 24 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/corelib/thread/qfutureinterface.cpp b/src/corelib/thread/qfutureinterface.cpp
index f29f950a1f..dfc905e828 100644
--- a/src/corelib/thread/qfutureinterface.cpp
+++ b/src/corelib/thread/qfutureinterface.cpp
@@ -858,12 +858,25 @@ void QFutureInterfaceBase::setContinuation(std::function<void(const QFutureInter
     }
 }
 
+void QFutureInterfaceBase::cleanContinuation()
+{
+    if (!d)
+        return;
+
+    // This is called when the associated QPromise is being destroyed.
+    // Clear the continuation, to make sure it doesn't keep any ref-counted
+    // copies of this, so that the allocated memory can be freed.
+    QMutexLocker lock(&d->continuationMutex);
+    d->continuation = nullptr;
+}
+
 void QFutureInterfaceBase::runContinuation() const
 {
     QMutexLocker lock(&d->continuationMutex);
     if (d->continuation) {
+        auto fn = std::exchange(d->continuation, nullptr);
         lock.unlock();
-        d->continuation(*this);
+        fn(*this);
     }
 }
 
diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h
index 3130c91420..205b2de9f9 100644
--- a/src/corelib/thread/qfutureinterface.h
+++ b/src/corelib/thread/qfutureinterface.h
@@ -212,10 +212,14 @@ private:
     friend class QtPrivate::FailureHandler;
 #endif
 
+    template<class T>
+    friend class QPromise;
+
 protected:
     void setContinuation(std::function<void(const QFutureInterfaceBase &)> func);
     void setContinuation(std::function<void(const QFutureInterfaceBase &)> func,
                          QFutureInterfaceBasePrivate *continuationFutureData);
+    void cleanContinuation();
     void runContinuation() const;
 
     void setLaunchAsync(bool value);
diff --git a/src/corelib/thread/qpromise.h b/src/corelib/thread/qpromise.h
index a970f40d12..f62a2a54e3 100644
--- a/src/corelib/thread/qpromise.h
+++ b/src/corelib/thread/qpromise.h
@@ -66,12 +66,16 @@ public:
     {
         const int state = d.loadState();
         // If QFutureInterface has no state, there is nothing to be done
-        if (state == static_cast<int>(QFutureInterfaceBase::State::NoState))
+        if (state == static_cast<int>(QFutureInterfaceBase::State::NoState)) {
+            d.cleanContinuation();
             return;
+        }
         // Otherwise, if computation is not finished at this point, cancel
         // potential waits
-        if (!(state & QFutureInterfaceBase::State::Finished))
+        if (!(state & QFutureInterfaceBase::State::Finished)) {
             d.cancelAndFinish(); // cancel and finalize the state
+            d.cleanContinuation();
+        }
     }
 
     // Core QPromise APIs
-- 
cgit v1.2.3