commit 0daad6bc93bd156fe1e7553eeb54c21c5ef5d406
author Alexander Hendrich <hendrich@chromium.org> Tue Nov 06 09:48:52 2018 +0100
committer ChromeOS Commit Bot <chromeos-commit-bot@chromium.org> Thu Nov 08 08:16:55 2018 +0000
tree ef232731c77729e46349141f0f1a656d29532f51
parent e1a1b12dc10fa879935b0f7455662946b7399ef5

[Merge M70] chaps: Fix crash in ~SystemShutdownBlocker

Fix crash caused by iterating and erasing over |blocked_slots_| in
~SystemShutdownBlocker.

BUG=chromium:902177
TEST=none

Change-Id: Ia4e7b77596e66a75991e544f1ca9dfcfa09d0227
Reviewed-on: https://chromium-review.googlesource.com/1319094
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
(cherry picked from commit 586c307cf05f0085abfe1bbeed8ba12c5b9c73e2)
Reviewed-on: https://chromium-review.googlesource.com/c/1325842
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>

chaps/system_shutdown_blocker.cc

diff --git a/chaps/system_shutdown_blocker.cc b/chaps/system_shutdown_blocker.cc
index 3a2f819..23e6891 100644
--- a/chaps/system_shutdown_blocker.cc
+++ b/chaps/system_shutdown_blocker.cc
@@ -21,6 +21,7 @@
 SystemShutdownBlocker::~SystemShutdownBlocker() {
   for (int slot_id : blocked_slots_)
     PerformUnblock(slot_id);
+  blocked_slots_.clear();
 }
 
 void SystemShutdownBlocker::Block(int slot_id,
@@ -32,18 +33,20 @@
   origin_thread_task_runner_->PostTask(FROM_HERE, perform_block_task);
 
   // Post delayed unblock task (fallback).
-  auto perform_unblock_task = base::Bind(&SystemShutdownBlocker::PerformUnblock,
-                                         base::Unretained(this),
-                                         slot_id);
+  auto perform_unblock_task =
+      base::Bind(&SystemShutdownBlocker::PerformUnblockIfBlocked,
+                 base::Unretained(this),
+                 slot_id);
   origin_thread_task_runner_->PostDelayedTask(FROM_HERE, perform_unblock_task,
                                               fallback_timeout);
 }
 
 void SystemShutdownBlocker::Unblock(int slot_id) {
   // Post unblock task.
-  auto perform_unblock_task = base::Bind(&SystemShutdownBlocker::PerformUnblock,
-                                         base::Unretained(this),
-                                         slot_id);
+  auto perform_unblock_task =
+      base::Bind(&SystemShutdownBlocker::PerformUnblockIfBlocked,
+                 base::Unretained(this),
+                 slot_id);
   origin_thread_task_runner_->PostTask(FROM_HERE, perform_unblock_task);
 }
 
@@ -74,22 +77,24 @@
   LOG(INFO) << "Created lock file: " << lock_path.value();
 }
 
-void SystemShutdownBlocker::PerformUnblock(int slot_id) {
-  if (blocked_slots_.find(slot_id) == blocked_slots_.end())
-    return;
+void SystemShutdownBlocker::PerformUnblockIfBlocked(int slot_id) {
+  if (blocked_slots_.count(slot_id) && PerformUnblock(slot_id))
+    blocked_slots_.erase(slot_id);
+}
 
+bool SystemShutdownBlocker::PerformUnblock(int slot_id) {
   const base::FilePath lock_path = GetPowerdLockFilePath(slot_id);
   if (!base::PathExists(lock_path)) {
     LOG(WARNING) << "Couldn't delete lock file (not existant): "
                  << lock_path.value();
-    return;
+    return true;
   }
   if (!base::DeleteFile(lock_path, false /* recursive */)) {
     PLOG(ERROR) << "Couldn't delete lock file: " << lock_path.value();
-    return;
+    return false;
   }
-  blocked_slots_.erase(slot_id);
   LOG(INFO) << "Deleted lock file: " << lock_path.value();
+  return true;
 }
 
 base::FilePath SystemShutdownBlocker::GetPowerdLockFilePath(int slot_id) const {

chaps/system_shutdown_blocker.h

diff --git a/chaps/system_shutdown_blocker.h b/chaps/system_shutdown_blocker.h
index e3b5173..df2171b 100644
--- a/chaps/system_shutdown_blocker.h
+++ b/chaps/system_shutdown_blocker.h
@@ -33,15 +33,19 @@
   // |origin_thread_task_runner_|.
   void Unblock(int slot_id);
 
+ private:
   // Creates a lock file readable by powerd with chapsd's PID and marks the slot
   // as blocked in |blocked_slots|.
   void PerformBlock(int slot_id);
 
-  // Deletes the lock file if the slot is marked as blocked in |blocked_slots_|,
-  // otherwise no-op.
-  void PerformUnblock(int slot_id);
+  // Calls |PerformUnblock()| and removes the block mark if the slot is marked
+  // as blocked in |blocked_slots_|, otherwise no-op.
+  void PerformUnblockIfBlocked(int slot_id);
 
- private:
+  // Deletes the lock file and returns whether or not the slot is unblocked
+  // (i.e. lock file removed).
+  bool PerformUnblock(int slot_id);
+
   // Gets the corresponding lock file path for |slot_id|
   // (/run/lock/power_override/chapsd_token_init_slot_<<<SLOT_ID>>>.lock).
   base::FilePath GetPowerdLockFilePath(int slot_id) const;