commit abc6e1240d49ac3909cffc0af3fe38f36e7d023e
author Paul Lawrence <paullawrence@google.com> Mon Jul 17 16:51:58 2023 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Mon Jul 17 16:51:58 2023 +0000
tree c1b13192108df5d626ef13df7b27b02d7cb2835f
parent d7de52d72175c943667455dcc15c2d17869393d3
parent 3196be61fcf0e0303cf819630876b9c812b9474e

Revert "Fix deadlock caused by two-threaded property controls" am: 3196be61fc

Original change: https://googleplex-android-review.googlesource.com/c/platform/system/core/+/24028968

Change-Id: I7289b8597a48f2be35484fd339fa6d6d66b10f0a
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

init/property_service.cpp

diff --git a/init/property_service.cpp b/init/property_service.cpp
index 4242912..2d084db 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -117,6 +117,7 @@
 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 @@
         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.
+    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 {
-        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};
-            }
+        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);