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
diff --git a/init/builtins.cpp b/init/builtins.cpp
index dec6f40..15594e6 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -170,11 +170,30 @@
}
static int do_exec(const std::vector<std::string>& 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<std::string>& 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<std::string>& args) {
diff --git a/init/init.cpp b/init/init.cpp
index 03f0bb7..a12afae 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -1180,10 +1180,10 @@
// 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 ce81483..6002040 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -524,10 +524,8 @@
// 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 d72433a..7f79584 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -156,6 +156,7 @@
}
unsigned long Service::next_start_order_ = 1;
+bool Service::is_exec_service_running_ = false;
Service::Service(const std::string& name, const std::vector<std::string>& args)
: Service(name, 0, 0, 0, {}, 0, 0, "", args) {}
@@ -280,9 +281,9 @@
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 @@
return (this->*parser)(args, err);
}
-bool Service::ExecStart(std::unique_ptr<android::base::Timer>* 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 @@
}
}
- 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 @@
close(fd);
}
-int ServiceManager::exec_count_ = 0;
-
ServiceManager::ServiceManager() {
}
@@ -949,36 +947,7 @@
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) {
+std::unique_ptr<Service> Service::MakeTemporaryOneshotService(const std::vector<std::string>& 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 @@
}
std::vector<std::string> 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 @@
}
}
- auto svc_p = std::make_unique<Service>(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<Service>(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 @@
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<std::chrono::milliseconds>(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 @@
svc->Reap();
- if (svc->flags() & SVC_EXEC) {
- exec_waiter_.reset();
- }
if (svc->flags() & SVC_TEMPORARY) {
RemoveService(*svc);
}
@@ -1189,15 +1154,6 @@
}
}
-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<std::string>&& 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 850aca4..4098005 100644
--- a/init/service.h
+++ b/init/service.h
@@ -73,9 +73,11 @@
unsigned namespace_flags, const std::string& seclabel,
const std::vector<std::string>& args);
+ static std::unique_ptr<Service> MakeTemporaryOneshotService(const std::vector<std::string>& args);
+
bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; }
bool ParseLine(const std::vector<std::string>& args, std::string* err);
- bool ExecStart(std::unique_ptr<android::base::Timer>* exec_waiter);
+ bool ExecStart();
bool Start();
bool StartIfNotDisabled();
bool Enable();
@@ -87,7 +89,12 @@
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<std::string>& classnames() const { return classnames_; }
@@ -151,6 +158,7 @@
bool AddDescriptor(const std::vector<std::string>& args, std::string* err);
static unsigned long next_start_order_;
+ static bool is_exec_service_running_;
std::string name_;
std::set<std::string> classnames_;
@@ -206,10 +214,6 @@
ServiceManager();
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;
@@ -220,16 +224,12 @@
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<android::base::Timer> exec_waiter_;
-
std::vector<std::unique_ptr<Service>> services_;
};
diff --git a/init/service_test.cpp b/init/service_test.cpp
index 123c8a5..62e46f4 100644
--- a/init/service_test.cpp
+++ b/init/service_test.cpp
@@ -73,23 +73,21 @@
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<std::string> 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<std::string> args;
args.push_back("exec");
args.push_back("seclabel");
@@ -100,12 +98,11 @@
}
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<std::string> args;
args.push_back("exec");
if (seclabel) {
@@ -126,7 +123,7 @@
}
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 @@
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