Fix memory leaks when capturing a QFuture in its continuation

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

@@ -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);
     }
 }
 

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);

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

tests/auto/corelib/thread/qfuture/tst_qfuture.cpp

@@ -194,6 +194,8 @@ private slots:
     void whenAnyDifferentTypesWithCanceled();
     void whenAnyDifferentTypesWithFailed();
 
+    void continuationsDontLeak();
+
 private:
     using size_type = std::vector<int>::size_type;
 
@@ -4355,5 +4357,65 @@ void tst_QFuture::whenAnyDifferentTypesWithFailed()
 #endif
 }
 
+struct InstanceCounter
+{
+    InstanceCounter() { ++count; }
+    InstanceCounter(const InstanceCounter &) { ++count; }
+    ~InstanceCounter() { --count; }
+    static int count;
+};
+int InstanceCounter::count = 0;
+
+void tst_QFuture::continuationsDontLeak()
+{
+    {
+        // QFuture isn't started and isn't finished (has no state)
+        QPromise<InstanceCounter> promise;
+        auto future = promise.future();
+
+        bool continuationIsRun = false;
+        future.then([future, &continuationIsRun](InstanceCounter) { continuationIsRun = true; });
+
+        promise.addResult(InstanceCounter {});
+
+        QVERIFY(!continuationIsRun);
+    }
+    QCOMPARE(InstanceCounter::count, 0);
+
+    {
+        // QFuture is started, but not finished
+        QPromise<InstanceCounter> promise;
+        auto future = promise.future();
+
+        bool continuationIsRun = false;
+        future.then([future, &continuationIsRun](InstanceCounter) { continuationIsRun = true; });
+
+        promise.start();
+        promise.addResult(InstanceCounter {});
+
+        QVERIFY(!continuationIsRun);
+    }
+    QCOMPARE(InstanceCounter::count, 0);
+
+    {
+        // QFuture is started and finished, the continuation is run
+        QPromise<InstanceCounter> promise;
+        auto future = promise.future();
+
+        bool continuationIsRun = false;
+        future.then([future, &continuationIsRun](InstanceCounter) {
+            QVERIFY(future.isFinished());
+            continuationIsRun = true;
+        });
+
+        promise.start();
+        promise.addResult(InstanceCounter {});
+        promise.finish();
+
+        QVERIFY(continuationIsRun);
+    }
+    QCOMPARE(InstanceCounter::count, 0);
+}
+
 QTEST_MAIN(tst_QFuture)
 #include "tst_qfuture.moc"