From 3b81f2d623c6cba45ad07fa91338d346d9b96482 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 28 Jul 2017 14:48:41 -0700 Subject: [PATCH 1/3] init: move exec operations out of ServiceManager These can be implemented without ServiceManager, so we remove them and make ServiceManager slightly less of a God class. Test: boot bullhead Test: init unit tests Change-Id: Ia6e546fe5292255412245256f7d230af4ece135f --- init/builtins.cpp | 23 ++++++++++- init/init.cpp | 4 +- init/reboot.cpp | 6 +-- init/service.cpp | 94 ++++++++++++------------------------------- init/service.h | 20 ++++----- init/service_test.cpp | 45 ++++++++++----------- 6 files changed, 81 insertions(+), 111 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index dec6f40e6..15594e687 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -170,11 +170,30 @@ static int do_enable(const std::vector& args) { } static int do_exec(const std::vector& args) { - return ServiceManager::GetInstance().Exec(args) ? 0 : -1; + auto service = Service::MakeTemporaryOneshotService(args); + if (!service) { + LOG(ERROR) << "Failed to create exec service: " << android::base::Join(args, " "); + return -1; + } + if (!service->ExecStart()) { + LOG(ERROR) << "Failed to Start exec service"; + return -1; + } + ServiceManager::GetInstance().AddService(std::move(service)); + return 0; } static int do_exec_start(const std::vector& args) { - return ServiceManager::GetInstance().ExecStart(args[1]) ? 0 : -1; + Service* service = ServiceManager::GetInstance().FindServiceByName(args[1]); + if (!service) { + LOG(ERROR) << "ExecStart(" << args[1] << "): Service not found"; + return -1; + } + if (!service->ExecStart()) { + LOG(ERROR) << "ExecStart(" << args[1] << "): Could not start Service"; + return -1; + } + return 0; } static int do_export(const std::vector& args) { diff --git a/init/init.cpp b/init/init.cpp index 03f0bb758..a12afae0a 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -1180,10 +1180,10 @@ int main(int argc, char** argv) { // By default, sleep until something happens. int epoll_timeout_ms = -1; - if (!(waiting_for_prop || sm.IsWaitingForExec())) { + if (!(waiting_for_prop || Service::is_exec_service_running())) { am.ExecuteOneCommand(); } - if (!(waiting_for_prop || sm.IsWaitingForExec())) { + if (!(waiting_for_prop || Service::is_exec_service_running())) { if (!shutting_down) { auto next_process_restart_time = RestartProcesses(); diff --git a/init/reboot.cpp b/init/reboot.cpp index ce814830b..6002040a1 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -524,10 +524,8 @@ bool HandlePowerctlMessage(const std::string& command) { // Skip wait for prop if it is in progress ResetWaitForProp(); - // Skip wait for exec if it is in progress - if (ServiceManager::GetInstance().IsWaitingForExec()) { - ServiceManager::GetInstance().ClearExecWait(); - } + // Clear EXEC flag if there is one pending + ServiceManager::GetInstance().ForEachService([](Service* s) { s->UnSetExec(); }); return true; } diff --git a/init/service.cpp b/init/service.cpp index d72433afa..7f7958483 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -156,6 +156,7 @@ ServiceEnvironmentInfo::ServiceEnvironmentInfo(const std::string& name, } unsigned long Service::next_start_order_ = 1; +bool Service::is_exec_service_running_ = false; Service::Service(const std::string& name, const std::vector& args) : Service(name, 0, 0, 0, {}, 0, 0, "", args) {} @@ -280,9 +281,9 @@ void Service::Reap() { std::for_each(descriptors_.begin(), descriptors_.end(), std::bind(&DescriptorInfo::Clean, std::placeholders::_1)); - if (flags_ & SVC_TEMPORARY) { - return; - } + if (flags_ & SVC_EXEC) UnSetExec(); + + if (flags_ & SVC_TEMPORARY) return; pid_ = 0; flags_ &= (~SVC_RUNNING); @@ -653,15 +654,20 @@ bool Service::ParseLine(const std::vector& args, std::string* err) return (this->*parser)(args, err); } -bool Service::ExecStart(std::unique_ptr* exec_waiter) { - flags_ |= SVC_EXEC | SVC_ONESHOT; - - exec_waiter->reset(new android::base::Timer); +bool Service::ExecStart() { + flags_ |= SVC_ONESHOT; if (!Start()) { - exec_waiter->reset(); return false; } + + flags_ |= SVC_EXEC; + is_exec_service_running_ = true; + + LOG(INFO) << "SVC_EXEC pid " << pid_ << " (uid " << uid_ << " gid " << gid_ << "+" + << supp_gids_.size() << " context " << (!seclabel_.empty() ? seclabel_ : "default") + << ") started; waiting..."; + return true; } @@ -836,12 +842,6 @@ bool Service::Start() { } } - if ((flags_ & SVC_EXEC) != 0) { - LOG(INFO) << "SVC_EXEC pid " << pid_ << " (uid " << uid_ << " gid " << gid_ << "+" - << supp_gids_.size() << " context " - << (!seclabel_.empty() ? seclabel_ : "default") << ") started; waiting..."; - } - NotifyStateChange("running"); return true; } @@ -935,8 +935,6 @@ void Service::OpenConsole() const { close(fd); } -int ServiceManager::exec_count_ = 0; - ServiceManager::ServiceManager() { } @@ -949,36 +947,7 @@ void ServiceManager::AddService(std::unique_ptr service) { services_.emplace_back(std::move(service)); } -bool ServiceManager::Exec(const std::vector& args) { - Service* svc = MakeExecOneshotService(args); - if (!svc) { - LOG(ERROR) << "Could not create exec service"; - return false; - } - if (!svc->ExecStart(&exec_waiter_)) { - LOG(ERROR) << "Could not start exec service"; - ServiceManager::GetInstance().RemoveService(*svc); - return false; - } - return true; -} - -bool ServiceManager::ExecStart(const std::string& name) { - Service* svc = FindServiceByName(name); - if (!svc) { - LOG(ERROR) << "ExecStart(" << name << "): Service not found"; - return false; - } - if (!svc->ExecStart(&exec_waiter_)) { - LOG(ERROR) << "ExecStart(" << name << "): Could not start Service"; - return false; - } - return true; -} - -bool ServiceManager::IsWaitingForExec() const { return exec_waiter_ != nullptr; } - -Service* ServiceManager::MakeExecOneshotService(const std::vector& args) { +std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector& args) { // Parse the arguments: exec [SECLABEL [UID [GID]*] --] COMMAND ARGS... // SECLABEL can be a - to denote default std::size_t command_arg = 1; @@ -999,10 +968,11 @@ Service* ServiceManager::MakeExecOneshotService(const std::vector& } std::vector str_args(args.begin() + command_arg, args.end()); - exec_count_++; - std::string name = "exec " + std::to_string(exec_count_) + " (" + Join(str_args, " ") + ")"; + static size_t exec_count = 0; + exec_count++; + std::string name = "exec " + std::to_string(exec_count) + " (" + Join(str_args, " ") + ")"; - unsigned flags = SVC_EXEC | SVC_ONESHOT | SVC_TEMPORARY; + unsigned flags = SVC_ONESHOT | SVC_TEMPORARY; CapSet no_capabilities; unsigned namespace_flags = 0; @@ -1037,12 +1007,8 @@ Service* ServiceManager::MakeExecOneshotService(const std::vector& } } - auto svc_p = std::make_unique(name, flags, uid, gid, supp_gids, no_capabilities, - namespace_flags, seclabel, str_args); - Service* svc = svc_p.get(); - services_.emplace_back(std::move(svc_p)); - - return svc; + return std::make_unique(name, flags, uid, gid, supp_gids, no_capabilities, + namespace_flags, seclabel, str_args); } Service* ServiceManager::FindServiceByName(const std::string& name) const { @@ -1154,8 +1120,10 @@ bool ServiceManager::ReapOneProcess() { if (svc) { name = StringPrintf("Service '%s' (pid %d)", svc->name().c_str(), pid); if (svc->flags() & SVC_EXEC) { - wait_string = StringPrintf(" waiting took %f seconds", - exec_waiter_->duration().count() / 1000.0f); + auto exec_duration = boot_clock::now() - svc->time_started(); + auto exec_duration_ms = + std::chrono::duration_cast(exec_duration).count(); + wait_string = StringPrintf(" waiting took %f seconds", exec_duration_ms / 1000.0f); } } else { name = StringPrintf("Untracked pid %d", pid); @@ -1174,9 +1142,6 @@ bool ServiceManager::ReapOneProcess() { svc->Reap(); - if (svc->flags() & SVC_EXEC) { - exec_waiter_.reset(); - } if (svc->flags() & SVC_TEMPORARY) { RemoveService(*svc); } @@ -1189,15 +1154,6 @@ void ServiceManager::ReapAnyOutstandingChildren() { } } -void ServiceManager::ClearExecWait() { - // Clear EXEC flag if there is one pending - // And clear the wait flag - for (const auto& s : services_) { - s->UnSetExec(); - } - exec_waiter_.reset(); -} - bool ServiceParser::ParseSection(std::vector&& args, const std::string& filename, int line, std::string* err) { if (args.size() < 3) { diff --git a/init/service.h b/init/service.h index 850aca487..4098005d3 100644 --- a/init/service.h +++ b/init/service.h @@ -73,9 +73,11 @@ class Service { unsigned namespace_flags, const std::string& seclabel, const std::vector& args); + static std::unique_ptr MakeTemporaryOneshotService(const std::vector& args); + bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; } bool ParseLine(const std::vector& args, std::string* err); - bool ExecStart(std::unique_ptr* exec_waiter); + bool ExecStart(); bool Start(); bool StartIfNotDisabled(); bool Enable(); @@ -87,7 +89,12 @@ class Service { void DumpState() const; void SetShutdownCritical() { flags_ |= SVC_SHUTDOWN_CRITICAL; } bool IsShutdownCritical() const { return (flags_ & SVC_SHUTDOWN_CRITICAL) != 0; } - void UnSetExec() { flags_ &= ~SVC_EXEC; } + void UnSetExec() { + is_exec_service_running_ = false; + flags_ &= ~SVC_EXEC; + } + + static bool is_exec_service_running() { return is_exec_service_running_; } const std::string& name() const { return name_; } const std::set& classnames() const { return classnames_; } @@ -151,6 +158,7 @@ class Service { bool AddDescriptor(const std::vector& args, std::string* err); static unsigned long next_start_order_; + static bool is_exec_service_running_; std::string name_; std::set classnames_; @@ -206,10 +214,6 @@ class ServiceManager { ServiceManager(); void AddService(std::unique_ptr service); - Service* MakeExecOneshotService(const std::vector& args); - bool Exec(const std::vector& args); - bool ExecStart(const std::string& name); - bool IsWaitingForExec() const; Service* FindServiceByName(const std::string& name) const; Service* FindServiceByPid(pid_t pid) const; Service* FindServiceByKeychord(int keychord_id) const; @@ -220,16 +224,12 @@ class ServiceManager { void ReapAnyOutstandingChildren(); void RemoveService(const Service& svc); void DumpState() const; - void ClearExecWait(); private: // Cleans up a child process that exited. // Returns true iff a children was cleaned up. bool ReapOneProcess(); - static int exec_count_; // Every service needs a unique name. - std::unique_ptr exec_waiter_; - std::vector> services_; }; diff --git a/init/service_test.cpp b/init/service_test.cpp index 123c8a5d6..62e46f4cd 100644 --- a/init/service_test.cpp +++ b/init/service_test.cpp @@ -73,23 +73,21 @@ TEST(service, pod_initialized) { EXPECT_FALSE(service_in_old_memory->process_cgroup_empty()); } -TEST(service, make_exec_oneshot_service_invalid_syntax) { - ServiceManager& sm = ServiceManager::GetInstance(); +TEST(service, make_temporary_oneshot_service_invalid_syntax) { std::vector args; // Nothing. - ASSERT_EQ(nullptr, sm.MakeExecOneshotService(args)); + ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); // No arguments to 'exec'. args.push_back("exec"); - ASSERT_EQ(nullptr, sm.MakeExecOneshotService(args)); + ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); // No command in "exec --". args.push_back("--"); - ASSERT_EQ(nullptr, sm.MakeExecOneshotService(args)); + ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); } -TEST(service, make_exec_oneshot_service_too_many_supplementary_gids) { - ServiceManager& sm = ServiceManager::GetInstance(); +TEST(service, make_temporary_oneshot_service_too_many_supplementary_gids) { std::vector args; args.push_back("exec"); args.push_back("seclabel"); @@ -100,12 +98,11 @@ TEST(service, make_exec_oneshot_service_too_many_supplementary_gids) { } args.push_back("--"); args.push_back("/system/bin/id"); - ASSERT_EQ(nullptr, sm.MakeExecOneshotService(args)); + ASSERT_EQ(nullptr, Service::MakeTemporaryOneshotService(args)); } -static void Test_make_exec_oneshot_service(bool dash_dash, bool seclabel, bool uid, bool gid, - bool supplementary_gids) { - ServiceManager& sm = ServiceManager::GetInstance(); +static void Test_make_temporary_oneshot_service(bool dash_dash, bool seclabel, bool uid, bool gid, + bool supplementary_gids) { std::vector args; args.push_back("exec"); if (seclabel) { @@ -126,7 +123,7 @@ static void Test_make_exec_oneshot_service(bool dash_dash, bool seclabel, bool u } args.push_back("/system/bin/toybox"); args.push_back("id"); - Service* svc = sm.MakeExecOneshotService(args); + auto svc = Service::MakeTemporaryOneshotService(args); ASSERT_NE(nullptr, svc); if (seclabel) { @@ -167,28 +164,28 @@ static void Test_make_exec_oneshot_service(bool dash_dash, bool seclabel, bool u ASSERT_EQ("id", svc->args()[1]); } -TEST(service, make_exec_oneshot_service_with_everything) { - Test_make_exec_oneshot_service(true, true, true, true, true); +TEST(service, make_temporary_oneshot_service_with_everything) { + Test_make_temporary_oneshot_service(true, true, true, true, true); } -TEST(service, make_exec_oneshot_service_with_seclabel_uid_gid) { - Test_make_exec_oneshot_service(true, true, true, true, false); +TEST(service, make_temporary_oneshot_service_with_seclabel_uid_gid) { + Test_make_temporary_oneshot_service(true, true, true, true, false); } -TEST(service, make_exec_oneshot_service_with_seclabel_uid) { - Test_make_exec_oneshot_service(true, true, true, false, false); +TEST(service, make_temporary_oneshot_service_with_seclabel_uid) { + Test_make_temporary_oneshot_service(true, true, true, false, false); } -TEST(service, make_exec_oneshot_service_with_seclabel) { - Test_make_exec_oneshot_service(true, true, false, false, false); +TEST(service, make_temporary_oneshot_service_with_seclabel) { + Test_make_temporary_oneshot_service(true, true, false, false, false); } -TEST(service, make_exec_oneshot_service_with_just_command) { - Test_make_exec_oneshot_service(true, false, false, false, false); +TEST(service, make_temporary_oneshot_service_with_just_command) { + Test_make_temporary_oneshot_service(true, false, false, false, false); } -TEST(service, make_exec_oneshot_service_with_just_command_no_dash) { - Test_make_exec_oneshot_service(false, false, false, false, false); +TEST(service, make_temporary_oneshot_service_with_just_command_no_dash) { + Test_make_temporary_oneshot_service(false, false, false, false, false); } } // namespace init From eeee83106bfbd719b52817c2ebb460ce38bcc494 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 28 Jul 2017 15:22:23 -0700 Subject: [PATCH 2/3] init: move reaping from ServiceManager to signal_handler.cpp signal_handler.cpp itself needs to be cleaned up, but this is a step to clean up ServiceManager. Test: boot bullhead Change-Id: I81f1e8ac4d09692cfb364bc702cbd3deb61aa55a --- init/reboot.cpp | 5 +-- init/service.cpp | 63 ----------------------------------- init/service.h | 5 --- init/signal_handler.cpp | 74 +++++++++++++++++++++++++++++++++++++++-- init/signal_handler.h | 2 ++ 5 files changed, 77 insertions(+), 72 deletions(-) diff --git a/init/reboot.cpp b/init/reboot.cpp index 6002040a1..b1cde9328 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -53,6 +53,7 @@ #include "init.h" #include "property_service.h" #include "service.h" +#include "signal_handler.h" using android::base::StringPrintf; using android::base::Timer; @@ -406,7 +407,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re // Only wait up to half of timeout here auto termination_wait_timeout = shutdown_timeout / 2; while (t.duration() < termination_wait_timeout) { - ServiceManager::GetInstance().ReapAnyOutstandingChildren(); + ReapAnyOutstandingChildren(); service_count = 0; ServiceManager::GetInstance().ForEachService([&service_count](Service* s) { @@ -437,7 +438,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re ServiceManager::GetInstance().ForEachServiceShutdownOrder([](Service* s) { if (!s->IsShutdownCritical()) s->Stop(); }); - ServiceManager::GetInstance().ReapAnyOutstandingChildren(); + ReapAnyOutstandingChildren(); // 3. send volume shutdown to vold Service* voldService = ServiceManager::GetInstance().FindServiceByName("vold"); diff --git a/init/service.cpp b/init/service.cpp index 7f7958483..b5e2067a5 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -1091,69 +1091,6 @@ void ServiceManager::DumpState() const { } } -bool ServiceManager::ReapOneProcess() { - siginfo_t siginfo = {}; - // This returns a zombie pid or informs us that there are no zombies left to be reaped. - // It does NOT reap the pid; that is done below. - if (TEMP_FAILURE_RETRY(waitid(P_ALL, 0, &siginfo, WEXITED | WNOHANG | WNOWAIT)) != 0) { - PLOG(ERROR) << "waitid failed"; - return false; - } - - auto pid = siginfo.si_pid; - if (pid == 0) return false; - - // At this point we know we have a zombie pid, so we use this scopeguard to reap the pid - // whenever the function returns from this point forward. - // We do NOT want to reap the zombie earlier as in Service::Reap(), we kill(-pid, ...) and we - // want the pid to remain valid throughout that (and potentially future) usages. - auto reaper = make_scope_guard([pid] { TEMP_FAILURE_RETRY(waitpid(pid, nullptr, WNOHANG)); }); - - if (PropertyChildReap(pid)) { - return true; - } - - Service* svc = FindServiceByPid(pid); - - std::string name; - std::string wait_string; - if (svc) { - name = StringPrintf("Service '%s' (pid %d)", svc->name().c_str(), pid); - if (svc->flags() & SVC_EXEC) { - auto exec_duration = boot_clock::now() - svc->time_started(); - auto exec_duration_ms = - std::chrono::duration_cast(exec_duration).count(); - wait_string = StringPrintf(" waiting took %f seconds", exec_duration_ms / 1000.0f); - } - } else { - name = StringPrintf("Untracked pid %d", pid); - } - - auto status = siginfo.si_status; - if (WIFEXITED(status)) { - LOG(INFO) << name << " exited with status " << WEXITSTATUS(status) << wait_string; - } else if (WIFSIGNALED(status)) { - LOG(INFO) << name << " killed by signal " << WTERMSIG(status) << wait_string; - } - - if (!svc) { - return true; - } - - svc->Reap(); - - if (svc->flags() & SVC_TEMPORARY) { - RemoveService(*svc); - } - - return true; -} - -void ServiceManager::ReapAnyOutstandingChildren() { - while (ReapOneProcess()) { - } -} - bool ServiceParser::ParseSection(std::vector&& args, const std::string& filename, int line, std::string* err) { if (args.size() < 3) { diff --git a/init/service.h b/init/service.h index 4098005d3..a7c91ae0a 100644 --- a/init/service.h +++ b/init/service.h @@ -221,15 +221,10 @@ class ServiceManager { void ForEachServiceShutdownOrder(const std::function& callback) const; void ForEachServiceInClass(const std::string& classname, void (*func)(Service* svc)) const; - void ReapAnyOutstandingChildren(); void RemoveService(const Service& svc); void DumpState() const; private: - // Cleans up a child process that exited. - // Returns true iff a children was cleaned up. - bool ReapOneProcess(); - std::vector> services_; }; diff --git a/init/signal_handler.cpp b/init/signal_handler.cpp index db1bfcf8c..d77a2123b 100644 --- a/init/signal_handler.cpp +++ b/init/signal_handler.cpp @@ -14,29 +14,94 @@ * limitations under the License. */ +#include "signal_handler.h" + #include #include #include #include +#include #include +#include #include +#include +#include #include "init.h" +#include "property_service.h" #include "service.h" +using android::base::StringPrintf; +using android::base::boot_clock; +using android::base::make_scope_guard; + namespace android { namespace init { static int signal_write_fd = -1; static int signal_read_fd = -1; +static bool ReapOneProcess() { + siginfo_t siginfo = {}; + // This returns a zombie pid or informs us that there are no zombies left to be reaped. + // It does NOT reap the pid; that is done below. + if (TEMP_FAILURE_RETRY(waitid(P_ALL, 0, &siginfo, WEXITED | WNOHANG | WNOWAIT)) != 0) { + PLOG(ERROR) << "waitid failed"; + return false; + } + + auto pid = siginfo.si_pid; + if (pid == 0) return false; + + // At this point we know we have a zombie pid, so we use this scopeguard to reap the pid + // whenever the function returns from this point forward. + // We do NOT want to reap the zombie earlier as in Service::Reap(), we kill(-pid, ...) and we + // want the pid to remain valid throughout that (and potentially future) usages. + auto reaper = make_scope_guard([pid] { TEMP_FAILURE_RETRY(waitpid(pid, nullptr, WNOHANG)); }); + + if (PropertyChildReap(pid)) return true; + + Service* service = ServiceManager::GetInstance().FindServiceByPid(pid); + + std::string name; + std::string wait_string; + if (service) { + name = StringPrintf("Service '%s' (pid %d)", service->name().c_str(), pid); + if (service->flags() & SVC_EXEC) { + auto exec_duration = boot_clock::now() - service->time_started(); + auto exec_duration_ms = + std::chrono::duration_cast(exec_duration).count(); + wait_string = StringPrintf(" waiting took %f seconds", exec_duration_ms / 1000.0f); + } + } else { + name = StringPrintf("Untracked pid %d", pid); + } + + auto status = siginfo.si_status; + if (WIFEXITED(status)) { + LOG(INFO) << name << " exited with status " << WEXITSTATUS(status) << wait_string; + } else if (WIFSIGNALED(status)) { + LOG(INFO) << name << " killed by signal " << WTERMSIG(status) << wait_string; + } + + if (!service) return true; + + service->Reap(); + + if (service->flags() & SVC_TEMPORARY) { + ServiceManager::GetInstance().RemoveService(*service); + } + + return true; +} + static void handle_signal() { // Clear outstanding requests. char buf[32]; read(signal_read_fd, buf, sizeof(buf)); - ServiceManager::GetInstance().ReapAnyOutstandingChildren(); + ReapAnyOutstandingChildren(); } static void SIGCHLD_handler(int) { @@ -45,6 +110,11 @@ static void SIGCHLD_handler(int) { } } +void ReapAnyOutstandingChildren() { + while (ReapOneProcess()) { + } +} + void signal_handler_init() { // Create a signalling mechanism for SIGCHLD. int s[2]; @@ -63,7 +133,7 @@ void signal_handler_init() { act.sa_flags = SA_NOCLDSTOP; sigaction(SIGCHLD, &act, 0); - ServiceManager::GetInstance().ReapAnyOutstandingChildren(); + ReapAnyOutstandingChildren(); register_epoll_handler(signal_read_fd, handle_signal); } diff --git a/init/signal_handler.h b/init/signal_handler.h index f7881abff..9362be532 100644 --- a/init/signal_handler.h +++ b/init/signal_handler.h @@ -20,6 +20,8 @@ namespace android { namespace init { +void ReapAnyOutstandingChildren(); + void signal_handler_init(void); } // namespace init From 911b9b1d6ead5f2e4c4158af0cd8787f4d458ddf Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Thu, 27 Jul 2017 16:20:58 -0700 Subject: [PATCH 3/3] init: rename ServiceManager to ServiceList and clean it up ServiceManager is essentially just a list now that the rest of its functionality has been moved elsewhere, so the class is renamed appropriately. The ServiceList::Find* functions have been cleaned up into a single smaller interface. The ServiceList::ForEach functions have been removed in favor of ServiceList itself being directly iterable. Test: boot bullhead Change-Id: Ibd57c103338f03b83d81e8b48ea0e46cd48fd8f0 --- init/builtins.cpp | 41 +++++++++++------------ init/init.cpp | 20 ++++++------ init/init.h | 2 +- init/keychords.cpp | 6 ++-- init/reboot.cpp | 37 +++++++++++---------- init/service.cpp | 72 ++++++----------------------------------- init/service.h | 36 +++++++++++++-------- init/signal_handler.cpp | 4 +-- 8 files changed, 91 insertions(+), 127 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 15594e687..035158201 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -124,31 +124,32 @@ static int reboot_into_recovery(const std::vector& options) { return 0; } +template +static void ForEachServiceInClass(const std::string& classname, F function) { + for (const auto& service : ServiceList::GetInstance()) { + if (service->classnames().count(classname)) std::invoke(function, service); + } +} + static int do_class_start(const std::vector& args) { - /* Starting a class does not start services - * which are explicitly disabled. They must - * be started individually. - */ - ServiceManager::GetInstance(). - ForEachServiceInClass(args[1], [] (Service* s) { s->StartIfNotDisabled(); }); + // Starting a class does not start services which are explicitly disabled. + // They must be started individually. + ForEachServiceInClass(args[1], &Service::StartIfNotDisabled); return 0; } static int do_class_stop(const std::vector& args) { - ServiceManager::GetInstance(). - ForEachServiceInClass(args[1], [] (Service* s) { s->Stop(); }); + ForEachServiceInClass(args[1], &Service::Stop); return 0; } static int do_class_reset(const std::vector& args) { - ServiceManager::GetInstance(). - ForEachServiceInClass(args[1], [] (Service* s) { s->Reset(); }); + ForEachServiceInClass(args[1], &Service::Reset); return 0; } static int do_class_restart(const std::vector& args) { - ServiceManager::GetInstance(). - ForEachServiceInClass(args[1], [] (Service* s) { s->Restart(); }); + ForEachServiceInClass(args[1], &Service::Restart); return 0; } @@ -162,7 +163,7 @@ static int do_domainname(const std::vector& args) { } static int do_enable(const std::vector& args) { - Service* svc = ServiceManager::GetInstance().FindServiceByName(args[1]); + Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) { return -1; } @@ -179,12 +180,12 @@ static int do_exec(const std::vector& args) { LOG(ERROR) << "Failed to Start exec service"; return -1; } - ServiceManager::GetInstance().AddService(std::move(service)); + ServiceList::GetInstance().AddService(std::move(service)); return 0; } static int do_exec_start(const std::vector& args) { - Service* service = ServiceManager::GetInstance().FindServiceByName(args[1]); + Service* service = ServiceList::GetInstance().FindService(args[1]); if (!service) { LOG(ERROR) << "ExecStart(" << args[1] << "): Service not found"; return -1; @@ -408,8 +409,8 @@ exit_success: */ static void import_late(const std::vector& args, size_t start_index, size_t end_index) { auto& action_manager = ActionManager::GetInstance(); - auto& service_manager = ServiceManager::GetInstance(); - Parser parser = CreateParser(action_manager, service_manager); + auto& service_list = ServiceList::GetInstance(); + Parser parser = CreateParser(action_manager, service_list); if (end_index <= start_index) { // Fallbacks for partitions on which early mount isn't enabled. for (const auto& path : late_import_paths) { @@ -599,7 +600,7 @@ static int do_setrlimit(const std::vector& args) { } static int do_start(const std::vector& args) { - Service* svc = ServiceManager::GetInstance().FindServiceByName(args[1]); + Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) { LOG(ERROR) << "do_start: Service " << args[1] << " not found"; return -1; @@ -610,7 +611,7 @@ static int do_start(const std::vector& args) { } static int do_stop(const std::vector& args) { - Service* svc = ServiceManager::GetInstance().FindServiceByName(args[1]); + Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) { LOG(ERROR) << "do_stop: Service " << args[1] << " not found"; return -1; @@ -620,7 +621,7 @@ static int do_stop(const std::vector& args) { } static int do_restart(const std::vector& args) { - Service* svc = ServiceManager::GetInstance().FindServiceByName(args[1]); + Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) { LOG(ERROR) << "do_restart: Service " << args[1] << " not found"; return -1; diff --git a/init/init.cpp b/init/init.cpp index a12afae0a..8f946f757 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -98,22 +98,22 @@ static bool shutting_down; std::vector late_import_paths; void DumpState() { - ServiceManager::GetInstance().DumpState(); + ServiceList::GetInstance().DumpState(); ActionManager::GetInstance().DumpState(); } -Parser CreateParser(ActionManager& action_manager, ServiceManager& service_manager) { +Parser CreateParser(ActionManager& action_manager, ServiceList& service_list) { Parser parser; - parser.AddSectionParser("service", std::make_unique(&service_manager)); + parser.AddSectionParser("service", std::make_unique(&service_list)); parser.AddSectionParser("on", std::make_unique(&action_manager)); parser.AddSectionParser("import", std::make_unique(&parser)); return parser; } -static void LoadBootScripts(ActionManager& action_manager, ServiceManager& service_manager) { - Parser parser = CreateParser(action_manager, service_manager); +static void LoadBootScripts(ActionManager& action_manager, ServiceList& service_list) { + Parser parser = CreateParser(action_manager, service_list); std::string bootscript = GetProperty("ro.boot.init_rc", ""); if (bootscript.empty()) { @@ -221,8 +221,8 @@ void property_changed(const std::string& name, const std::string& value) { static std::optional RestartProcesses() { std::optional next_process_restart_time; - ServiceManager::GetInstance().ForEachService([&next_process_restart_time](Service* s) { - if (!(s->flags() & SVC_RESTARTING)) return; + for (const auto& s : ServiceList::GetInstance()) { + if (!(s->flags() & SVC_RESTARTING)) continue; auto restart_time = s->time_started() + 5s; if (boot_clock::now() > restart_time) { @@ -232,12 +232,12 @@ static std::optional RestartProcesses() { next_process_restart_time = restart_time; } } - }); + } return next_process_restart_time; } void handle_control_message(const std::string& msg, const std::string& name) { - Service* svc = ServiceManager::GetInstance().FindServiceByName(name); + Service* svc = ServiceList::GetInstance().FindService(name); if (svc == nullptr) { LOG(ERROR) << "no such service '" << name << "'"; return; @@ -1139,7 +1139,7 @@ int main(int argc, char** argv) { Action::set_function_map(&function_map); ActionManager& am = ActionManager::GetInstance(); - ServiceManager& sm = ServiceManager::GetInstance(); + ServiceList& sm = ServiceList::GetInstance(); LoadBootScripts(am, sm); diff --git a/init/init.h b/init/init.h index 0a77bd211..92b9b7003 100644 --- a/init/init.h +++ b/init/init.h @@ -38,7 +38,7 @@ extern struct selabel_handle *sehandle_prop; extern std::vector late_import_paths; -Parser CreateParser(ActionManager& action_manager, ServiceManager& service_manager); +Parser CreateParser(ActionManager& action_manager, ServiceList& service_list); void handle_control_message(const std::string& msg, const std::string& arg); diff --git a/init/keychords.cpp b/init/keychords.cpp index a0d7cc5de..2ef0ce776 100644 --- a/init/keychords.cpp +++ b/init/keychords.cpp @@ -79,7 +79,7 @@ static void handle_keychord() { // Only handle keychords if adb is enabled. std::string adb_enabled = android::base::GetProperty("init.svc.adbd", ""); if (adb_enabled == "running") { - Service* svc = ServiceManager::GetInstance().FindServiceByKeychord(id); + Service* svc = ServiceList::GetInstance().FindService(id, &Service::keychord_id); if (svc) { LOG(INFO) << "Starting service " << svc->name() << " from keychord " << id; svc->Start(); @@ -92,7 +92,9 @@ static void handle_keychord() { } void keychord_init() { - ServiceManager::GetInstance().ForEachService(add_service_keycodes); + for (const auto& service : ServiceList::GetInstance()) { + add_service_keycodes(service.get()); + } // Nothing to do if no services require keychords. if (!keychords) { diff --git a/init/reboot.cpp b/init/reboot.cpp index b1cde9328..cfd703ec1 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -374,7 +374,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re const std::set kill_after_apps{"tombstoned", "logd", "adbd"}; // watchdogd is a vendor specific component but should be alive to complete shutdown safely. const std::set to_starts{"watchdogd"}; - ServiceManager::GetInstance().ForEachService([&kill_after_apps, &to_starts](Service* s) { + for (const auto& s : ServiceList::GetInstance()) { if (kill_after_apps.count(s->name())) { s->SetShutdownCritical(); } else if (to_starts.count(s->name())) { @@ -383,14 +383,15 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re } else if (s->IsShutdownCritical()) { s->Start(); // start shutdown critical service if not started } - }); + } - Service* bootAnim = ServiceManager::GetInstance().FindServiceByName("bootanim"); - Service* surfaceFlinger = ServiceManager::GetInstance().FindServiceByName("surfaceflinger"); + Service* bootAnim = ServiceList::GetInstance().FindService("bootanim"); + Service* surfaceFlinger = ServiceList::GetInstance().FindService("surfaceflinger"); if (bootAnim != nullptr && surfaceFlinger != nullptr && surfaceFlinger->IsRunning()) { - ServiceManager::GetInstance().ForEachServiceInClass("animation", [](Service* s) { - s->SetShutdownCritical(); // will not check animation class separately - }); + // will not check animation class separately + for (const auto& service : ServiceList::GetInstance()) { + if (service->classnames().count("animation")) service->SetShutdownCritical(); + } } // optional shutdown step @@ -399,9 +400,9 @@ 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().ForEachServiceShutdownOrder([](Service* s) { + for (const auto& s : ServiceList::GetInstance().services_in_shutdown_order()) { if (!s->IsShutdownCritical()) s->Terminate(); - }); + } int service_count = 0; // Only wait up to half of timeout here @@ -410,7 +411,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re ReapAnyOutstandingChildren(); service_count = 0; - ServiceManager::GetInstance().ForEachService([&service_count](Service* s) { + for (const auto& s : ServiceList::GetInstance()) { // Count the number of services running except shutdown critical. // Exclude the console as it will ignore the SIGTERM signal // and not exit. @@ -419,7 +420,7 @@ void DoReboot(unsigned int cmd, const std::string& reason, const std::string& re if (!s->IsShutdownCritical() && s->pid() != 0 && (s->flags() & SVC_CONSOLE) == 0) { service_count++; } - }); + } if (service_count == 0) { // All terminable services terminated. We can exit early. @@ -435,13 +436,13 @@ 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().ForEachServiceShutdownOrder([](Service* s) { + for (const auto& s : ServiceList::GetInstance().services_in_shutdown_order()) { if (!s->IsShutdownCritical()) s->Stop(); - }); + } ReapAnyOutstandingChildren(); // 3. send volume shutdown to vold - Service* voldService = ServiceManager::GetInstance().FindServiceByName("vold"); + Service* voldService = ServiceList::GetInstance().FindService("vold"); if (voldService != nullptr && voldService->IsRunning()) { ShutdownVold(); voldService->Stop(); @@ -449,9 +450,9 @@ 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().ForEachServiceShutdownOrder([&kill_after_apps](Service* s) { + for (const auto& s : ServiceList::GetInstance().services_in_shutdown_order()) { if (kill_after_apps.count(s->name())) s->Stop(); - }); + } // 4. sync, try umount, and optionally run fsck for user shutdown sync(); UmountStat stat = TryUmountAndFsck(runFsck, shutdown_timeout - t.duration()); @@ -526,7 +527,9 @@ bool HandlePowerctlMessage(const std::string& command) { ResetWaitForProp(); // Clear EXEC flag if there is one pending - ServiceManager::GetInstance().ForEachService([](Service* s) { s->UnSetExec(); }); + for (const auto& s : ServiceList::GetInstance()) { + s->UnSetExec(); + } return true; } diff --git a/init/service.cpp b/init/service.cpp index b5e2067a5..6f756fa6b 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -935,15 +935,14 @@ void Service::OpenConsole() const { close(fd); } -ServiceManager::ServiceManager() { -} +ServiceList::ServiceList() {} -ServiceManager& ServiceManager::GetInstance() { - static ServiceManager instance; +ServiceList& ServiceList::GetInstance() { + static ServiceList instance; return instance; } -void ServiceManager::AddService(std::unique_ptr service) { +void ServiceList::AddService(std::unique_ptr service) { services_.emplace_back(std::move(service)); } @@ -1011,69 +1010,18 @@ std::unique_ptr Service::MakeTemporaryOneshotService(const std::vector< namespace_flags, seclabel, str_args); } -Service* ServiceManager::FindServiceByName(const std::string& name) const { - auto svc = std::find_if(services_.begin(), services_.end(), - [&name] (const std::unique_ptr& s) { - return name == s->name(); - }); - if (svc != services_.end()) { - return svc->get(); - } - return nullptr; -} - -Service* ServiceManager::FindServiceByPid(pid_t pid) const { - auto svc = std::find_if(services_.begin(), services_.end(), - [&pid] (const std::unique_ptr& s) { - return s->pid() == pid; - }); - if (svc != services_.end()) { - return svc->get(); - } - return nullptr; -} - -Service* ServiceManager::FindServiceByKeychord(int keychord_id) const { - auto svc = std::find_if(services_.begin(), services_.end(), - [&keychord_id] (const std::unique_ptr& s) { - return s->keychord_id() == keychord_id; - }); - - if (svc != services_.end()) { - return svc->get(); - } - return nullptr; -} - -void ServiceManager::ForEachService(const std::function& callback) const { - for (const auto& s : services_) { - callback(s.get()); - } -} - // Shutdown services in the opposite order that they were started. -void ServiceManager::ForEachServiceShutdownOrder(const std::function& callback) const { +const std::vector ServiceList::services_in_shutdown_order() 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); - } + return shutdown_services; } -void ServiceManager::ForEachServiceInClass(const std::string& classname, - void (*func)(Service* svc)) const { - for (const auto& s : services_) { - if (s->classnames().find(classname) != s->classnames().end()) { - func(s.get()); - } - } -} - -void ServiceManager::RemoveService(const Service& svc) { +void ServiceList::RemoveService(const Service& svc) { auto svc_it = std::find_if(services_.begin(), services_.end(), [&svc] (const std::unique_ptr& s) { return svc.name() == s->name(); @@ -1085,7 +1033,7 @@ void ServiceManager::RemoveService(const Service& svc) { services_.erase(svc_it); } -void ServiceManager::DumpState() const { +void ServiceList::DumpState() const { for (const auto& s : services_) { s->DumpState(); } @@ -1104,7 +1052,7 @@ bool ServiceParser::ParseSection(std::vector&& args, const std::str return false; } - Service* old_service = service_manager_->FindServiceByName(name); + Service* old_service = service_list_->FindService(name); if (old_service) { *err = "ignored duplicate definition of service '" + name + "'"; return false; @@ -1121,7 +1069,7 @@ bool ServiceParser::ParseLineSection(std::vector&& args, int line, void ServiceParser::EndSection() { if (service_) { - service_manager_->AddService(std::move(service_)); + service_list_->AddService(std::move(service_)); } } diff --git a/init/service.h b/init/service.h index a7c91ae0a..6c143cb61 100644 --- a/init/service.h +++ b/init/service.h @@ -206,32 +206,42 @@ class Service { std::vector args_; }; -class ServiceManager { +class ServiceList { public: - static ServiceManager& GetInstance(); + static ServiceList& GetInstance(); // Exposed for testing - ServiceManager(); + ServiceList(); void AddService(std::unique_ptr service); - Service* FindServiceByName(const std::string& name) const; - 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 RemoveService(const Service& svc); + + template + Service* FindService(T value, F function = &Service::name) const { + auto svc = std::find_if(services_.begin(), services_.end(), + [&function, &value](const std::unique_ptr& s) { + return std::invoke(function, s) == value; + }); + if (svc != services_.end()) { + return svc->get(); + } + return nullptr; + } + void DumpState() const; + auto begin() const { return services_.begin(); } + auto end() const { return services_.end(); } + const std::vector>& services() const { return services_; } + const std::vector services_in_shutdown_order() const; + private: std::vector> services_; }; class ServiceParser : public SectionParser { public: - ServiceParser(ServiceManager* service_manager) - : service_manager_(service_manager), service_(nullptr) {} + ServiceParser(ServiceList* service_list) : service_list_(service_list), service_(nullptr) {} bool ParseSection(std::vector&& args, const std::string& filename, int line, std::string* err) override; bool ParseLineSection(std::vector&& args, int line, std::string* err) override; @@ -240,7 +250,7 @@ class ServiceParser : public SectionParser { private: bool IsValidName(const std::string& name) const; - ServiceManager* service_manager_; + ServiceList* service_list_; std::unique_ptr service_; }; diff --git a/init/signal_handler.cpp b/init/signal_handler.cpp index d77a2123b..9e49c48a7 100644 --- a/init/signal_handler.cpp +++ b/init/signal_handler.cpp @@ -62,7 +62,7 @@ static bool ReapOneProcess() { if (PropertyChildReap(pid)) return true; - Service* service = ServiceManager::GetInstance().FindServiceByPid(pid); + Service* service = ServiceList::GetInstance().FindService(pid, &Service::pid); std::string name; std::string wait_string; @@ -90,7 +90,7 @@ static bool ReapOneProcess() { service->Reap(); if (service->flags() & SVC_TEMPORARY) { - ServiceManager::GetInstance().RemoveService(*service); + ServiceList::GetInstance().RemoveService(*service); } return true;