ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Revert "Fix deadlock caused by two-threaded property controls"

Browse source 
This reverts commit 606afc7b74.

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
This commit is contained in:
Paul Lawrence 2023-07-11 09:05:11 -07:00
parent c742bfda6b
commit 3196be61fc
1 changed files with 23 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
init/property_service.cpp
View file

@ -117,6 +117,7 @@ static bool accept_messages = false;
static std::mutex accept_messages_lock; static std::mutex accept_messages_lock;
static std::thread property_service_thread; static std::thread property_service_thread;
static std::thread property_service_for_system_thread; static std::thread property_service_for_system_thread;
static std::mutex set_property_lock;
static std::unique_ptr<PersistWriteThread> persist_write_thread; 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}; return {PROP_ERROR_INVALID_VALUE};
} }
if (name == "sys.powerctl") { auto lock = std::lock_guard{set_property_lock};
// No action here - NotifyPropertyChange will trigger the appropriate action, and since this prop_info* pi = (prop_info*)__system_property_find(name.c_str());
// can come to the second thread, we mustn't call out to the __system_property_* functions if (pi != nullptr) {
// which support multiple readers but only one mutator. // 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 { } else {
prop_info* pi = (prop_info*)__system_property_find(name.c_str()); int rc = __system_property_add(name.c_str(), name.size(), value.c_str(), valuelen);
if (pi != nullptr) { if (rc < 0) {
// ro.* properties are actually "write-once". *error = "__system_property_add failed";
if (StartsWith(name, "ro.")) { return {PROP_ERROR_SET_FAILED};
*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 // Don't write properties to disk until after we have read all default
// properties to prevent them from being overwritten by default values. // properties to prevent them from being overwritten by default values.
if (socket && persistent_properties_loaded && StartsWith(name, "persist.")) { if (socket && persistent_properties_loaded && StartsWith(name, "persist.")) {
if (persist_write_thread) { if (persist_write_thread) {
persist_write_thread->Write(name, value, std::move(*socket)); persist_write_thread->Write(name, value, std::move(*socket));
return {}; return {};
}
WritePersistentProperty(name, value);
} }
WritePersistentProperty(name, value);
} }
NotifyPropertyChange(name, value); NotifyPropertyChange(name, value);