init: handle property service callbacks asynchronously
A previous change moved property_service into its own thread, since
there was otherwise a deadlock whenever a process called by init would
try to set a property. This new thread, however, would send a message
via a blocking socket to init for each property that it received,
since init may need to take action depending on which property it is.
Unfortunately, this means that the deadlock is still possible, the
only difference is the socket's buffer must be filled before init deadlocks.
There are possible partial solutions here: the socket's buffer may be
increased or property_service may only send messages for the
properties that init will take action on, however all of these
solutions still lead to eventual deadlock. The only complete solution
is to handle these messages asynchronously.
This change, therefore, adds the following:
1) A lock for instructing init to reboot
2) A lock for waiting on properties
3) A lock for queueing new properties
4) A lock for any actions with ServiceList or any Services, enforced
through thread annotations, particularly since this code was not
designed with the intention of being multi-threaded.
Bug: 146877356
Bug: 148236233
Test: boot
Test: kill hwservicemanager without deadlock
Change-Id: I84108e54217866205a48c45e8b59355012c32ea8
diff --git a/init/reboot.cpp b/init/reboot.cpp
index 048c1e7..f7cc36e 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -85,7 +85,7 @@
static const std::set<std::string> kDebuggingServices{"tombstoned", "logd", "adbd", "console"};
-static std::vector<Service*> GetDebuggingServices(bool only_post_data) {
+static std::vector<Service*> GetDebuggingServices(bool only_post_data) REQUIRES(service_lock) {
std::vector<Service*> ret;
ret.reserve(kDebuggingServices.size());
for (const auto& s : ServiceList::GetInstance()) {
@@ -179,7 +179,7 @@
};
// Turn off backlight while we are performing power down cleanup activities.
-static void TurnOffBacklight() {
+static void TurnOffBacklight() REQUIRES(service_lock) {
Service* service = ServiceList::GetInstance().FindService("blank_screen");
if (service == nullptr) {
LOG(WARNING) << "cannot find blank_screen in TurnOffBacklight";
@@ -587,6 +587,7 @@
// Start reboot monitor thread
sem_post(&reboot_semaphore);
+ auto lock = std::lock_guard{service_lock};
// watchdogd is a vendor specific component but should be alive to complete shutdown safely.
const std::set<std::string> to_starts{"watchdogd"};
std::vector<Service*> stop_first;
@@ -706,6 +707,7 @@
// Skip wait for prop if it is in progress
ResetWaitForProp();
// Clear EXEC flag if there is one pending
+ auto lock = std::lock_guard{service_lock};
for (const auto& s : ServiceList::GetInstance()) {
s->UnSetExec();
}
@@ -749,6 +751,7 @@
return Error() << "Failed to set sys.init.userspace_reboot.in_progress property";
}
EnterShutdown();
+ auto lock = std::lock_guard{service_lock};
if (!SetProperty("sys.powerctl", "")) {
return Error() << "Failed to reset sys.powerctl property";
}
@@ -907,6 +910,7 @@
run_fsck = true;
} else if (cmd_params[1] == "thermal") {
// Turn off sources of heat immediately.
+ auto lock = std::lock_guard{service_lock};
TurnOffBacklight();
// run_fsck is false to avoid delay
cmd = ANDROID_RB_THERMOFF;