From 5938379e910c4cbd7ed2429901bb3c45e62405d4 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 26 Jul 2017 16:09:09 -0700 Subject: init: shutdown services in the opposite order that they started Currently, the order that we kill to services during shutdown is the order of services_ in ServiceManager and that is defacto the order in which they were parsed, which is not a very useful ordering. Related to this, we have seen a few issues during shutdown that may be related to services with dependencies on other services, where the dependency is killed first and the dependent service then misbehaves. This change allows services to keep track of the order in which they were started and shutdown then uses that information to kill running services in the opposite order that they were started. Bug: 64067984 Test: Boot and reboot bullhead Change-Id: I6b4cacb03aed2a72ae98a346bce41ed5434a09c2 --- init/reboot.cpp | 6 +++--- init/service.cpp | 18 ++++++++++++++++++ init/service.h | 6 ++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/init/reboot.cpp b/init/reboot.cpp index 17e3576a7..ce814830b 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -398,7 +398,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re LOG(INFO) << "terminating init services"; // Ask all services to terminate except shutdown critical ones. - ServiceManager::GetInstance().ForEachService([](Service* s) { + ServiceManager::GetInstance().ForEachServiceShutdownOrder([](Service* s) { if (!s->IsShutdownCritical()) s->Terminate(); }); @@ -434,7 +434,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re // minimum safety steps before restarting // 2. kill all services except ones that are necessary for the shutdown sequence. - ServiceManager::GetInstance().ForEachService([](Service* s) { + ServiceManager::GetInstance().ForEachServiceShutdownOrder([](Service* s) { if (!s->IsShutdownCritical()) s->Stop(); }); ServiceManager::GetInstance().ReapAnyOutstandingChildren(); @@ -448,7 +448,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re LOG(INFO) << "vold not running, skipping vold shutdown"; } // logcat stopped here - ServiceManager::GetInstance().ForEachService([&kill_after_apps](Service* s) { + ServiceManager::GetInstance().ForEachServiceShutdownOrder([&kill_after_apps](Service* s) { if (kill_after_apps.count(s->name())) s->Stop(); }); // 4. sync, try umount, and optionally run fsck for user shutdown diff --git a/init/service.cpp b/init/service.cpp index fc64db69a..14c2846d5 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -155,6 +155,8 @@ ServiceEnvironmentInfo::ServiceEnvironmentInfo(const std::string& name, : name(name), value(value) { } +unsigned long Service::next_start_order_ = 1; + Service::Service(const std::string& name, const std::vector& args) : Service(name, 0, 0, 0, {}, 0, 0, "", args) {} @@ -182,6 +184,7 @@ Service::Service(const std::string& name, unsigned flags, uid_t uid, gid_t gid, swappiness_(-1), soft_limit_in_bytes_(-1), limit_in_bytes_(-1), + start_order_(0), args_(args) { onrestart_.InitSingleTrigger("onrestart"); } @@ -283,6 +286,7 @@ void Service::Reap() { pid_ = 0; flags_ &= (~SVC_RUNNING); + start_order_ = 0; // Oneshot processes go into the disabled state on exit, // except when manually restarted. @@ -805,6 +809,7 @@ bool Service::Start() { time_started_ = boot_clock::now(); pid_ = pid; flags_ |= SVC_RUNNING; + start_order_ = next_start_order_++; process_cgroup_empty_ = false; errno = -createProcessGroup(uid_, pid_); @@ -1096,6 +1101,19 @@ void ServiceManager::ForEachService(const std::function& callbac } } +// Shutdown services in the opposite order that they were started. +void ServiceManager::ForEachServiceShutdownOrder(const std::function& callback) const { + std::vector shutdown_services; + for (const auto& service : services_) { + if (service->start_order() > 0) shutdown_services.emplace_back(service.get()); + } + std::sort(shutdown_services.begin(), shutdown_services.end(), + [](const auto& a, const auto& b) { return a->start_order() > b->start_order(); }); + for (const auto& service : shutdown_services) { + callback(service); + } +} + void ServiceManager::ForEachServiceInClass(const std::string& classname, void (*func)(Service* svc)) const { for (const auto& s : services_) { diff --git a/init/service.h b/init/service.h index 62a3299e1..a0dc1b475 100644 --- a/init/service.h +++ b/init/service.h @@ -108,6 +108,7 @@ class Service { int priority() const { return priority_; } int oom_score_adjust() const { return oom_score_adjust_; } bool process_cgroup_empty() const { return process_cgroup_empty_; } + unsigned long start_order() const { return start_order_; } const std::vector& args() const { return args_; } private: @@ -149,6 +150,8 @@ class Service { template bool AddDescriptor(const std::vector& args, std::string* err); + static unsigned long next_start_order_; + std::string name_; std::set classnames_; std::string console_; @@ -190,6 +193,8 @@ class Service { bool process_cgroup_empty_ = false; + unsigned long start_order_; + std::vector args_; }; @@ -209,6 +214,7 @@ class ServiceManager { Service* FindServiceByPid(pid_t pid) const; Service* FindServiceByKeychord(int keychord_id) const; void ForEachService(const std::function& callback) const; + void ForEachServiceShutdownOrder(const std::function& callback) const; void ForEachServiceInClass(const std::string& classname, void (*func)(Service* svc)) const; void ForEachServiceWithFlags(unsigned matchflags, -- cgit v1.2.3