From cfd929dffd2f9f0bdc6997f06309992000fbc289 Mon Sep 17 00:00:00 2001
From: John Reck <jreck@google.com>
Date: Mon, 8 Apr 2019 11:28:15 -0700
Subject: [PATCH] Revert "Revert "Fix lifecycle issue in CommonPool""

This reverts commit 162305aaceca5f6cbaa03db1aa124f67e313612e.

Adjusted tests to ensure they pass on cf_x86

Bug: 129250875
Test: this on cf_x86 & blueline

Change-Id: Ic3245ec8db784ae356b7fa66dda9a2fc91c622ea
---
 libs/hwui/tests/unit/CommonPoolTests.cpp | 44 ++++++++++++++++++++++++
 libs/hwui/thread/CommonPool.cpp          | 21 +++++++++--
 libs/hwui/thread/CommonPool.h            | 12 +++++--
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/libs/hwui/tests/unit/CommonPoolTests.cpp b/libs/hwui/tests/unit/CommonPoolTests.cpp
index c564ed6327869..70a5f5acbb6ee 100644
--- a/libs/hwui/tests/unit/CommonPoolTests.cpp
+++ b/libs/hwui/tests/unit/CommonPoolTests.cpp
@@ -135,4 +135,48 @@ TEST(CommonPool, fullQueue) {
     for (auto& f : futures) {
         f.get();
     }
+}
+
+class ObjectTracker {
+    static std::atomic_int sGlobalCount;
+
+public:
+    ObjectTracker() {
+        sGlobalCount++;
+    }
+    ObjectTracker(const ObjectTracker&) {
+        sGlobalCount++;
+    }
+    ObjectTracker(ObjectTracker&&) {
+        sGlobalCount++;
+    }
+    ~ObjectTracker() {
+        sGlobalCount--;
+    }
+
+    static int count() { return sGlobalCount.load(); }
+};
+
+std::atomic_int ObjectTracker::sGlobalCount{0};
+
+TEST(CommonPool, asyncLifecycleCheck) {
+    ASSERT_EQ(0, ObjectTracker::count());
+    {
+        ObjectTracker obj;
+        ASSERT_EQ(1, ObjectTracker::count());
+        EXPECT_LT(1, CommonPool::async([obj] { return ObjectTracker::count(); }).get());
+    }
+    CommonPool::waitForIdle();
+    ASSERT_EQ(0, ObjectTracker::count());
+}
+
+TEST(CommonPool, syncLifecycleCheck) {
+    ASSERT_EQ(0, ObjectTracker::count());
+    {
+        ObjectTracker obj;
+        ASSERT_EQ(1, ObjectTracker::count());
+        EXPECT_LT(1, CommonPool::runSync([obj] { return ObjectTracker::count(); }));
+    }
+    CommonPool::waitForIdle();
+    ASSERT_EQ(0, ObjectTracker::count());
 }
\ No newline at end of file
diff --git a/libs/hwui/thread/CommonPool.cpp b/libs/hwui/thread/CommonPool.cpp
index 7f94a152cf8d0..d011bdfe945ea 100644
--- a/libs/hwui/thread/CommonPool.cpp
+++ b/libs/hwui/thread/CommonPool.cpp
@@ -49,9 +49,13 @@ CommonPool::CommonPool() {
     }
 }
 
-void CommonPool::post(Task&& task) {
+CommonPool& CommonPool::instance() {
     static CommonPool pool;
-    pool.enqueue(std::move(task));
+    return pool;
+}
+
+void CommonPool::post(Task&& task) {
+    instance().enqueue(std::move(task));
 }
 
 void CommonPool::enqueue(Task&& task) {
@@ -86,5 +90,18 @@ void CommonPool::workerLoop() {
     }
 }
 
+void CommonPool::waitForIdle() {
+    instance().doWaitForIdle();
+}
+
+void CommonPool::doWaitForIdle() {
+    std::unique_lock lock(mLock);
+    while (mWaitingThreads != THREAD_COUNT) {
+        lock.unlock();
+        usleep(100);
+        lock.lock();
+    }
+}
+
 }  // namespace uirenderer
 }  // namespace android
\ No newline at end of file
diff --git a/libs/hwui/thread/CommonPool.h b/libs/hwui/thread/CommonPool.h
index aef2990d63434..7603eeef46928 100644
--- a/libs/hwui/thread/CommonPool.h
+++ b/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:
@@ -95,11 +97,17 @@ public:
         return task.get_future().get();
     };
 
+    // For testing purposes only, blocks until all worker threads are parked.
+    static void waitForIdle();
+
 private:
+    static CommonPool& instance();
+
     CommonPool();
     ~CommonPool() {}
 
     void enqueue(Task&&);
+    void doWaitForIdle();
 
     void workerLoop();