Make system property reads wait-free
Right now, when we read a system property, we first (assuming we've
already looked up the property's prop_info) read the property's serial
number; if we find that the low bit (the dirty bit) in the serial
number is set, we futex-wait for that serial number to become
non-dirty. By doing so, we spare readers from seeing partially-updated
property values if they race with the property service's non-atomic
memcpy to the property value slot. (The futex-wait here isn't
essential to the algorithm: spinning while dirty would suffice,
although it'd be somewhat less efficient.)
The problem with this approach is that readers can wait on the
property service process, potentially causing delays due to scheduling
variance. Property reads are not guaranteed to complete in finite time
right now.
This change makes property reads wait-free and ensures that they
complete in finite time in all cases. In the new approach, we prevent
value tearing by backing up each property we're about to modify and
directing readers to the backup copy if they try to read a property
with the dirty bit set.
(The wait freedom is limited to the case of readers racing against
*one* property update. A writer can still delay readers by rapidly
updating a property --- but after this change, readers can't hang due
to PID 1 scheduling delays.)
I considered adding explicit atomic access to short property values,
but between binary compatibility with the existing property database
and the need to carefully handle transitions of property values
between "short" (compatible with atomics) and "long" (incompatible
with atomics) length domains, I figured the complexity wasn't worth it
and that making property reads wait-free would be adequate.
Test: boots
Bug: 143561649
Change-Id: Ifd3108aedba5a4b157b66af6ca0a4ed084bd5982
diff --git a/tests/system_properties_test.cpp b/tests/system_properties_test.cpp
index 245e42f..6d696c7 100644
--- a/tests/system_properties_test.cpp
+++ b/tests/system_properties_test.cpp
@@ -340,9 +340,9 @@
ASSERT_EQ(0, system_properties.Add("property", 8, "value1", 6));
const prop_info* pi = system_properties.Find("property");
ASSERT_TRUE(pi != nullptr);
- unsigned serial = system_properties.Serial(pi);
+ unsigned serial = __system_property_serial(pi);
ASSERT_EQ(0, system_properties.Update(const_cast<prop_info*>(pi), "value2", 6));
- ASSERT_NE(serial, system_properties.Serial(pi));
+ ASSERT_NE(serial, __system_property_serial(pi));
#else // __BIONIC__
GTEST_SKIP() << "bionic-only test";
#endif // __BIONIC__
@@ -389,7 +389,7 @@
prop_info* pi = const_cast<prop_info*>(system_properties.Find("property"));
ASSERT_TRUE(pi != nullptr);
- unsigned serial = system_properties.Serial(pi);
+ unsigned serial = __system_property_serial(pi);
std::thread thread([&system_properties]() {
prop_info* pi = const_cast<prop_info*>(system_properties.Find("property"));