Merge "Fix lifecycle issue in CommonPool" into qt-dev

am: 562888399c

Change-Id: Ie82d2a9cc474e84358e799b963b82b7813b832c4

libs/hwui/tests/unit/CommonPoolTests.cpp

@@ -135,4 +135,127 @@ TEST(CommonPool, fullQueue) {
     for (auto& f : futures) {
         f.get();
     }
+}
+
+struct DestructorObserver {
+    DestructorObserver(int* destroyCount) : mDestroyCount(destroyCount) {}
+    DestructorObserver(const DestructorObserver& other) : mDestroyCount(other.mDestroyCount) {}
+    DestructorObserver(DestructorObserver&& other) {
+        mDestroyCount = other.mDestroyCount;
+        other.mDestroyCount = nullptr;
+    }
+    ~DestructorObserver() {
+        if (mDestroyCount) {
+            (*mDestroyCount)++;
+        }
+    }
+    DestructorObserver& operator=(DestructorObserver&& other) {
+        mDestroyCount = other.mDestroyCount;
+        other.mDestroyCount = nullptr;
+        return *this;
+    }
+    int* mDestroyCount;
+};
+
+struct CopyObserver {
+    CopyObserver(int* copyCount) : mCopyCount(copyCount) {}
+    CopyObserver(CopyObserver&& other) = default;
+    CopyObserver& operator=(CopyObserver&& other) = default;
+
+    CopyObserver(const CopyObserver& other) {
+        mCopyCount = other.mCopyCount;
+        if (mCopyCount) {
+            (*mCopyCount)++;
+        }
+    }
+
+    CopyObserver& operator=(const CopyObserver& other) {
+        mCopyCount = other.mCopyCount;
+        if (mCopyCount) {
+            (*mCopyCount)++;
+        }
+        return *this;
+    }
+
+    int* mCopyCount;
+};
+
+TEST(CommonPool, asyncLifecycleCheck) {
+    std::vector<std::future<void>> mFrameFences;
+    int destroyCount = 0;
+    int runCount = 0;
+    {
+        DestructorObserver observer{&destroyCount};
+        auto func = [observer = std::move(observer), count = &runCount]() {
+            if (observer.mDestroyCount) {
+                (*count)++;
+            }
+        };
+        mFrameFences.push_back(CommonPool::async(std::move(func)));
+    }
+    for (auto& fence : mFrameFences) {
+        EXPECT_TRUE(fence.valid());
+        fence.get();
+        EXPECT_FALSE(fence.valid());
+    }
+    mFrameFences.clear();
+    EXPECT_EQ(1, runCount);
+    EXPECT_EQ(1, destroyCount);
+}
+
+TEST(CommonPool, asyncCopyCheck) {
+    std::vector<std::future<void>> mFrameFences;
+    int copyCount = 0;
+    int runCount = 0;
+    {
+        CopyObserver observer{&copyCount};
+        auto func = [observer = std::move(observer), count = &runCount]() {
+            if (observer.mCopyCount) {
+                (*count)++;
+            }
+        };
+        mFrameFences.push_back(CommonPool::async(std::move(func)));
+    }
+    for (auto& fence : mFrameFences) {
+        EXPECT_TRUE(fence.valid());
+        fence.get();
+        EXPECT_FALSE(fence.valid());
+    }
+    mFrameFences.clear();
+    EXPECT_EQ(1, runCount);
+    // We expect std::move all the way
+    EXPECT_EQ(0, copyCount);
+}
+
+TEST(CommonPool, syncLifecycleCheck) {
+    int destroyCount = 0;
+    int runCount = 0;
+    {
+        DestructorObserver observer{&destroyCount};
+        auto func = [observer = std::move(observer), count = &runCount]() {
+            if (observer.mDestroyCount) {
+                (*count)++;
+            }
+        };
+        CommonPool::runSync(std::move(func));
+    }
+    EXPECT_EQ(1, runCount);
+    EXPECT_EQ(1, destroyCount);
+}
+
+TEST(CommonPool, syncCopyCheck) {
+    int copyCount = 0;
+    int runCount = 0;
+    {
+        CopyObserver observer{&copyCount};
+        auto func = [observer = std::move(observer), count = &runCount]() {
+            if (observer.mCopyCount) {
+                (*count)++;
+            }
+        };
+        CommonPool::runSync(std::move(func));
+    }
+    EXPECT_EQ(1, runCount);
+    // We expect std::move all the way
+    EXPECT_EQ(0, copyCount);
 }

libs/hwui/thread/CommonPool.h

@@ -57,11 +57,13 @@ public:
         mHead = newHead;
     }
 
-    constexpr T&& pop() {
+    constexpr T pop() {
         LOG_ALWAYS_FATAL_IF(mTail == mHead, "empty");
         int index = mTail;
         mTail = (mTail + 1) % SIZE;
-        return std::move(mBuffer[index]);
+        T ret = std::move(mBuffer[index]);
+        mBuffer[index] = nullptr;
+        return ret;
     }
 
 private: