From d14a178d01fd1690cf8c9f69dd8672b29f946a10 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 22 Jul 2021 12:56:39 -0700 Subject: [PATCH] Revert "init: make reboot_on_failure not apply to manually stopped services" This reverts commit 1c51525f6686a97e32c166facad93fc97eacf0f9 because it accidentally made reboot_on_failure be a no-op for all services. This is because Reap() itself calls KillProcessGroup() on devices with a vendor level >= R, which in turn sets SVC_STOPPING. I had overlooked this somehow, probably because I didn't consider that a service can consist of multiple processes. It turns out that real FDE devices don't actually need the above commit because FDE devices aren't allowed to have updatable apexes enabled, and without updatable apexes enabled, apexd exits automatically and therefore doesn't have to be stopped. This can be verified by using the aosp_cf_x86_phone_noapex build target, rather than aosp_cf_x86_phone which I had used for testing before. So just revert it for now. Bug: 194370048 Change-Id: I90eddf2a87397449b241e5acaaa8d4a4241d73a9 --- init/README.md | 2 -- init/service.cpp | 10 +++------- init/service.h | 1 - 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/init/README.md b/init/README.md index 4b0462800..f447ab2a9 100644 --- a/init/README.md +++ b/init/README.md @@ -277,8 +277,6 @@ runs the service. CLD_EXITED or an status other than '0', reboot the system with the target specified in _target_. _target_ takes the same format as the parameter to sys.powerctl. This is particularly intended to be used with the `exec_start` builtin for any must-have checks during boot. - A service being stopped by init (e.g. using the `stop` or `class_reset` commands) is not - considered a failure for the purpose of this setting. `restart_period ` > If a non-oneshot service exits, it will be restarted at its start time plus diff --git a/init/service.cpp b/init/service.cpp index 5af81bf87..c3069f5b2 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -194,8 +194,6 @@ void Service::KillProcessGroup(int signal, bool report_oneshot) { << ") process group..."; int max_processes = 0; int r; - - flags_ |= SVC_STOPPING; if (signal == SIGTERM) { r = killProcessGroupOnce(proc_attr_.uid, pid_, signal, &max_processes); } else { @@ -279,8 +277,7 @@ void Service::Reap(const siginfo_t& siginfo) { f(siginfo); } - if ((siginfo.si_code != CLD_EXITED || siginfo.si_status != 0) && on_failure_reboot_target_ && - !(flags_ & SVC_STOPPING)) { + 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."; trigger_shutdown(*on_failure_reboot_target_); } @@ -290,7 +287,7 @@ void Service::Reap(const siginfo_t& siginfo) { if (flags_ & SVC_TEMPORARY) return; pid_ = 0; - flags_ &= ~(SVC_RUNNING | SVC_STOPPING); + flags_ &= (~SVC_RUNNING); start_order_ = 0; // Oneshot processes go into the disabled state on exit, @@ -414,8 +411,7 @@ Result Service::Start() { bool disabled = (flags_ & (SVC_DISABLED | SVC_RESET)); // Starting a service removes it from the disabled or reset state and // immediately takes it out of the restarting state if it was in there. - flags_ &= (~(SVC_DISABLED | SVC_RESTARTING | SVC_RESET | SVC_RESTART | SVC_DISABLED_START | - SVC_STOPPING)); + flags_ &= (~(SVC_DISABLED|SVC_RESTARTING|SVC_RESET|SVC_RESTART|SVC_DISABLED_START)); // Running processes require no additional work --- if they're in the // process of exiting, we've ensured that they will immediately restart diff --git a/init/service.h b/init/service.h index 89b1f0970..043555fa4 100644 --- a/init/service.h +++ b/init/service.h @@ -54,7 +54,6 @@ // should not be killed during shutdown #define SVC_TEMPORARY 0x1000 // This service was started by 'exec' and should be removed from the // service list once it is reaped. -#define SVC_STOPPING 0x2000 // service is being stopped by init #define NR_SVC_SUPP_GIDS 12 // twelve supplementary groups