Combinator: Changed to use the ref count system to manage the life cy…

…cle of CombinedFuture
benlau · Mar 20, 2017 · 241dd78 · 241dd78
1 parent e9de1b4
commit 241dd78
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 10 deletions.
diff --git a/asyncfuture.h b/asyncfuture.h
@@ -124,7 +124,7 @@ class DeferredFuture : public QObject, public QFutureInterface<T>{
     }
 
     void complete(QFuture<T> future) {
-        addRefCount();
+        incRefCount();
         auto onFinished = [=]() {
             Value<T> value(future);
             complete(value);
@@ -167,7 +167,7 @@ class DeferredFuture : public QObject, public QFutureInterface<T>{
 
     template <typename ANY>
     void cancel(QFuture<ANY> future) {
-        addRefCount();
+        incRefCount();
         auto onFinished = [=]() {
             cancel();
             decRefCount();
@@ -183,7 +183,7 @@ class DeferredFuture : public QObject, public QFutureInterface<T>{
               onCanceled);
     }
 
-    void addRefCount() {
+    void incRefCount() {
         refCount++;
     }
 
@@ -240,10 +240,8 @@ class DeferredFuture : public QObject, public QFutureInterface<T>{
 
 };
 
-
-
 // Obtain the result of future in QVariant.
-// It is used within Combinator only as other component do
+// It is used within Combinator only as other component do not
 // require to register QMetaType
 template <typename T>
 inline QVariant obtainFutureResult(QFuture<T> future) {
@@ -266,14 +264,20 @@ class CombinedFuture: public DeferredFuture<QVariantList> {
 
     template <typename T>
     void addFuture(const QFuture<T> future) {
+        if (resolved) {
+            return;
+        }
         int index = count++;
         results << QVariant();
+        incRefCount();
 
         Private::watch(future, this,
                        [=]() {
             completeFutureAt(index, future);
+            decRefCount();
         },[=]() {
             cancelFutureAt(index);
+            decRefCount();
         });
     }
 
@@ -283,6 +287,22 @@ class CombinedFuture: public DeferredFuture<QVariantList> {
     bool settleAllMode;
     QVariantList results;
 
+    static QSharedPointer<CombinedFuture> create(bool settleAllMode) {
+
+        auto deleter = [](CombinedFuture *object) {
+            if (object->resolved) {
+                // If that is already resolved, it is not necessary to keep it in memory
+                object->deleteLater();
+            } else {
+                object->autoDelete = true;
+                object->decRefCount();
+            }
+        };
+
+        QSharedPointer<CombinedFuture> ptr(new CombinedFuture(settleAllMode), deleter);
+        return ptr;
+    }
+
 private:
 
     template <typename T>
@@ -640,12 +660,11 @@ class Deferred<void> : public Observable<void> {
 
 class Combinator : public Observable<QVariantList> {
 private:
-    QPointer<Private::CombinedFuture> combinedFuture;
+    QSharedPointer<Private::CombinedFuture> combinedFuture;
 
 public:
     inline Combinator(bool settleAllMode = false) : Observable<QVariantList>() {
-        combinedFuture = new Private::CombinedFuture(settleAllMode);
-        combinedFuture->autoDelete = true;
+        combinedFuture = Private::CombinedFuture::create(settleAllMode);
         m_future = combinedFuture->future();
     }
 
@@ -667,7 +686,6 @@ class Combinator : public Observable<QVariantList> {
         combinedFuture->addFuture(future);
         return *this;
     }
-
 };
 
 template <typename T>

diff --git a/tests/asyncfutureunittests/asyncfuturetests.cpp b/tests/asyncfutureunittests/asyncfuturetests.cpp
@@ -58,6 +58,21 @@ void AsyncFutureTests::test_QFuture_cancel()
     QCOMPARE(future.isCanceled(), true);
 }
 
+void AsyncFutureTests::test_QFuture_isResultReadyAt()
+{
+    auto defer = deferred<int>();
+    auto future = defer.future();
+
+    QVERIFY(!future.isResultReadyAt(0));
+    defer.complete(10);
+    QVERIFY(future.isResultReadyAt(0));
+    QVERIFY(!future.isResultReadyAt(1));
+    QVERIFY(future.results().size() == 1);
+    QVERIFY(future.result() == 10);
+    QVERIFY(future.results()[0] == 10);
+
+}
+
 void AsyncFutureTests::test_private_DeferredFuture()
 {
     Private::DeferredFuture<void> defer;
@@ -771,3 +786,39 @@ void AsyncFutureTests::test_Combinator()
     }
 
 }
+
+void AsyncFutureTests::test_Combinator_add_to_already_finished()
+{
+    {
+        // case: combine(true), cancel
+        auto d1 = deferred<int>();
+        auto d2 = deferred<QString>();
+        auto d3 = deferred<void>();
+        auto d4 = deferred<bool>();
+
+        Combinator copy;
+
+        {
+            auto combinator = combine();
+            copy = combinator;
+
+            combinator << d1.future() << d2.future() << d3.future();
+
+            d1.complete(1);
+            d2.complete("second");
+            d3.complete();
+
+            QVERIFY(waitUntil(combinator.future(), 1000));
+        }
+
+        copy << d4.future();
+        d4.complete(true);
+
+        QVERIFY(waitUntil(copy.future(), 1000)); // It is already resolved
+        QVERIFY(copy.future().isResultReadyAt(0));
+        QVariantList result = copy.future().result();
+        QCOMPARE(result.size(), 3);
+
+    }
+
+}
diff --git a/tests/asyncfutureunittests/asyncfuturetests.h b/tests/asyncfutureunittests/asyncfuturetests.h
@@ -26,6 +26,8 @@ public slots:
 private slots:
     void test_QFuture_cancel();
 
+    void test_QFuture_isResultReadyAt();
+
     void test_private_DeferredFuture();
 
     void test_private_run();
@@ -49,4 +51,6 @@ private slots:
 
 
     void test_Combinator();
+
+    void test_Combinator_add_to_already_finished();
 };