From 3196be61fcf0e0303cf819630876b9c812b9474e Mon Sep 17 00:00:00 2001
From: Paul Lawrence <paullawrence@google.com>
Date: Tue, 11 Jul 2023 09:05:11 -0700
Subject: [PATCH] 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
Change-Id: I9ae6863b0ea5e064c59d9d34c03d33fa1da12fdc
---
 init/property_service.cpp | 50 ++++++++++++++++++---------------------
 1 file 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.
+    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);