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

@@ -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 <service>`
+> 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 <name> <value>`
 > Set the environment variable _name_ equal to _value_ in the

init/builtins.cpp

@@ -161,19 +161,11 @@ static int do_enable(const std::vector<std::string>& args) {
 }
 
 static int do_exec(const std::vector<std::string>& args) {
-    Service* svc = ServiceManager::GetInstance().MakeExecOneshotService(args);
+    return ServiceManager::GetInstance().Exec(args) ? 0 : -1;
-    if (!svc) {
+}
-        return -1;
+
-    }
+static int do_exec_start(const std::vector<std::string>& args) {
-    if (!start_waiting_for_exec()) {
+    return ServiceManager::GetInstance().ExecStart(args[1]) ? 0 : -1;
-        return -1;
-    }
-    if (!svc->Start()) {
-        stop_waiting_for_exec();
-        ServiceManager::GetInstance().RemoveService(*svc);
-        return -1;
-    }
-    return 0;
 }
 
 static int do_export(const std::vector<std::string>& args) {
@@ -892,6 +884,7 @@ static int do_init_user0(const std::vector<std::string>& args) {
 
 BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const {
     constexpr std::size_t kMax = std::numeric_limits<std::size_t>::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;
 }

init/init.cpp

@@ -86,8 +86,6 @@ static time_t process_needs_restart_at;
 
 const char *ENV[32];
 
-static std::unique_ptr<Timer> waiting_for_exec(nullptr);
-
 static int epoll_fd = -1;
 
 static std::unique_ptr<Timer> 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.

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 */

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) {
+    if ((flags_ & SVC_TEMPORARY) != 0) {
-        // 'exec' commands don't have properties tracking their state.
+        // 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<std::string>& args, std::string* err)
     return (this->*parser)(args, err);
 }
 
+bool Service::ExecStart(std::unique_ptr<Timer>* 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> service) {
     services_.emplace_back(std::move(service));
 }
 
+bool ServiceManager::Exec(const std::vector<std::string>& 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<std::string>& 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<std::string>&
 
     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()) {
+    svc->Reap();
-        stop_waiting_for_exec();
+
+    if (svc->flags() & SVC_EXEC) {
+        LOG(INFO) << "Wait for exec took " << *exec_waiter_;
+        exec_waiter_.reset();
+    }
+    if (svc->flags() & SVC_TEMPORARY) {
         RemoveService(*svc);
     }
 

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<std::string>& args, std::string* err);
+    bool ExecStart(std::unique_ptr<Timer>* 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);
     Service* MakeExecOneshotService(const std::vector<std::string>& args);
+    bool Exec(const std::vector<std::string>& 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<Timer> exec_waiter_;
+
     std::vector<std::unique_ptr<Service>> services_;
 };