diff options
author | Paul Lawrence <paullawrence@google.com> | 2023-07-11 09:05:11 -0700 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-07-18 16:49:19 +0000 |
commit | 3adb2a63bed9e97a5a179316b4f14a3196df81e3 (patch) | |
tree | a256b7f8b68e78db7635abb186572311d0bb87c8 | |
parent | 1cc206ba29bf03fce55edaaad551ec3947be4bbf (diff) | |
download | core-3adb2a63bed9e97a5a179316b4f14a3196df81e3.tar.gz |
Revert "Fix deadlock caused by two-threaded property controls"
This reverts commit 606afc7b7451aba90e3634076d9b59a5ef08186b.
These fixes for b/262208935 introduced a race condition. We believe the
race is fixed by ag/23879563, but at this point in the release feel that
reverting the fixes and refixing in main is the better solution
Test: Builds, boots
Bug: 283202477
Bug: 288991737
Ignore-AOSP-First: Reverting CL only in internal
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3196be61fcf0e0303cf819630876b9c812b9474e)
Merged-In: I9ae6863b0ea5e064c59d9d34c03d33fa1da12fdc
Change-Id: I9ae6863b0ea5e064c59d9d34c03d33fa1da12fdc
-rw-r--r-- | init/property_service.cpp | 50 |
1 files changed, 23 insertions, 27 deletions
diff --git a/init/property_service.cpp b/init/property_service.cpp index 4242912ed..2d084db46 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -117,6 +117,7 @@ static bool accept_messages = false; static std::mutex accept_messages_lock; static std::thread property_service_thread; static std::thread property_service_for_system_thread; +static std::mutex set_property_lock; static std::unique_ptr<PersistWriteThread> persist_write_thread; @@ -394,37 +395,32 @@ static std::optional<uint32_t> PropertySet(const std::string& name, const std::s return {PROP_ERROR_INVALID_VALUE}; } - if (name == "sys.powerctl") { - // No action here - NotifyPropertyChange will trigger the appropriate action, and since this - // can come to the second thread, we mustn't call out to the __system_property_* functions - // which support multiple readers but only one mutator. - } else { - 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}; - } + auto lock = std::lock_guard{set_property_lock}; + 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}; - } + __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); + // 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); |