summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Lawrence <paullawrence@google.com>2023-07-11 09:05:11 -0700
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-07-18 16:49:19 +0000
commit3adb2a63bed9e97a5a179316b4f14a3196df81e3 (patch)
treea256b7f8b68e78db7635abb186572311d0bb87c8
parent1cc206ba29bf03fce55edaaad551ec3947be4bbf (diff)
downloadcore-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.cpp50
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);