From 0dbfea7b0778bff25a43602e523deae5bb65b114 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 11 Oct 2019 13:18:44 -0700 Subject: [PATCH] init: trigger shutdown directly from builtins Especially now that property_service is a thread, there may be some delay between when init sets sys.powerctl and when the main thread of init receives this and triggers shutdown. It's possible that outstanding init commands are run during this gap and that is not desirable. Instead, have builtins call TriggerShutdown() directly, so we can be sure that the next action that init runs will be to shutdown the device. Test: reboot works Test: reboot into recovery due to bad /data works Change-Id: I26fb9f4f57f46c7451b8b58187138cfedd6fd9eb --- init/builtins.cpp | 9 ++++++++- init/host_init_stubs.h | 2 +- init/init.cpp | 4 ++-- init/init.h | 2 +- init/reboot.cpp | 2 +- init/service.cpp | 6 +++--- 6 files changed, 16 insertions(+), 9 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 42211b2c1..2f2ead006 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -138,7 +138,14 @@ static Result reboot_into_recovery(const std::vector& options if (!write_bootloader_message(options, &err)) { return Error() << "Failed to set bootloader message: " << err; } - property_set("sys.powerctl", "reboot,recovery"); + // This function should only be reached from init and not from vendor_init, and we want to + // immediately trigger reboot instead of relaying through property_service. Older devices may + // still have paths that reach here from vendor_init, so we keep the property_set as a fallback. + if (getpid() == 1) { + TriggerShutdown("reboot,recovery"); + } else { + property_set("sys.powerctl", "reboot,recovery"); + } return {}; } diff --git a/init/host_init_stubs.h b/init/host_init_stubs.h index 71f78a558..5dd5cf1a4 100644 --- a/init/host_init_stubs.h +++ b/init/host_init_stubs.h @@ -35,7 +35,7 @@ namespace android { namespace init { // init.h -inline void EnterShutdown(const std::string&) { +inline void TriggerShutdown(const std::string&) { abort(); } diff --git a/init/init.cpp b/init/init.cpp index ab6dbcf3c..f775d8fa0 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -179,7 +179,7 @@ void ResetWaitForProp() { waiting_for_prop.reset(); } -void EnterShutdown(const std::string& command) { +void TriggerShutdown(const std::string& command) { // We can't call HandlePowerctlMessage() directly in this function, // because it modifies the contents of the action queue, which can cause the action queue // to get into a bad state if this function is called from a command being executed by the @@ -197,7 +197,7 @@ void property_changed(const std::string& name, const std::string& value) { // In non-thermal-shutdown case, 'shutdown' trigger will be fired to let device specific // commands to be executed. if (name == "sys.powerctl") { - EnterShutdown(value); + TriggerShutdown(value); } if (property_triggers_enabled) ActionManager::GetInstance().QueuePropertyChange(name, value); diff --git a/init/init.h b/init/init.h index c7918e7e7..d884a9447 100644 --- a/init/init.h +++ b/init/init.h @@ -31,7 +31,7 @@ namespace init { Parser CreateParser(ActionManager& action_manager, ServiceList& service_list); Parser CreateServiceOnlyParser(ServiceList& service_list); -void EnterShutdown(const std::string& command); +void TriggerShutdown(const std::string& command); bool start_waiting_for_property(const char *name, const char *value); diff --git a/init/reboot.cpp b/init/reboot.cpp index 4b892b775..d77b9757e 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -710,7 +710,7 @@ static Result DoUserspaceReboot() { auto guard = android::base::make_scope_guard([] { // Leave shutdown so that we can handle a full reboot. LeaveShutdown(); - property_set("sys.powerctl", "reboot,abort-userspace-reboot"); + TriggerShutdown("reboot,abort-userspace-reboot"); }); // Triggering userspace-reboot-requested will result in a bunch of set_prop // actions. We should make sure, that all of them are propagated before diff --git a/init/service.cpp b/init/service.cpp index a2db07056..c8568a0e9 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -255,7 +255,7 @@ void Service::Reap(const siginfo_t& siginfo) { if ((siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) && on_failure_reboot_target_) { LOG(ERROR) << "Service with 'reboot_on_failure' option failed, shutting down system."; - EnterShutdown(*on_failure_reboot_target_); + TriggerShutdown(*on_failure_reboot_target_); } if (flags_ & SVC_EXEC) UnSetExec(); @@ -335,7 +335,7 @@ void Service::DumpState() const { Result Service::ExecStart() { auto reboot_on_failure = make_scope_guard([this] { if (on_failure_reboot_target_) { - EnterShutdown(*on_failure_reboot_target_); + TriggerShutdown(*on_failure_reboot_target_); } }); @@ -366,7 +366,7 @@ Result Service::ExecStart() { Result Service::Start() { auto reboot_on_failure = make_scope_guard([this] { if (on_failure_reboot_target_) { - EnterShutdown(*on_failure_reboot_target_); + TriggerShutdown(*on_failure_reboot_target_); } });