From 0f296e06d6a4c26e7886b98964057ddbc9070c6e Mon Sep 17 00:00:00 2001
From: Tom Cherry <tomcherry@google.com>
Date: Fri, 30 Jun 2017 12:58:39 -0700
Subject: [PATCH] ueventd: don't double fork firmware handlers

ueventd may be asked to handle firmware during the time critical
coldboot process.  If we double fork to avoid needing to reap the
firmware handler, then we may add significant delay to this process,
as the first child may not get scheduled quickly enough for waitpid()
to complete without delay.

Bug: 63081260
Test: boot bullhead and sailfish, check that firmwares are loaded,
      no zombie ueventd processes remain, and no new errors are shown
Change-Id: I2bac3b1fbc3a58557a00326e491c104656db27ae
---
 init/firmware_handler.cpp | 23 ++++-------------------
 init/ueventd.cpp          |  7 +++++++
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/init/firmware_handler.cpp b/init/firmware_handler.cpp
index 8cd5cc5f9..860b30bc2 100644
--- a/init/firmware_handler.cpp
+++ b/init/firmware_handler.cpp
@@ -110,31 +110,16 @@ void HandleFirmwareEvent(const Uevent& uevent) {
     if (uevent.subsystem != "firmware" || uevent.action != "add") return;
 
     // Loading the firmware in a child means we can do that in parallel...
-    // We double fork instead of waiting for these processes.
-    pid_t pid = fork();
+    auto pid = fork();
     if (pid == -1) {
         PLOG(ERROR) << "could not fork to process firmware event for " << uevent.firmware;
-        return;
     }
-
     if (pid == 0) {
-        pid = fork();
-        if (pid == -1) {
-            PLOG(ERROR) << "could not fork a sceond time to process firmware event for "
-                        << uevent.firmware;
-            _exit(EXIT_FAILURE);
-        }
-        if (pid == 0) {
-            Timer t;
-            ProcessFirmwareEvent(uevent);
-            LOG(INFO) << "loading " << uevent.path << " took " << t;
-            _exit(EXIT_SUCCESS);
-        }
-
+        Timer t;
+        ProcessFirmwareEvent(uevent);
+        LOG(INFO) << "loading " << uevent.path << " took " << t;
         _exit(EXIT_SUCCESS);
     }
-
-    waitpid(pid, nullptr, 0);
 }
 
 }  // namespace init
diff --git a/init/ueventd.cpp b/init/ueventd.cpp
index 81a0572cb..a713beb0a 100644
--- a/init/ueventd.cpp
+++ b/init/ueventd.cpp
@@ -268,6 +268,13 @@ int ueventd_main(int argc, char** argv) {
         cold_boot.Run();
     }
 
+    // We use waitpid() in ColdBoot, so we can't ignore SIGCHLD until now.
+    signal(SIGCHLD, SIG_IGN);
+    // Reap and pending children that exited between the last call to waitpid() and setting SIG_IGN
+    // for SIGCHLD above.
+    while (waitpid(-1, nullptr, WNOHANG) > 0) {
+    }
+
     uevent_listener.Poll([&device_handler](const Uevent& uevent) {
         HandleFirmwareEvent(uevent);
         device_handler.HandleDeviceEvent(uevent);