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
diff --git a/init/firmware_handler.cpp b/init/firmware_handler.cpp
index 8cd5cc5..860b30b 100644
--- a/init/firmware_handler.cpp
+++ b/init/firmware_handler.cpp
@@ -110,31 +110,16 @@
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 81a0572..a713beb 100644
--- a/init/ueventd.cpp
+++ b/init/ueventd.cpp
@@ -268,6 +268,13 @@
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);