commit b1e5ee3dfd3a83b4c8cbaacc0d38d624ca949daa
author Kelvin Zhang <zhangkelvin@google.com> Sat Sep 30 00:59:21 2023 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Sat Sep 30 00:59:21 2023 +0000
tree 0ecd2fbbde54aa5cf5dcc7ba23248e3d25200880
parent 3b46b6732f9412cc66bd7b9eb0f13f4a5632f291
parent 1e926315eea85ec493028c167f569a53ddb40020

Wait for process to exit before unmounting postinstall partition am: 1e926315ee

Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/2767665

Change-Id: Ie00a4d46c8acbc53c97b7a2fd30c18a2e7585a87
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

common/subprocess.cc

diff --git a/common/subprocess.cc b/common/subprocess.cc
index a0cb9a7..9bd00d7 100644
--- a/common/subprocess.cc
+++ b/common/subprocess.cc
@@ -213,12 +213,21 @@
   pid_record->second->callback.Reset();
   // We don't care about output/return code, so we use SIGKILL here to ensure it
   // will be killed, SIGTERM might lead to leaked subprocess.
-  if (kill(pid, SIGKILL) != 0) {
-    PLOG(WARNING) << "Error sending SIGKILL to " << pid;
+  CHECK_EQ(pid_record->second->proc.pid(), pid);
+  if (!pid_record->second->proc.Kill(SIGKILL, 5)) {
+    PLOG(WARNING) << "Error sending SIGKILL to "
+                  << pid_record->second->proc.pid();
   }
   // Release the pid now so we don't try to kill it if Subprocess is destroyed
   // before the corresponding ChildExitedCallback() is called.
   pid_record->second->proc.Release();
+  if (subprocess_records_.count(pid)) {
+    siginfo_t info;
+    info.si_code = CLD_KILLED;
+    info.si_status = SIGKILL;
+    info.si_pid = pid;
+    ChildExitedCallback(info);
+  }
 }
 
 int Subprocess::GetPipeFd(pid_t pid, int fd) const {

common/subprocess_unittest.cc

diff --git a/common/subprocess_unittest.cc b/common/subprocess_unittest.cc
index ff4158e..8200e68 100644
--- a/common/subprocess_unittest.cc
+++ b/common/subprocess_unittest.cc
@@ -271,16 +271,20 @@
   watcher_ = base::FileDescriptorWatcher::WatchReadable(
       fifo_fd,
       base::Bind(
-          [](unique_ptr<base::FileDescriptorWatcher::Controller>* watcher,
+          [](brillo::BaseMessageLoop* loop,
+             unique_ptr<base::FileDescriptorWatcher::Controller>* watcher,
              int fifo_fd,
              uint32_t tag) {
-            char c;
+            char c{};
             EXPECT_EQ(1, HANDLE_EINTR(read(fifo_fd, &c, 1)));
             EXPECT_EQ('X', c);
             LOG(INFO) << "Killing tag " << tag;
             Subprocess::Get().KillExec(tag);
+            loop->BreakLoop();
+            ASSERT_TRUE(Subprocess::Get().subprocess_records_.empty());
             *watcher = nullptr;
           },
+          &loop_,
           // watcher_ is no longer used outside the clousure.
           base::Unretained(&watcher_),
           fifo_fd,