Revert "init: handle property service callbacks asynchronously"
This is apparently causing problems with reboot.
This reverts commit d2dab830d3ec260ccd982d147d4eb0ef74071261.
Bug: 150863651
Test: build
Merged-In: Ib8a4835cdc8358a54c7acdebc5c95038963a0419
Change-Id: Ib8a4835cdc8358a54c7acdebc5c95038963a0419
diff --git a/init/Android.bp b/init/Android.bp
index 52628f3..3bb08db 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -28,7 +28,6 @@
"rlimit_parser.cpp",
"service.cpp",
"service_list.cpp",
- "service_lock.cpp",
"service_parser.cpp",
"service_utils.cpp",
"subcontext.cpp",
@@ -82,7 +81,6 @@
"-Wextra",
"-Wno-unused-parameter",
"-Werror",
- "-Wthread-safety",
"-DALLOW_FIRST_STAGE_CONSOLE=0",
"-DALLOW_LOCAL_PROP_OVERRIDE=0",
"-DALLOW_PERMISSIVE_SELINUX=0",
diff --git a/init/action_manager.cpp b/init/action_manager.cpp
index b45f5cd..ebca762 100644
--- a/init/action_manager.cpp
+++ b/init/action_manager.cpp
@@ -41,12 +41,10 @@
}
void ActionManager::QueueEventTrigger(const std::string& trigger) {
- auto lock = std::lock_guard{event_queue_lock_};
event_queue_.emplace(trigger);
}
void ActionManager::QueuePropertyChange(const std::string& name, const std::string& value) {
- auto lock = std::lock_guard{event_queue_lock_};
event_queue_.emplace(std::make_pair(name, value));
}
@@ -55,7 +53,6 @@
}
void ActionManager::QueueBuiltinAction(BuiltinFunction func, const std::string& name) {
- auto lock = std::lock_guard{event_queue_lock_};
auto action = std::make_unique<Action>(true, nullptr, "<Builtin Action>", 0, name,
std::map<std::string, std::string>{});
action->AddCommand(std::move(func), {name}, 0);
@@ -65,18 +62,15 @@
}
void ActionManager::ExecuteOneCommand() {
- {
- auto lock = std::lock_guard{event_queue_lock_};
- // Loop through the event queue until we have an action to execute
- while (current_executing_actions_.empty() && !event_queue_.empty()) {
- for (const auto& action : actions_) {
- if (std::visit([&action](const auto& event) { return action->CheckEvent(event); },
- event_queue_.front())) {
- current_executing_actions_.emplace(action.get());
- }
+ // Loop through the event queue until we have an action to execute
+ while (current_executing_actions_.empty() && !event_queue_.empty()) {
+ for (const auto& action : actions_) {
+ if (std::visit([&action](const auto& event) { return action->CheckEvent(event); },
+ event_queue_.front())) {
+ current_executing_actions_.emplace(action.get());
}
- event_queue_.pop();
}
+ event_queue_.pop();
}
if (current_executing_actions_.empty()) {
@@ -109,7 +103,6 @@
}
bool ActionManager::HasMoreCommands() const {
- auto lock = std::lock_guard{event_queue_lock_};
return !current_executing_actions_.empty() || !event_queue_.empty();
}
@@ -120,7 +113,6 @@
}
void ActionManager::ClearQueue() {
- auto lock = std::lock_guard{event_queue_lock_};
// We are shutting down so don't claim the oneshot builtin actions back
current_executing_actions_ = {};
event_queue_ = {};
diff --git a/init/action_manager.h b/init/action_manager.h
index b6f93d9..a2b95ac 100644
--- a/init/action_manager.h
+++ b/init/action_manager.h
@@ -16,12 +16,9 @@
#pragma once
-#include <mutex>
#include <string>
#include <vector>
-#include <android-base/thread_annotations.h>
-
#include "action.h"
#include "builtins.h"
@@ -51,9 +48,7 @@
void operator=(ActionManager const&) = delete;
std::vector<std::unique_ptr<Action>> actions_;
- std::queue<std::variant<EventTrigger, PropertyChange, BuiltinAction>> event_queue_
- GUARDED_BY(event_queue_lock_);
- mutable std::mutex event_queue_lock_;
+ std::queue<std::variant<EventTrigger, PropertyChange, BuiltinAction>> event_queue_;
std::queue<const Action*> current_executing_actions_;
std::size_t current_command_;
};
diff --git a/init/builtins.cpp b/init/builtins.cpp
index dd5af72..200bfff 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -151,7 +151,6 @@
template <typename F>
static void ForEachServiceInClass(const std::string& classname, F function) {
- auto lock = std::lock_guard{service_lock};
for (const auto& service : ServiceList::GetInstance()) {
if (service->classnames().count(classname)) std::invoke(function, service);
}
@@ -163,7 +162,6 @@
return {};
// Starting a class does not start services which are explicitly disabled.
// They must be started individually.
- auto lock = std::lock_guard{service_lock};
for (const auto& service : ServiceList::GetInstance()) {
if (service->classnames().count(args[1])) {
if (auto result = service->StartIfNotDisabled(); !result.ok()) {
@@ -186,7 +184,6 @@
// stopped either.
return {};
}
- auto lock = std::lock_guard{service_lock};
for (const auto& service : ServiceList::GetInstance()) {
if (service->classnames().count(args[1])) {
if (auto result = service->StartIfPostData(); !result.ok()) {
@@ -237,7 +234,6 @@
}
static Result<void> do_enable(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindService(args[1]);
if (!svc) return Error() << "Could not find service";
@@ -249,7 +245,6 @@
}
static Result<void> do_exec(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
auto service = Service::MakeTemporaryOneshotService(args.args);
if (!service.ok()) {
return Error() << "Could not create exec service: " << service.error();
@@ -263,7 +258,6 @@
}
static Result<void> do_exec_background(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
auto service = Service::MakeTemporaryOneshotService(args.args);
if (!service.ok()) {
return Error() << "Could not create exec background service: " << service.error();
@@ -277,7 +271,6 @@
}
static Result<void> do_exec_start(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* service = ServiceList::GetInstance().FindService(args[1]);
if (!service) {
return Error() << "Service not found";
@@ -347,7 +340,6 @@
}
static Result<void> do_interface_restart(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindInterface(args[1]);
if (!svc) return Error() << "interface " << args[1] << " not found";
svc->Restart();
@@ -355,7 +347,6 @@
}
static Result<void> do_interface_start(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindInterface(args[1]);
if (!svc) return Error() << "interface " << args[1] << " not found";
if (auto result = svc->Start(); !result.ok()) {
@@ -365,7 +356,6 @@
}
static Result<void> do_interface_stop(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindInterface(args[1]);
if (!svc) return Error() << "interface " << args[1] << " not found";
svc->Stop();
@@ -750,7 +740,6 @@
}
static Result<void> do_start(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindService(args[1]);
if (!svc) return Error() << "service " << args[1] << " not found";
if (auto result = svc->Start(); !result.ok()) {
@@ -760,7 +749,6 @@
}
static Result<void> do_stop(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindService(args[1]);
if (!svc) return Error() << "service " << args[1] << " not found";
svc->Stop();
@@ -768,7 +756,6 @@
}
static Result<void> do_restart(const BuiltinArguments& args) {
- auto lock = std::lock_guard{service_lock};
Service* svc = ServiceList::GetInstance().FindService(args[1]);
if (!svc) return Error() << "service " << args[1] << " not found";
svc->Restart();
@@ -1124,7 +1111,6 @@
function(StringPrintf("Exec service failed, status %d", siginfo.si_status));
}
});
- auto lock = std::lock_guard{service_lock};
if (auto result = (*service)->ExecStart(); !result.ok()) {
function("ExecStart failed: " + result.error().message());
}
@@ -1264,7 +1250,6 @@
}
success &= parser.ParseConfigFile(c);
}
- auto lock = std::lock_guard{service_lock};
ServiceList::GetInstance().MarkServicesUpdate();
if (success) {
return {};
diff --git a/init/init.cpp b/init/init.cpp
index b0f929c..a7518fc 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -33,9 +33,7 @@
#include <functional>
#include <map>
#include <memory>
-#include <mutex>
#include <optional>
-#include <thread>
#include <vector>
#include <android-base/chrono_utils.h>
@@ -97,148 +95,15 @@
static int signal_fd = -1;
static int property_fd = -1;
+static std::unique_ptr<Timer> waiting_for_prop(nullptr);
+static std::string wait_prop_name;
+static std::string wait_prop_value;
+static std::string shutdown_command;
+static bool do_shutdown = false;
+
static std::unique_ptr<Subcontext> subcontext;
-// Init epolls various FDs to wait for various inputs. It previously waited on property changes
-// with a blocking socket that contained the information related to the change, however, it was easy
-// to fill that socket and deadlock the system. Now we use locks to handle the property changes
-// directly in the property thread, however we still must wake the epoll to inform init that there
-// is a change to process, so we use this FD. It is non-blocking, since we do not care how many
-// times WakeEpoll() is called, only that the epoll will wake.
-static int wake_epoll_fd = -1;
-static void InstallInitNotifier(Epoll* epoll) {
- int sockets[2];
- if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, sockets) != 0) {
- PLOG(FATAL) << "Failed to socketpair() between property_service and init";
- }
- int epoll_fd = sockets[0];
- wake_epoll_fd = sockets[1];
-
- auto drain_socket = [epoll_fd] {
- char buf[512];
- while (read(epoll_fd, buf, sizeof(buf)) > 0) {
- }
- };
-
- if (auto result = epoll->RegisterHandler(epoll_fd, drain_socket); !result) {
- LOG(FATAL) << result.error();
- }
-}
-
-static void WakeEpoll() {
- constexpr char value[] = "1";
- write(wake_epoll_fd, value, sizeof(value));
-}
-
-static class PropWaiterState {
- public:
- bool StartWaiting(const char* name, const char* value) {
- auto lock = std::lock_guard{lock_};
- if (waiting_for_prop_) {
- return false;
- }
- if (GetProperty(name, "") != value) {
- // Current property value is not equal to expected value
- wait_prop_name_ = name;
- wait_prop_value_ = value;
- waiting_for_prop_.reset(new Timer());
- } else {
- LOG(INFO) << "start_waiting_for_property(\"" << name << "\", \"" << value
- << "\"): already set";
- }
- return true;
- }
-
- void ResetWaitForProp() {
- auto lock = std::lock_guard{lock_};
- ResetWaitForPropLocked();
- }
-
- void CheckAndResetWait(const std::string& name, const std::string& value) {
- auto lock = std::lock_guard{lock_};
- // We always record how long init waited for ueventd to tell us cold boot finished.
- // If we aren't waiting on this property, it means that ueventd finished before we even
- // started to wait.
- if (name == kColdBootDoneProp) {
- auto time_waited = waiting_for_prop_ ? waiting_for_prop_->duration().count() : 0;
- std::thread([time_waited] {
- SetProperty("ro.boottime.init.cold_boot_wait", std::to_string(time_waited));
- }).detach();
- }
-
- if (waiting_for_prop_) {
- if (wait_prop_name_ == name && wait_prop_value_ == value) {
- LOG(INFO) << "Wait for property '" << wait_prop_name_ << "=" << wait_prop_value_
- << "' took " << *waiting_for_prop_;
- ResetWaitForPropLocked();
- WakeEpoll();
- }
- }
- }
-
- // This is not thread safe because it releases the lock when it returns, so the waiting state
- // may change. However, we only use this function to prevent running commands in the main
- // thread loop when we are waiting, so we do not care about false positives; only false
- // negatives. StartWaiting() and this function are always called from the same thread, so false
- // negatives are not possible and therefore we're okay.
- bool MightBeWaiting() {
- auto lock = std::lock_guard{lock_};
- return static_cast<bool>(waiting_for_prop_);
- }
-
- private:
- void ResetWaitForPropLocked() {
- wait_prop_name_.clear();
- wait_prop_value_.clear();
- waiting_for_prop_.reset();
- }
-
- std::mutex lock_;
- std::unique_ptr<Timer> waiting_for_prop_{nullptr};
- std::string wait_prop_name_;
- std::string wait_prop_value_;
-
-} prop_waiter_state;
-
-bool start_waiting_for_property(const char* name, const char* value) {
- return prop_waiter_state.StartWaiting(name, value);
-}
-
-void ResetWaitForProp() {
- prop_waiter_state.ResetWaitForProp();
-}
-
-static class ShutdownState {
- public:
- void TriggerShutdown(const std::string& command) {
- // We can't call HandlePowerctlMessage() directly in this function,
- // because it modifies the contents of the action queue, which can cause the action queue
- // to get into a bad state if this function is called from a command being executed by the
- // action queue. Instead we set this flag and ensure that shutdown happens before the next
- // command is run in the main init loop.
- auto lock = std::lock_guard{shutdown_command_lock_};
- shutdown_command_ = command;
- do_shutdown_ = true;
- WakeEpoll();
- }
-
- std::optional<std::string> CheckShutdown() {
- auto lock = std::lock_guard{shutdown_command_lock_};
- if (do_shutdown_ && !IsShuttingDown()) {
- do_shutdown_ = false;
- return shutdown_command_;
- }
- return {};
- }
-
- private:
- std::mutex shutdown_command_lock_;
- std::string shutdown_command_;
- bool do_shutdown_ = false;
-} shutdown_state;
-
void DumpState() {
- auto lock = std::lock_guard{service_lock};
ServiceList::GetInstance().DumpState();
ActionManager::GetInstance().DumpState();
}
@@ -291,7 +156,39 @@
}
}
-void PropertyChanged(const std::string& name, const std::string& value) {
+bool start_waiting_for_property(const char* name, const char* value) {
+ if (waiting_for_prop) {
+ return false;
+ }
+ if (GetProperty(name, "") != value) {
+ // Current property value is not equal to expected value
+ wait_prop_name = name;
+ wait_prop_value = value;
+ waiting_for_prop.reset(new Timer());
+ } else {
+ LOG(INFO) << "start_waiting_for_property(\"" << name << "\", \"" << value
+ << "\"): already set";
+ }
+ return true;
+}
+
+void ResetWaitForProp() {
+ wait_prop_name.clear();
+ wait_prop_value.clear();
+ waiting_for_prop.reset();
+}
+
+static void TriggerShutdown(const std::string& command) {
+ // We can't call HandlePowerctlMessage() directly in this function,
+ // because it modifies the contents of the action queue, which can cause the action queue
+ // to get into a bad state if this function is called from a command being executed by the
+ // action queue. Instead we set this flag and ensure that shutdown happens before the next
+ // command is run in the main init loop.
+ shutdown_command = command;
+ do_shutdown = true;
+}
+
+void property_changed(const std::string& name, const std::string& value) {
// If the property is sys.powerctl, we bypass the event queue and immediately handle it.
// This is to ensure that init will always and immediately shutdown/reboot, regardless of
// if there are other pending events to process or if init is waiting on an exec service or
@@ -299,20 +196,30 @@
// In non-thermal-shutdown case, 'shutdown' trigger will be fired to let device specific
// commands to be executed.
if (name == "sys.powerctl") {
- trigger_shutdown(value);
+ TriggerShutdown(value);
}
- if (property_triggers_enabled) {
- ActionManager::GetInstance().QueuePropertyChange(name, value);
- WakeEpoll();
+ if (property_triggers_enabled) ActionManager::GetInstance().QueuePropertyChange(name, value);
+
+ // We always record how long init waited for ueventd to tell us cold boot finished.
+ // If we aren't waiting on this property, it means that ueventd finished before we even started
+ // to wait.
+ if (name == kColdBootDoneProp) {
+ auto time_waited = waiting_for_prop ? waiting_for_prop->duration().count() : 0;
+ SetProperty("ro.boottime.init.cold_boot_wait", std::to_string(time_waited));
}
- prop_waiter_state.CheckAndResetWait(name, value);
+ if (waiting_for_prop) {
+ if (wait_prop_name == name && wait_prop_value == value) {
+ LOG(INFO) << "Wait for property '" << wait_prop_name << "=" << wait_prop_value
+ << "' took " << *waiting_for_prop;
+ ResetWaitForProp();
+ }
+ }
}
static std::optional<boot_clock::time_point> HandleProcessActions() {
std::optional<boot_clock::time_point> next_process_action_time;
- auto lock = std::lock_guard{service_lock};
for (const auto& s : ServiceList::GetInstance()) {
if ((s->flags() & SVC_RUNNING) && s->timeout_period()) {
auto timeout_time = s->time_started() + *s->timeout_period();
@@ -341,7 +248,7 @@
return next_process_action_time;
}
-static Result<void> DoControlStart(Service* service) REQUIRES(service_lock) {
+static Result<void> DoControlStart(Service* service) {
return service->Start();
}
@@ -350,7 +257,7 @@
return {};
}
-static Result<void> DoControlRestart(Service* service) REQUIRES(service_lock) {
+static Result<void> DoControlRestart(Service* service) {
service->Restart();
return {};
}
@@ -384,7 +291,7 @@
return control_message_functions;
}
-bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t from_pid) {
+bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t pid) {
const auto& map = get_control_message_map();
const auto it = map.find(msg);
@@ -393,7 +300,7 @@
return false;
}
- std::string cmdline_path = StringPrintf("proc/%d/cmdline", from_pid);
+ std::string cmdline_path = StringPrintf("proc/%d/cmdline", pid);
std::string process_cmdline;
if (ReadFileToString(cmdline_path, &process_cmdline)) {
std::replace(process_cmdline.begin(), process_cmdline.end(), '\0', ' ');
@@ -404,8 +311,6 @@
const ControlMessageFunction& function = it->second;
- auto lock = std::lock_guard{service_lock};
-
Service* svc = nullptr;
switch (function.target) {
@@ -423,24 +328,23 @@
if (svc == nullptr) {
LOG(ERROR) << "Control message: Could not find '" << name << "' for ctl." << msg
- << " from pid: " << from_pid << " (" << process_cmdline << ")";
+ << " from pid: " << pid << " (" << process_cmdline << ")";
return false;
}
if (auto result = function.action(svc); !result.ok()) {
LOG(ERROR) << "Control message: Could not ctl." << msg << " for '" << name
- << "' from pid: " << from_pid << " (" << process_cmdline
- << "): " << result.error();
+ << "' from pid: " << pid << " (" << process_cmdline << "): " << result.error();
return false;
}
LOG(INFO) << "Control message: Processed ctl." << msg << " for '" << name
- << "' from pid: " << from_pid << " (" << process_cmdline << ")";
+ << "' from pid: " << pid << " (" << process_cmdline << ")";
return true;
}
static Result<void> wait_for_coldboot_done_action(const BuiltinArguments& args) {
- if (!prop_waiter_state.StartWaiting(kColdBootDoneProp, "true")) {
+ if (!start_waiting_for_property(kColdBootDoneProp, "true")) {
LOG(FATAL) << "Could not wait for '" << kColdBootDoneProp << "'";
}
@@ -588,7 +492,6 @@
}
auto found = false;
- auto lock = std::lock_guard{service_lock};
for (const auto& service : ServiceList::GetInstance()) {
auto svc = service.get();
if (svc->keycodes() == keycodes) {
@@ -675,6 +578,44 @@
}
}
+static void HandlePropertyFd() {
+ auto message = ReadMessage(property_fd);
+ if (!message.ok()) {
+ LOG(ERROR) << "Could not read message from property service: " << message.error();
+ return;
+ }
+
+ auto property_message = PropertyMessage{};
+ if (!property_message.ParseFromString(*message)) {
+ LOG(ERROR) << "Could not parse message from property service";
+ return;
+ }
+
+ switch (property_message.msg_case()) {
+ case PropertyMessage::kControlMessage: {
+ auto& control_message = property_message.control_message();
+ bool success = HandleControlMessage(control_message.msg(), control_message.name(),
+ control_message.pid());
+
+ uint32_t response = success ? PROP_SUCCESS : PROP_ERROR_HANDLE_CONTROL_MESSAGE;
+ if (control_message.has_fd()) {
+ int fd = control_message.fd();
+ TEMP_FAILURE_RETRY(send(fd, &response, sizeof(response), 0));
+ close(fd);
+ }
+ break;
+ }
+ case PropertyMessage::kChangedMessage: {
+ auto& changed_message = property_message.changed_message();
+ property_changed(changed_message.name(), changed_message.value());
+ break;
+ }
+ default:
+ LOG(ERROR) << "Unknown message type from property service: "
+ << property_message.msg_case();
+ }
+}
+
int SecondStageMain(int argc, char** argv) {
if (REBOOT_BOOTLOADER_ON_PANIC) {
InstallRebootSignalHandlers();
@@ -682,7 +623,7 @@
boot_clock::time_point start_time = boot_clock::now();
- trigger_shutdown = [](const std::string& command) { shutdown_state.TriggerShutdown(command); };
+ trigger_shutdown = TriggerShutdown;
SetStdioToDevNull(argv);
InitKernelLogging(argv);
@@ -742,8 +683,11 @@
}
InstallSignalFdHandler(&epoll);
- InstallInitNotifier(&epoll);
+
StartPropertyService(&property_fd);
+ if (auto result = epoll.RegisterHandler(property_fd, HandlePropertyFd); !result.ok()) {
+ LOG(FATAL) << "Could not register epoll handler for property fd: " << result.error();
+ }
// Make the time that init stages started available for bootstat to log.
RecordStageBoottimes(start_time);
@@ -796,7 +740,6 @@
Keychords keychords;
am.QueueBuiltinAction(
[&epoll, &keychords](const BuiltinArguments& args) -> Result<void> {
- auto lock = std::lock_guard{service_lock};
for (const auto& svc : ServiceList::GetInstance()) {
keychords.Register(svc->keycodes());
}
@@ -827,12 +770,12 @@
// By default, sleep until something happens.
auto epoll_timeout = std::optional<std::chrono::milliseconds>{};
- auto shutdown_command = shutdown_state.CheckShutdown();
- if (shutdown_command) {
- HandlePowerctlMessage(*shutdown_command);
+ if (do_shutdown && !IsShuttingDown()) {
+ do_shutdown = false;
+ HandlePowerctlMessage(shutdown_command);
}
- if (!(prop_waiter_state.MightBeWaiting() || Service::is_exec_service_running())) {
+ if (!(waiting_for_prop || Service::is_exec_service_running())) {
am.ExecuteOneCommand();
}
if (!IsShuttingDown()) {
@@ -846,7 +789,7 @@
}
}
- if (!(prop_waiter_state.MightBeWaiting() || Service::is_exec_service_running())) {
+ if (!(waiting_for_prop || Service::is_exec_service_running())) {
// If there's more work to do, wake up again immediately.
if (am.HasMoreCommands()) epoll_timeout = 0ms;
}
diff --git a/init/init.h b/init/init.h
index bcf24e7..4bbca6f 100644
--- a/init/init.h
+++ b/init/init.h
@@ -41,9 +41,6 @@
void SendStopSendingMessagesMessage();
void SendStartSendingMessagesMessage();
-void PropertyChanged(const std::string& name, const std::string& value);
-bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t from_pid);
-
int SecondStageMain(int argc, char** argv);
} // namespace init
diff --git a/init/init_test.cpp b/init/init_test.cpp
index 3053bd8..caf3e03 100644
--- a/init/init_test.cpp
+++ b/init/init_test.cpp
@@ -167,7 +167,6 @@
ServiceList service_list;
TestInitText(init_script, BuiltinFunctionMap(), {}, &service_list);
- auto lock = std::lock_guard{service_lock};
ASSERT_EQ(1, std::distance(service_list.begin(), service_list.end()));
auto service = service_list.begin()->get();
diff --git a/init/lmkd_service.cpp b/init/lmkd_service.cpp
index a531d0a..dd1ab4d 100644
--- a/init/lmkd_service.cpp
+++ b/init/lmkd_service.cpp
@@ -79,8 +79,7 @@
}
static void RegisterServices(pid_t exclude_pid) {
- auto lock = std::lock_guard{service_lock};
- for (const auto& service : ServiceList::GetInstance()) {
+ for (const auto& service : ServiceList::GetInstance().services()) {
auto svc = service.get();
if (svc->oom_score_adjust() != DEFAULT_OOM_SCORE_ADJUST) {
// skip if process is excluded or not yet forked (pid==0)
diff --git a/init/mount_namespace.cpp b/init/mount_namespace.cpp
index aa36849..0749fe3 100644
--- a/init/mount_namespace.cpp
+++ b/init/mount_namespace.cpp
@@ -29,7 +29,6 @@
#include <android-base/unique_fd.h>
#include <apex_manifest.pb.h>
-#include "property_service.h"
#include "util.h"
namespace android {
@@ -291,14 +290,6 @@
return true;
}
if (default_ns_id != GetMountNamespaceId()) {
- // The property service thread and its descendent threads must be in the correct mount
- // namespace to call Service::Start(), however setns() only operates on a single thread and
- // fails when secondary threads attempt to join the same mount namespace. Therefore, we
- // must join the property service thread and its descendents before the setns() call. Those
- // threads are then started again after the setns() call, and they'll be in the proper
- // namespace.
- PausePropertyService();
-
if (setns(default_ns_fd.get(), CLONE_NEWNS) == -1) {
PLOG(ERROR) << "Failed to switch back to the default mount namespace.";
return false;
@@ -308,8 +299,6 @@
LOG(ERROR) << result.error();
return false;
}
-
- ResumePropertyService();
}
LOG(INFO) << "Switched to default mount namespace";
diff --git a/init/property_service.cpp b/init/property_service.cpp
index 319a241..730bf6d 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -92,10 +92,8 @@
static bool persistent_properties_loaded = false;
static int property_set_fd = -1;
-static int from_init_socket = -1;
static int init_socket = -1;
static bool accept_messages = false;
-static std::thread property_service_thread;
static PropertyInfoAreaFile property_info_area;
@@ -149,6 +147,17 @@
return has_access;
}
+static void SendPropertyChanged(const std::string& name, const std::string& value) {
+ auto property_msg = PropertyMessage{};
+ auto* changed_message = property_msg.mutable_changed_message();
+ changed_message->set_name(name);
+ changed_message->set_value(value);
+
+ if (auto result = SendMessage(init_socket, property_msg); !result.ok()) {
+ LOG(ERROR) << "Failed to send property changed message: " << result.error();
+ }
+}
+
static uint32_t PropertySet(const std::string& name, const std::string& value, std::string* error) {
size_t valuelen = value.size();
@@ -187,137 +196,47 @@
// If init hasn't started its main loop, then it won't be handling property changed messages
// anyway, so there's no need to try to send them.
if (accept_messages) {
- PropertyChanged(name, value);
+ SendPropertyChanged(name, value);
}
return PROP_SUCCESS;
}
-template <typename T>
-class SingleThreadExecutor {
+class AsyncRestorecon {
public:
- virtual ~SingleThreadExecutor() {}
-
- template <typename F = T>
- void Run(F&& item) {
+ void TriggerRestorecon(const std::string& path) {
auto guard = std::lock_guard{mutex_};
- items_.emplace(std::forward<F>(item));
+ paths_.emplace(path);
- if (thread_state_ == ThreadState::kRunning || thread_state_ == ThreadState::kStopped) {
- return;
- }
-
- if (thread_state_ == ThreadState::kPendingJoin) {
- thread_.join();
- }
-
- StartThread();
- }
-
- void StopAndJoin() {
- auto lock = std::unique_lock{mutex_};
- if (thread_state_ == ThreadState::kPendingJoin) {
- thread_.join();
- } else if (thread_state_ == ThreadState::kRunning) {
- thread_state_ = ThreadState::kStopped;
- lock.unlock();
- thread_.join();
- lock.lock();
- }
-
- thread_state_ = ThreadState::kStopped;
- }
-
- void Restart() {
- auto guard = std::lock_guard{mutex_};
- if (items_.empty()) {
- thread_state_ = ThreadState::kNotStarted;
- } else {
- StartThread();
- }
- }
-
- void MaybeJoin() {
- auto guard = std::lock_guard{mutex_};
- if (thread_state_ == ThreadState::kPendingJoin) {
- thread_.join();
- thread_state_ = ThreadState::kNotStarted;
+ if (!thread_started_) {
+ thread_started_ = true;
+ std::thread{&AsyncRestorecon::ThreadFunction, this}.detach();
}
}
private:
- virtual void Execute(T&& item) = 0;
-
- void StartThread() {
- thread_state_ = ThreadState::kRunning;
- auto thread = std::thread{&SingleThreadExecutor::ThreadFunction, this};
- std::swap(thread_, thread);
- }
-
void ThreadFunction() {
auto lock = std::unique_lock{mutex_};
- while (!items_.empty()) {
- auto item = items_.front();
- items_.pop();
+ while (!paths_.empty()) {
+ auto path = paths_.front();
+ paths_.pop();
lock.unlock();
- Execute(std::move(item));
+ if (selinux_android_restorecon(path.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE) != 0) {
+ LOG(ERROR) << "Asynchronous restorecon of '" << path << "' failed'";
+ }
+ android::base::SetProperty(kRestoreconProperty, path);
lock.lock();
}
- if (thread_state_ != ThreadState::kStopped) {
- thread_state_ = ThreadState::kPendingJoin;
- }
+ thread_started_ = false;
}
std::mutex mutex_;
- std::queue<T> items_;
- enum class ThreadState {
- kNotStarted, // Initial state when starting the program or when restarting with no items to
- // process.
- kRunning, // The thread is running and is in a state that it will process new items if
- // are run.
- kPendingJoin, // The thread has run to completion and is pending join(). A new thread must
- // be launched for new items to be processed.
- kStopped, // This executor has stopped and will not process more items until Restart() is
- // called. Currently pending items will be processed and the thread will be
- // joined.
- };
- ThreadState thread_state_ = ThreadState::kNotStarted;
- std::thread thread_;
+ std::queue<std::string> paths_;
+ bool thread_started_ = false;
};
-class RestoreconThread : public SingleThreadExecutor<std::string> {
- virtual void Execute(std::string&& path) override {
- if (selinux_android_restorecon(path.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE) != 0) {
- LOG(ERROR) << "Asynchronous restorecon of '" << path << "' failed'";
- }
- android::base::SetProperty(kRestoreconProperty, path);
- }
-};
-
-struct ControlMessageInfo {
- std::string message;
- std::string name;
- pid_t pid;
- int fd;
-};
-
-class ControlMessageThread : public SingleThreadExecutor<ControlMessageInfo> {
- virtual void Execute(ControlMessageInfo&& info) override {
- bool success = HandleControlMessage(info.message, info.name, info.pid);
-
- uint32_t response = success ? PROP_SUCCESS : PROP_ERROR_HANDLE_CONTROL_MESSAGE;
- if (info.fd != -1) {
- TEMP_FAILURE_RETRY(send(info.fd, &response, sizeof(response), 0));
- close(info.fd);
- }
- }
-};
-
-static RestoreconThread restorecon_thread;
-static ControlMessageThread control_message_thread;
-
class SocketConnection {
public:
SocketConnection(int socket, const ucred& cred) : socket_(socket), cred_(cred) {}
@@ -459,17 +378,29 @@
return PROP_ERROR_HANDLE_CONTROL_MESSAGE;
}
- // We must release the fd before spawning the thread, otherwise there will be a race with the
- // thread. If the thread calls close() before this function calls Release(), then fdsan will see
- // the wrong tag and abort().
+ auto property_msg = PropertyMessage{};
+ auto* control_message = property_msg.mutable_control_message();
+ control_message->set_msg(msg);
+ control_message->set_name(name);
+ control_message->set_pid(pid);
+
+ // We must release the fd before sending it to init, otherwise there will be a race with init.
+ // If init calls close() before Release(), then fdsan will see the wrong tag and abort().
int fd = -1;
if (socket != nullptr && SelinuxGetVendorAndroidVersion() > __ANDROID_API_Q__) {
fd = socket->Release();
+ control_message->set_fd(fd);
}
- // Handling a control message likely calls SetProperty, which we must synchronously handle,
- // therefore we must fork a thread to handle it.
- control_message_thread.Run({msg, name, pid, fd});
+ if (auto result = SendMessage(init_socket, property_msg); !result.ok()) {
+ // We've already released the fd above, so if we fail to send the message to init, we need
+ // to manually free it here.
+ if (fd != -1) {
+ close(fd);
+ }
+ *error = "Failed to send control message: " + result.error().message();
+ return PROP_ERROR_HANDLE_CONTROL_MESSAGE;
+ }
return PROP_SUCCESS;
}
@@ -571,7 +502,8 @@
// We use a thread to do this restorecon operation to prevent holding up init, as it may take
// a long time to complete.
if (name == kRestoreconProperty && cr.pid != 1 && !value.empty()) {
- restorecon_thread.Run(value);
+ static AsyncRestorecon async_restorecon;
+ async_restorecon.TriggerRestorecon(value);
return PROP_SUCCESS;
}
@@ -1152,8 +1084,6 @@
PropertyLoadBootDefaults();
}
-static bool pause_property_service = false;
-
static void HandleInitSocket() {
auto message = ReadMessage(init_socket);
if (!message.ok()) {
@@ -1188,10 +1118,6 @@
accept_messages = true;
break;
}
- case InitMessage::kPausePropertyService: {
- pause_property_service = true;
- break;
- }
default:
LOG(ERROR) << "Unknown message type from init: " << init_message.msg_case();
}
@@ -1212,7 +1138,7 @@
LOG(FATAL) << result.error();
}
- while (!pause_property_service) {
+ while (true) {
auto pending_functions = epoll.Wait(std::nullopt);
if (!pending_functions.ok()) {
LOG(ERROR) << pending_functions.error();
@@ -1221,34 +1147,9 @@
(*function)();
}
}
- control_message_thread.MaybeJoin();
- restorecon_thread.MaybeJoin();
}
}
-void SendStopPropertyServiceMessage() {
- auto init_message = InitMessage{};
- init_message.set_pause_property_service(true);
- if (auto result = SendMessage(from_init_socket, init_message); !result.ok()) {
- LOG(ERROR) << "Failed to send stop property service message: " << result.error();
- }
-}
-
-void PausePropertyService() {
- control_message_thread.StopAndJoin();
- restorecon_thread.StopAndJoin();
- SendStopPropertyServiceMessage();
- property_service_thread.join();
-}
-
-void ResumePropertyService() {
- pause_property_service = false;
- auto new_thread = std::thread{PropertyServiceThread};
- property_service_thread.swap(new_thread);
- restorecon_thread.Restart();
- control_message_thread.Restart();
-}
-
void StartPropertyService(int* epoll_socket) {
InitPropertySet("ro.property_service.version", "2");
@@ -1256,7 +1157,7 @@
if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sockets) != 0) {
PLOG(FATAL) << "Failed to socketpair() between property_service and init";
}
- *epoll_socket = from_init_socket = sockets[0];
+ *epoll_socket = sockets[0];
init_socket = sockets[1];
accept_messages = true;
@@ -1270,8 +1171,7 @@
listen(property_set_fd, 8);
- auto new_thread = std::thread{PropertyServiceThread};
- property_service_thread.swap(new_thread);
+ std::thread{PropertyServiceThread}.detach();
}
} // namespace init
diff --git a/init/property_service.h b/init/property_service.h
index e921326..506d116 100644
--- a/init/property_service.h
+++ b/init/property_service.h
@@ -31,8 +31,6 @@
void PropertyInit();
void StartPropertyService(int* epoll_socket);
-void ResumePropertyService();
-void PausePropertyService();
} // namespace init
} // namespace android
diff --git a/init/property_service.proto b/init/property_service.proto
index 36245b2..08268d9 100644
--- a/init/property_service.proto
+++ b/init/property_service.proto
@@ -41,6 +41,5 @@
bool load_persistent_properties = 1;
bool stop_sending_messages = 2;
bool start_sending_messages = 3;
- bool pause_property_service = 4;
};
}
diff --git a/init/reboot.cpp b/init/reboot.cpp
index cad192d..8d8bd8e 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -85,7 +85,7 @@
static const std::set<std::string> kDebuggingServices{"tombstoned", "logd", "adbd", "console"};
-static std::vector<Service*> GetDebuggingServices(bool only_post_data) REQUIRES(service_lock) {
+static std::vector<Service*> GetDebuggingServices(bool only_post_data) {
std::vector<Service*> ret;
ret.reserve(kDebuggingServices.size());
for (const auto& s : ServiceList::GetInstance()) {
@@ -181,7 +181,7 @@
};
// Turn off backlight while we are performing power down cleanup activities.
-static void TurnOffBacklight() REQUIRES(service_lock) {
+static void TurnOffBacklight() {
Service* service = ServiceList::GetInstance().FindService("blank_screen");
if (service == nullptr) {
LOG(WARNING) << "cannot find blank_screen in TurnOffBacklight";
@@ -589,7 +589,6 @@
// Start reboot monitor thread
sem_post(&reboot_semaphore);
- auto lock = std::lock_guard{service_lock};
// watchdogd is a vendor specific component but should be alive to complete shutdown safely.
const std::set<std::string> to_starts{"watchdogd"};
std::vector<Service*> stop_first;
@@ -709,7 +708,6 @@
// Skip wait for prop if it is in progress
ResetWaitForProp();
// Clear EXEC flag if there is one pending
- auto lock = std::lock_guard{service_lock};
for (const auto& s : ServiceList::GetInstance()) {
s->UnSetExec();
}
@@ -753,7 +751,6 @@
return Error() << "Failed to set sys.init.userspace_reboot.in_progress property";
}
EnterShutdown();
- auto lock = std::lock_guard{service_lock};
if (!SetProperty("sys.powerctl", "")) {
return Error() << "Failed to reset sys.powerctl property";
}
@@ -914,7 +911,6 @@
run_fsck = true;
} else if (cmd_params[1] == "thermal") {
// Turn off sources of heat immediately.
- auto lock = std::lock_guard{service_lock};
TurnOffBacklight();
// run_fsck is false to avoid delay
cmd = ANDROID_RB_THERMOFF;
diff --git a/init/service.h b/init/service.h
index d2a4462..cf3f0c2 100644
--- a/init/service.h
+++ b/init/service.h
@@ -27,14 +27,12 @@
#include <vector>
#include <android-base/chrono_utils.h>
-#include <android-base/thread_annotations.h>
#include <cutils/iosched_policy.h>
#include "action.h"
#include "capabilities.h"
#include "keyword_map.h"
#include "parser.h"
-#include "service_lock.h"
#include "service_utils.h"
#include "subcontext.h"
@@ -79,17 +77,17 @@
bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; }
bool IsEnabled() { return (flags_ & SVC_DISABLED) == 0; }
- Result<void> ExecStart() REQUIRES(service_lock);
- Result<void> Start() REQUIRES(service_lock);
- Result<void> StartIfNotDisabled() REQUIRES(service_lock);
- Result<void> StartIfPostData() REQUIRES(service_lock);
- Result<void> Enable() REQUIRES(service_lock);
+ Result<void> ExecStart();
+ Result<void> Start();
+ Result<void> StartIfNotDisabled();
+ Result<void> StartIfPostData();
+ Result<void> Enable();
void Reset();
void ResetIfPostData();
void Stop();
void Terminate();
void Timeout();
- void Restart() REQUIRES(service_lock);
+ void Restart();
void Reap(const siginfo_t& siginfo);
void DumpState() const;
void SetShutdownCritical() { flags_ |= SVC_SHUTDOWN_CRITICAL; }
diff --git a/init/service_list.h b/init/service_list.h
index 280a228..3b9018b 100644
--- a/init/service_list.h
+++ b/init/service_list.h
@@ -17,13 +17,9 @@
#pragma once
#include <memory>
-#include <mutex>
#include <vector>
-#include <android-base/thread_annotations.h>
-
#include "service.h"
-#include "service_lock.h"
namespace android {
namespace init {
@@ -36,16 +32,16 @@
ServiceList();
size_t CheckAllCommands();
- void AddService(std::unique_ptr<Service> service) REQUIRES(service_lock);
- void RemoveService(const Service& svc) REQUIRES(service_lock);
+ void AddService(std::unique_ptr<Service> service);
+ void RemoveService(const Service& svc);
template <class UnaryPredicate>
- void RemoveServiceIf(UnaryPredicate predicate) REQUIRES(service_lock) {
+ void RemoveServiceIf(UnaryPredicate predicate) {
services_.erase(std::remove_if(services_.begin(), services_.end(), predicate),
services_.end());
}
template <typename T, typename F = decltype(&Service::name)>
- Service* FindService(T value, F function = &Service::name) const REQUIRES(service_lock) {
+ Service* FindService(T value, F function = &Service::name) const {
auto svc = std::find_if(services_.begin(), services_.end(),
[&function, &value](const std::unique_ptr<Service>& s) {
return std::invoke(function, s) == value;
@@ -56,7 +52,7 @@
return nullptr;
}
- Service* FindInterface(const std::string& interface_name) REQUIRES(service_lock) {
+ Service* FindInterface(const std::string& interface_name) {
for (const auto& svc : services_) {
if (svc->interfaces().count(interface_name) > 0) {
return svc.get();
@@ -66,20 +62,18 @@
return nullptr;
}
- void DumpState() const REQUIRES(service_lock);
+ void DumpState() const;
- auto begin() const REQUIRES(service_lock) { return services_.begin(); }
- auto end() const REQUIRES(service_lock) { return services_.end(); }
- const std::vector<std::unique_ptr<Service>>& services() const REQUIRES(service_lock) {
- return services_;
- }
- const std::vector<Service*> services_in_shutdown_order() const REQUIRES(service_lock);
+ auto begin() const { return services_.begin(); }
+ auto end() const { return services_.end(); }
+ const std::vector<std::unique_ptr<Service>>& services() const { return services_; }
+ const std::vector<Service*> services_in_shutdown_order() const;
void MarkPostData();
bool IsPostData();
- void MarkServicesUpdate() REQUIRES(service_lock);
+ void MarkServicesUpdate();
bool IsServicesUpdated() const { return services_update_finished_; }
- void DelayService(const Service& service) REQUIRES(service_lock);
+ void DelayService(const Service& service);
void ResetState() {
post_data_ = false;
diff --git a/init/service_lock.cpp b/init/service_lock.cpp
deleted file mode 100644
index 404d439..0000000
--- a/init/service_lock.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "service_lock.h"
-
-namespace android {
-namespace init {
-
-RecursiveMutex service_lock;
-
-} // namespace init
-} // namespace android
diff --git a/init/service_lock.h b/init/service_lock.h
deleted file mode 100644
index 6b94271..0000000
--- a/init/service_lock.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include <mutex>
-
-#include <android-base/thread_annotations.h>
-
-namespace android {
-namespace init {
-
-// This class exists to add thread annotations, since they're absent from std::recursive_mutex.
-
-class CAPABILITY("mutex") RecursiveMutex {
- public:
- void lock() ACQUIRE() { mutex_.lock(); }
- void unlock() RELEASE() { mutex_.unlock(); }
-
- private:
- std::recursive_mutex mutex_;
-};
-
-extern RecursiveMutex service_lock;
-
-} // namespace init
-} // namespace android
diff --git a/init/service_parser.cpp b/init/service_parser.cpp
index 51f4c97..560f693 100644
--- a/init/service_parser.cpp
+++ b/init/service_parser.cpp
@@ -168,7 +168,6 @@
const std::string fullname = interface_name + "/" + instance_name;
- auto lock = std::lock_guard{service_lock};
for (const auto& svc : *service_list_) {
if (svc->interfaces().count(fullname) > 0) {
return Error() << "Interface '" << fullname << "' redefined in " << service_->name()
@@ -599,7 +598,6 @@
}
}
- auto lock = std::lock_guard{service_lock};
Service* old_service = service_list_->FindService(service_->name());
if (old_service) {
if (!service_->is_override()) {
diff --git a/init/sigchld_handler.cpp b/init/sigchld_handler.cpp
index 064d64d..9b2c7d9 100644
--- a/init/sigchld_handler.cpp
+++ b/init/sigchld_handler.cpp
@@ -64,8 +64,6 @@
std::string wait_string;
Service* service = nullptr;
- auto lock = std::lock_guard{service_lock};
-
if (SubcontextChildReap(pid)) {
name = "Subcontext";
} else {