From b27004aa05039b5196f1e878169dca41b68aadd6 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 27 Mar 2017 16:27:30 -0700 Subject: [PATCH 1/2] init: add exec_start command Exec services may also want to set other service flags such as priority. Instead of expanding the exec syntax to handle this, create a new command, exec_start, that will treat an existing service definition as an exec service. The new exec_start command will start the service then halt init from executing further commands until the service has exited. This change additionally encapsulates the waiting_for_exec logic into ServiceManager and removes the ambiguous 'bool' return value from Reap() which previously indicated if a Reaped service was an exec service or not. Bug: 36511808 Bug: 36102163 Test: Bullhead boots, services run with exec_start as they do exec. Change-Id: I44f775cf1c1dd81d5c715f44fdc150c651a2c80a --- init/README.md | 6 +++++ init/builtins.cpp | 21 ++++++--------- init/init.cpp | 23 ++-------------- init/init.h | 4 --- init/service.cpp | 67 ++++++++++++++++++++++++++++++++++++++++------- init/service.h | 13 +++++++-- 6 files changed, 85 insertions(+), 49 deletions(-) diff --git a/init/README.md b/init/README.md index 709c6675f..1b2690cab 100644 --- a/init/README.md +++ b/init/README.md @@ -307,6 +307,12 @@ Commands groups can be provided. No other commands will be run until this one finishes. _seclabel_ can be a - to denote default. Properties are expanded within _argument_. + Init halts executing commands until the forked process exits. + +`exec_start ` +> Start service a given service and halt processing of additional init commands + until it returns. It functions similarly to the `exec` command, but uses an + existing service definition instead of providing them as arguments. `export ` > Set the environment variable _name_ equal to _value_ in the diff --git a/init/builtins.cpp b/init/builtins.cpp index 32e9ef642..c860e4001 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -161,19 +161,11 @@ static int do_enable(const std::vector& args) { } static int do_exec(const std::vector& args) { - Service* svc = ServiceManager::GetInstance().MakeExecOneshotService(args); - if (!svc) { - return -1; - } - if (!start_waiting_for_exec()) { - return -1; - } - if (!svc->Start()) { - stop_waiting_for_exec(); - ServiceManager::GetInstance().RemoveService(*svc); - return -1; - } - return 0; + return ServiceManager::GetInstance().Exec(args) ? 0 : -1; +} + +static int do_exec_start(const std::vector& args) { + return ServiceManager::GetInstance().ExecStart(args[1]) ? 0 : -1; } static int do_export(const std::vector& args) { @@ -892,6 +884,7 @@ static int do_init_user0(const std::vector& args) { BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { constexpr std::size_t kMax = std::numeric_limits::max(); + // clang-format off static const Map builtin_functions = { {"bootchart", {1, 1, do_bootchart}}, {"chmod", {2, 2, do_chmod}}, @@ -903,6 +896,7 @@ BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"domainname", {1, 1, do_domainname}}, {"enable", {1, 1, do_enable}}, {"exec", {1, kMax, do_exec}}, + {"exec_start", {1, 1, do_exec_start}}, {"export", {2, 2, do_export}}, {"hostname", {1, 1, do_hostname}}, {"ifup", {1, 1, do_ifup}}, @@ -936,5 +930,6 @@ BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"wait_for_prop", {2, 2, do_wait_for_prop}}, {"write", {2, 2, do_write}}, }; + // clang-format on return builtin_functions; } diff --git a/init/init.cpp b/init/init.cpp index 23448d609..5447007ef 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -86,8 +86,6 @@ static time_t process_needs_restart_at; const char *ENV[32]; -static std::unique_ptr waiting_for_exec(nullptr); - static int epoll_fd = -1; static std::unique_ptr waiting_for_prop(nullptr); @@ -135,23 +133,6 @@ int add_environment(const char *key, const char *val) return -1; } -bool start_waiting_for_exec() -{ - if (waiting_for_exec) { - return false; - } - waiting_for_exec.reset(new Timer()); - return true; -} - -void stop_waiting_for_exec() -{ - if (waiting_for_exec) { - LOG(INFO) << "Wait for exec took " << *waiting_for_exec; - waiting_for_exec.reset(); - } -} - bool start_waiting_for_property(const char *name, const char *value) { if (waiting_for_prop) { @@ -1325,10 +1306,10 @@ int main(int argc, char** argv) { // By default, sleep until something happens. int epoll_timeout_ms = -1; - if (!(waiting_for_exec || waiting_for_prop)) { + if (!(waiting_for_prop || ServiceManager::GetInstance().IsWaitingForExec())) { am.ExecuteOneCommand(); } - if (!(waiting_for_exec || waiting_for_prop)) { + if (!(waiting_for_prop || ServiceManager::GetInstance().IsWaitingForExec())) { restart_processes(); // If there's a process that needs restarting, wake up in time for that. diff --git a/init/init.h b/init/init.h index 3768c02b2..1a05d98cd 100644 --- a/init/init.h +++ b/init/init.h @@ -35,10 +35,6 @@ void register_epoll_handler(int fd, void (*fn)()); int add_environment(const char* key, const char* val); -bool start_waiting_for_exec(); - -void stop_waiting_for_exec(); - bool start_waiting_for_property(const char *name, const char *value); #endif /* _INIT_INIT_H */ diff --git a/init/service.cpp b/init/service.cpp index c8d1cb166..a6efe52f6 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -174,8 +174,8 @@ Service::Service(const std::string& name, const std::string& classname, } void Service::NotifyStateChange(const std::string& new_state) const { - if ((flags_ & SVC_EXEC) != 0) { - // 'exec' commands don't have properties tracking their state. + if ((flags_ & SVC_TEMPORARY) != 0) { + // Services created by 'exec' are temporary and don't have properties tracking their state. return; } @@ -242,7 +242,7 @@ void Service::SetProcessAttributes() { } } -bool Service::Reap() { +void Service::Reap() { if (!(flags_ & SVC_ONESHOT) || (flags_ & SVC_RESTART)) { KillProcessGroup(SIGKILL); } @@ -253,7 +253,10 @@ bool Service::Reap() { if (flags_ & SVC_EXEC) { LOG(INFO) << "SVC_EXEC pid " << pid_ << " finished..."; - return true; + } + + if (flags_ & SVC_TEMPORARY) { + return; } pid_ = 0; @@ -268,7 +271,7 @@ bool Service::Reap() { // Disabled and reset processes do not get restarted automatically. if (flags_ & (SVC_DISABLED | SVC_RESET)) { NotifyStateChange("stopped"); - return false; + return; } // If we crash > 4 times in 4 minutes, reboot into recovery. @@ -292,7 +295,7 @@ bool Service::Reap() { onrestart_.ExecuteAllCommands(); NotifyStateChange("restarting"); - return false; + return; } void Service::DumpState() const { @@ -558,6 +561,18 @@ 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 Timer); + + if (!Start()) { + exec_waiter->reset(); + return false; + } + return true; +} + bool Service::Start() { // 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. @@ -844,6 +859,35 @@ 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) { // Parse the arguments: exec [SECLABEL [UID [GID]*] --] COMMAND ARGS... // SECLABEL can be a - to denote default @@ -867,7 +911,7 @@ Service* ServiceManager::MakeExecOneshotService(const std::vector& exec_count_++; std::string name = StringPrintf("exec %d (%s)", exec_count_, str_args[0].c_str()); - unsigned flags = SVC_EXEC | SVC_ONESHOT; + unsigned flags = SVC_EXEC | SVC_ONESHOT | SVC_TEMPORARY; CapSet no_capabilities; unsigned namespace_flags = 0; @@ -1007,8 +1051,13 @@ bool ServiceManager::ReapOneProcess() { return true; } - if (svc->Reap()) { - stop_waiting_for_exec(); + svc->Reap(); + + if (svc->flags() & SVC_EXEC) { + LOG(INFO) << "Wait for exec took " << *exec_waiter_; + exec_waiter_.reset(); + } + if (svc->flags() & SVC_TEMPORARY) { RemoveService(*svc); } diff --git a/init/service.h b/init/service.h index 08388e239..3ca7c32aa 100644 --- a/init/service.h +++ b/init/service.h @@ -43,10 +43,13 @@ #define SVC_RC_DISABLED 0x080 // Remember if the disabled flag was set in the rc script. #define SVC_RESTART 0x100 // Use to safely restart (stop, wait, start) a service. #define SVC_DISABLED_START 0x200 // A start was requested but it was disabled at the time. -#define SVC_EXEC 0x400 // This synthetic service corresponds to an 'exec'. +#define SVC_EXEC 0x400 // This service was started by either 'exec' or 'exec_start' and stops + // init from processing more commands until it completes #define SVC_SHUTDOWN_CRITICAL 0x800 // This service is critical for shutdown and // 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 NR_SVC_SUPP_GIDS 12 // twelve supplementary groups @@ -73,6 +76,7 @@ public: bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; } bool ParseLine(const std::vector& args, std::string* err); + bool ExecStart(std::unique_ptr* exec_waiter); bool Start(); bool StartIfNotDisabled(); bool Enable(); @@ -81,7 +85,7 @@ public: void Terminate(); void Restart(); void RestartIfNeeded(time_t* process_needs_restart_at); - bool Reap(); + void Reap(); void DumpState() const; void SetShutdownCritical() { flags_ |= SVC_SHUTDOWN_CRITICAL; } bool IsShutdownCritical() { return (flags_ & SVC_SHUTDOWN_CRITICAL) != 0; } @@ -179,6 +183,9 @@ public: 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; @@ -199,6 +206,8 @@ private: bool ReapOneProcess(); static int exec_count_; // Every service needs a unique name. + std::unique_ptr exec_waiter_; + std::vector> services_; }; From 1e2d8c7fce132773da8058f7210c0d990c4cb1a9 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 28 Mar 2017 13:28:38 -0700 Subject: [PATCH 2/2] init.rc: launch update_verifier with exec_start This allows update_verifier to raise it's priority and ioprio. Bug: 36511808 Bug: 36102163 Test: Boot bullhead Test: Verify boottime decrease on sailfish Change-Id: I5710c6a98dc7acee9063d1fa1d1c80668f0f1528 --- rootdir/init.rc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rootdir/init.rc b/rootdir/init.rc index 7091ab95b..d48b892b1 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -598,7 +598,7 @@ on boot on nonencrypted # A/B update verifier that marks a successful boot. - exec - root cache -- /system/bin/update_verifier nonencrypted + exec_start update_verifier_nonencrypted class_start main class_start late_start @@ -621,12 +621,12 @@ on property:vold.decrypt=trigger_post_fs_data on property:vold.decrypt=trigger_restart_min_framework # A/B update verifier that marks a successful boot. - exec - root cache -- /system/bin/update_verifier trigger_restart_min_framework + exec_start update_verifier class_start main on property:vold.decrypt=trigger_restart_framework # A/B update verifier that marks a successful boot. - exec - root cache -- /system/bin/update_verifier trigger_restart_framework + exec_start update_verifier class_start main class_start late_start