From 0af9ab33085ca6d992bc976b55e8ecae56c3b99c Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 7 Oct 2022 00:19:55 -0700 Subject: [PATCH] init: Allow persistent properties to be written asynchronously. Persistent properties are flushed with fsync which can cause the non-persistent property write path to block. Bug: 250125146 Test: manual test with ro.property_service.async_persist_write = true Change-Id: Id123bfc7de948b76c51b4d98d00c8cb4a0850bf1 --- init/README.md | 11 ++ init/property_service.cpp | 226 ++++++++++++++++++++++++++++---------- init/property_service.h | 4 + 3 files changed, 180 insertions(+), 61 deletions(-) diff --git a/init/README.md b/init/README.md index f48fa6beb..120d29323 100644 --- a/init/README.md +++ b/init/README.md @@ -174,6 +174,17 @@ If the property `true` becomes `true` *AFTER* `boot` was triggered, nothing will be executed. The condition `boot && property:true=true` will be evaluated to false because the `boot` trigger is a past event. +Note that when `ro.property_service.async_persist_writes` is `true`, there is no +defined ordering between persistent setprops and non-persistent setprops. For +example: + + on boot + setprop a 1 + setprop persist.b 2 + +When `ro.property_service.async_persist_writes` is `true`, triggers for these +two properties may execute in any order. + Services -------- Services are programs which init launches and (optionally) restarts diff --git a/init/property_service.cpp b/init/property_service.cpp index 9df9828f8..87ffdb917 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -98,6 +98,9 @@ using android::sysprop::InitProperties::is_userspace_reboot_supported; namespace android { namespace init { + +class PersistWriteThread; + constexpr auto FINGERPRINT_PROP = "ro.build.fingerprint"; constexpr auto LEGACY_FINGERPRINT_PROP = "ro.build.legacy.fingerprint"; constexpr auto ID_PROP = "ro.build.id"; @@ -115,6 +118,8 @@ static bool accept_messages = false; static std::mutex accept_messages_lock; static std::thread property_service_thread; +static std::unique_ptr persist_write_thread; + static PropertyInfoAreaFile property_info_area; struct PropertyAuditData { @@ -177,48 +182,13 @@ static bool CheckMacPerms(const std::string& name, const char* target_context, return has_access; } -static uint32_t PropertySet(const std::string& name, const std::string& value, std::string* error) { - size_t valuelen = value.size(); - - if (!IsLegalPropertyName(name)) { - *error = "Illegal property name"; - return PROP_ERROR_INVALID_NAME; - } - - if (auto result = IsLegalPropertyValue(name, value); !result.ok()) { - *error = result.error().message(); - return PROP_ERROR_INVALID_VALUE; - } - - prop_info* pi = (prop_info*) __system_property_find(name.c_str()); - if (pi != nullptr) { - // ro.* properties are actually "write-once". - if (StartsWith(name, "ro.")) { - *error = "Read-only property was already set"; - return PROP_ERROR_READ_ONLY_PROPERTY; - } - - __system_property_update(pi, value.c_str(), valuelen); - } else { - int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen); - if (rc < 0) { - *error = "__system_property_add failed"; - return PROP_ERROR_SET_FAILED; - } - } - - // Don't write properties to disk until after we have read all default - // properties to prevent them from being overwritten by default values. - if (persistent_properties_loaded && StartsWith(name, "persist.")) { - WritePersistentProperty(name, value); - } +void NotifyPropertyChange(const std::string& name, const std::string& value) { // 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. auto lock = std::lock_guard{accept_messages_lock}; if (accept_messages) { PropertyChanged(name, value); } - return PROP_SUCCESS; } class AsyncRestorecon { @@ -259,7 +229,9 @@ class AsyncRestorecon { class SocketConnection { public: + SocketConnection() = default; SocketConnection(int socket, const ucred& cred) : socket_(socket), cred_(cred) {} + SocketConnection(SocketConnection&&) = default; bool RecvUint32(uint32_t* value, uint32_t* timeout_ms) { return RecvFully(value, sizeof(*value), timeout_ms); @@ -318,6 +290,8 @@ class SocketConnection { const ucred& cred() { return cred_; } + SocketConnection& operator=(SocketConnection&&) = default; + private: bool PollIn(uint32_t* timeout_ms) { struct pollfd ufd = { @@ -388,9 +362,78 @@ class SocketConnection { unique_fd socket_; ucred cred_; - DISALLOW_IMPLICIT_CONSTRUCTORS(SocketConnection); + DISALLOW_COPY_AND_ASSIGN(SocketConnection); }; +class PersistWriteThread { + public: + PersistWriteThread(); + void Write(std::string name, std::string value, SocketConnection socket); + + private: + void Work(); + + private: + std::thread thread_; + std::mutex mutex_; + std::condition_variable cv_; + std::deque> work_; +}; + +static std::optional PropertySet(const std::string& name, const std::string& value, + SocketConnection* socket, std::string* error) { + size_t valuelen = value.size(); + + if (!IsLegalPropertyName(name)) { + *error = "Illegal property name"; + return {PROP_ERROR_INVALID_NAME}; + } + + if (auto result = IsLegalPropertyValue(name, value); !result.ok()) { + *error = result.error().message(); + return {PROP_ERROR_INVALID_VALUE}; + } + + prop_info* pi = (prop_info*)__system_property_find(name.c_str()); + if (pi != nullptr) { + // ro.* properties are actually "write-once". + if (StartsWith(name, "ro.")) { + *error = "Read-only property was already set"; + return {PROP_ERROR_READ_ONLY_PROPERTY}; + } + + __system_property_update(pi, value.c_str(), valuelen); + } else { + int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen); + if (rc < 0) { + *error = "__system_property_add failed"; + return {PROP_ERROR_SET_FAILED}; + } + } + + // Don't write properties to disk until after we have read all default + // properties to prevent them from being overwritten by default values. + if (socket && persistent_properties_loaded && StartsWith(name, "persist.")) { + if (persist_write_thread) { + persist_write_thread->Write(name, value, std::move(*socket)); + return {}; + } + WritePersistentProperty(name, value); + } + + NotifyPropertyChange(name, value); + return {PROP_SUCCESS}; +} + +// Helper for PropertySet, for the case where no socket is used, and therefore an asynchronous +// return is not possible. +static uint32_t PropertySetNoSocket(const std::string& name, const std::string& value, + std::string* error) { + auto ret = PropertySet(name, value, nullptr, error); + CHECK(ret.has_value()); + return *ret; +} + static uint32_t SendControlMessage(const std::string& msg, const std::string& name, pid_t pid, SocketConnection* socket, std::string* error) { auto lock = std::lock_guard{accept_messages_lock}; @@ -481,16 +524,17 @@ uint32_t CheckPermissions(const std::string& name, const std::string& value, return PROP_SUCCESS; } -// This returns one of the enum of PROP_SUCCESS or PROP_ERROR*. -uint32_t HandlePropertySet(const std::string& name, const std::string& value, - const std::string& source_context, const ucred& cr, - SocketConnection* socket, std::string* error) { +// This returns one of the enum of PROP_SUCCESS or PROP_ERROR*, or std::nullopt +// if asynchronous. +std::optional HandlePropertySet(const std::string& name, const std::string& value, + const std::string& source_context, const ucred& cr, + SocketConnection* socket, std::string* error) { if (auto ret = CheckPermissions(name, value, source_context, cr, error); ret != PROP_SUCCESS) { - return ret; + return {ret}; } if (StartsWith(name, "ctl.")) { - return SendControlMessage(name.c_str() + 4, value, cr.pid, socket, error); + return {SendControlMessage(name.c_str() + 4, value, cr.pid, socket, error)}; } // sys.powerctl is a special property that is used to make the device reboot. We want to log @@ -511,7 +555,7 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, } if (value == "reboot,userspace" && !is_userspace_reboot_supported().value_or(false)) { *error = "Userspace reboot is not supported by this device"; - return PROP_ERROR_INVALID_VALUE; + return {PROP_ERROR_INVALID_VALUE}; } } @@ -522,10 +566,20 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, if (name == kRestoreconProperty && cr.pid != 1 && !value.empty()) { static AsyncRestorecon async_restorecon; async_restorecon.TriggerRestorecon(value); - return PROP_SUCCESS; + return {PROP_SUCCESS}; } - return PropertySet(name, value, error); + return PropertySet(name, value, socket, error); +} + +// Helper for HandlePropertySet, for the case where no socket is used, and +// therefore an asynchronous return is not possible. +uint32_t HandlePropertySetNoSocket(const std::string& name, const std::string& value, + const std::string& source_context, const ucred& cr, + std::string* error) { + auto ret = HandlePropertySet(name, value, source_context, cr, nullptr, error); + CHECK(ret.has_value()); + return *ret; } static void handle_property_set_fd() { @@ -576,8 +630,7 @@ static void handle_property_set_fd() { const auto& cr = socket.cred(); std::string error; - uint32_t result = - HandlePropertySet(prop_name, prop_value, source_context, cr, nullptr, &error); + auto result = HandlePropertySetNoSocket(prop_name, prop_value, source_context, cr, &error); if (result != PROP_SUCCESS) { LOG(ERROR) << "Unable to set property '" << prop_name << "' from uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid << ": " << error; @@ -603,14 +656,19 @@ static void handle_property_set_fd() { return; } + // HandlePropertySet takes ownership of the socket if the set is handled asynchronously. const auto& cr = socket.cred(); std::string error; - uint32_t result = HandlePropertySet(name, value, source_context, cr, &socket, &error); - if (result != PROP_SUCCESS) { + auto result = HandlePropertySet(name, value, source_context, cr, &socket, &error); + if (!result) { + // Result will be sent after completion. + return; + } + if (*result != PROP_SUCCESS) { LOG(ERROR) << "Unable to set property '" << name << "' from uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid << ": " << error; } - socket.SendUint32(result); + socket.SendUint32(*result); break; } @@ -622,10 +680,9 @@ static void handle_property_set_fd() { } uint32_t InitPropertySet(const std::string& name, const std::string& value) { - uint32_t result = 0; ucred cr = {.pid = 1, .uid = 0, .gid = 0}; std::string error; - result = HandlePropertySet(name, value, kInitContext, cr, nullptr, &error); + auto result = HandlePropertySetNoSocket(name, value, kInitContext, cr, &error); if (result != PROP_SUCCESS) { LOG(ERROR) << "Init cannot set '" << name << "' to '" << value << "': " << error; } @@ -795,7 +852,7 @@ static void load_override_properties() { load_properties_from_file("/data/local.prop", nullptr, &properties); for (const auto& [name, value] : properties) { std::string error; - if (PropertySet(name, value, &error) != PROP_SUCCESS) { + if (PropertySetNoSocket(name, value, &error) != PROP_SUCCESS) { LOG(ERROR) << "Could not set '" << name << "' to '" << value << "' in /data/local.prop: " << error; } @@ -861,7 +918,7 @@ static void property_initialize_ro_product_props() { LOG(INFO) << "Setting product property " << base_prop << " to '" << target_prop_val << "' (from " << target_prop << ")"; std::string error; - uint32_t res = PropertySet(base_prop, target_prop_val, &error); + auto res = PropertySetNoSocket(base_prop, target_prop_val, &error); if (res != PROP_SUCCESS) { LOG(ERROR) << "Error setting product property " << base_prop << ": err=" << res << " (" << error << ")"; @@ -890,7 +947,7 @@ static void property_initialize_build_id() { } std::string error; - auto res = PropertySet(ID_PROP, build_id, &error); + auto res = PropertySetNoSocket(ID_PROP, build_id, &error); if (res != PROP_SUCCESS) { LOG(ERROR) << "Failed to set " << ID_PROP << " to " << build_id; } @@ -938,7 +995,7 @@ static void property_derive_legacy_build_fingerprint() { << legacy_build_fingerprint << "'"; std::string error; - uint32_t res = PropertySet(LEGACY_FINGERPRINT_PROP, legacy_build_fingerprint, &error); + auto res = PropertySetNoSocket(LEGACY_FINGERPRINT_PROP, legacy_build_fingerprint, &error); if (res != PROP_SUCCESS) { LOG(ERROR) << "Error setting property '" << LEGACY_FINGERPRINT_PROP << "': err=" << res << " (" << error << ")"; @@ -956,7 +1013,7 @@ static void property_derive_build_fingerprint() { LOG(INFO) << "Setting property '" << FINGERPRINT_PROP << "' to '" << build_fingerprint << "'"; std::string error; - uint32_t res = PropertySet(FINGERPRINT_PROP, build_fingerprint, &error); + auto res = PropertySetNoSocket(FINGERPRINT_PROP, build_fingerprint, &error); if (res != PROP_SUCCESS) { LOG(ERROR) << "Error setting property '" << FINGERPRINT_PROP << "': err=" << res << " (" << error << ")"; @@ -1018,7 +1075,7 @@ static void property_initialize_ro_cpu_abilist() { LOG(INFO) << "Setting property '" << prop << "' to '" << prop_val << "'"; std::string error; - uint32_t res = PropertySet(prop, prop_val, &error); + auto res = PropertySetNoSocket(prop, prop_val, &error); if (res != PROP_SUCCESS) { LOG(ERROR) << "Error setting property '" << prop << "': err=" << res << " (" << error << ")"; @@ -1052,7 +1109,7 @@ static void property_initialize_ro_vendor_api_level() { int api_level = std::min(read_api_level_props(BOARD_API_LEVEL_PROPS), read_api_level_props(DEVICE_API_LEVEL_PROPS)); std::string error; - uint32_t res = PropertySet(VENDOR_API_LEVEL_PROP, std::to_string(api_level), &error); + auto res = PropertySetNoSocket(VENDOR_API_LEVEL_PROP, std::to_string(api_level), &error); if (res != PROP_SUCCESS) { LOG(ERROR) << "Failed to set " << VENDOR_API_LEVEL_PROP << " with " << api_level << ": " << error << "(" << res << ")"; @@ -1146,7 +1203,7 @@ void PropertyLoadBootDefaults() { for (const auto& [name, value] : properties) { std::string error; - if (PropertySet(name, value, &error) != PROP_SUCCESS) { + if (PropertySetNoSocket(name, value, &error) != PROP_SUCCESS) { LOG(ERROR) << "Could not set '" << name << "' to '" << value << "' while loading .prop files" << error; } @@ -1388,6 +1445,46 @@ static void PropertyServiceThread() { } } +PersistWriteThread::PersistWriteThread() { + auto new_thread = std::thread([this]() -> void { Work(); }); + thread_.swap(new_thread); +} + +void PersistWriteThread::Work() { + while (true) { + std::tuple item; + + // Grab the next item within the lock. + { + std::unique_lock lock(mutex_); + + while (work_.empty()) { + cv_.wait(lock); + } + + item = std::move(work_.front()); + work_.pop_front(); + } + + std::this_thread::sleep_for(1s); + + // Perform write/fsync outside the lock. + WritePersistentProperty(std::get<0>(item), std::get<1>(item)); + NotifyPropertyChange(std::get<0>(item), std::get<1>(item)); + + SocketConnection& socket = std::get<2>(item); + socket.SendUint32(PROP_SUCCESS); + } +} + +void PersistWriteThread::Write(std::string name, std::string value, SocketConnection socket) { + { + std::unique_lock lock(mutex_); + work_.emplace_back(std::move(name), std::move(value), std::move(socket)); + } + cv_.notify_all(); +} + void StartPropertyService(int* epoll_socket) { InitPropertySet("ro.property_service.version", "2"); @@ -1412,6 +1509,13 @@ void StartPropertyService(int* epoll_socket) { auto new_thread = std::thread{PropertyServiceThread}; property_service_thread.swap(new_thread); + + auto async_persist_writes = + android::base::GetBoolProperty("ro.property_service.async_persist_writes", false); + + if (async_persist_writes) { + persist_write_thread = std::make_unique(); + } } } // namespace init diff --git a/init/property_service.h b/init/property_service.h index 2d49a36fa..71a609cf7 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -18,7 +18,11 @@ #include +#include +#include +#include #include +#include #include "epoll.h"