From 34b041f4102976e32de6ccc1570a31801dbfcb4a Mon Sep 17 00:00:00 2001
From: Rhed Jao <rhedjao@google.com>
Date: Wed, 17 Mar 2021 18:53:39 +0800
Subject: [PATCH] Fixes dumpstate crashed when shutdown the thread pool

The future of the packaged_task should be cleared before we
clear the packaged_task in the task queue.

Bug: 182798355
Test: atest DumpPoolTest#ShutdownBeforeTasksFinished_withoutCrash
Change-Id: I85e6aca67c61629d786f0586c6fe3d714f400233
---
 cmds/dumpstate/DumpPool.cpp             |  2 +-
 cmds/dumpstate/tests/dumpstate_test.cpp | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/cmds/dumpstate/DumpPool.cpp b/cmds/dumpstate/DumpPool.cpp
index e15ac3fe82..c2c8a72f52 100644
--- a/cmds/dumpstate/DumpPool.cpp
+++ b/cmds/dumpstate/DumpPool.cpp
@@ -64,8 +64,8 @@ void DumpPool::shutdown() {
     if (shutdown_ || threads_.empty()) {
         return;
     }
-    while (!tasks_.empty()) tasks_.pop();
     futures_map_.clear();
+    while (!tasks_.empty()) tasks_.pop();
 
     shutdown_ = true;
     condition_variable_.notify_all();
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index b2518ad9eb..2c573e4f4a 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -1762,6 +1762,27 @@ TEST_F(DumpPoolTest, EnqueueTask_withDurationLog) {
     EXPECT_THAT(getTempFileCounts(kTestDataPath), Eq(0));
 }
 
+TEST_F(DumpPoolTest, Shutdown_withoutCrash) {
+    bool run_1 = false;
+    auto dump_func_1 = [&]() {
+        run_1 = true;
+    };
+    auto dump_func = []() {
+        sleep(1);
+    };
+
+    dump_pool_->start(/* thread_counts = */1);
+    dump_pool_->enqueueTask(/* task_name = */"1", dump_func_1);
+    dump_pool_->enqueueTask(/* task_name = */"2", dump_func);
+    dump_pool_->enqueueTask(/* task_name = */"3", dump_func);
+    dump_pool_->enqueueTask(/* task_name = */"4", dump_func);
+    dump_pool_->waitForTask("1", "", out_fd_.get());
+    dump_pool_->shutdown();
+
+    EXPECT_TRUE(run_1);
+    EXPECT_THAT(getTempFileCounts(kTestDataPath), Eq(0));
+}
+
 class TaskQueueTest : public DumpstateBaseTest {
 public:
     void SetUp() {