Add __system_property_wait and return the serial in __system_property_read_callback.
In order to implement android::base::WaitForProperty well, we need a way to
wait not for *any* property to change (__system_property_wait_any), but to
specifically wait for the property represented by a given `prop_info` to
change.
The android::base::WaitForProperty implementation, like attempts to cache
system properties in the past, also needs a way to keep serials and values
in sync, but the existing functions don't provide a cheap way to get a
consistent snapshot. Change the __system_property_read_callback callback's
type to include the serial corresponding to the given value.
Add a test, slightly clean up some of the existing tests (and name them to
include the names of the functions they're testing, in our usual style).
Bug: http://b/35201172
Test: ran tests
Change-Id: Ibc8ebe2e88eef1e333a1bd3dd7f68135f1ba7fb5
diff --git a/libc/bionic/system_properties.cpp b/libc/bionic/system_properties.cpp
index a62ce14..32d1e31 100644
--- a/libc/bionic/system_properties.cpp
+++ b/libc/bionic/system_properties.cpp
@@ -1102,7 +1102,7 @@
return fsetxattr_failed ? -2 : 0;
}
-unsigned int __system_property_area_serial() {
+uint32_t __system_property_area_serial() {
prop_area* pa = __system_property_area__;
if (!pa) {
return -1;
@@ -1163,8 +1163,10 @@
}
void __system_property_read_callback(const prop_info* pi,
- void (*callback)(void* cookie, const char* name,
- const char* value),
+ void (*callback)(void* cookie,
+ const char* name,
+ const char* value,
+ uint32_t serial),
void* cookie) {
while (true) {
uint32_t serial = __system_property_serial(pi); // acquire semantics
@@ -1177,7 +1179,7 @@
// TODO: see todo in __system_property_read function
atomic_thread_fence(memory_order_acquire);
if (serial == load_const_atomic(&(pi->serial), memory_order_relaxed)) {
- callback(cookie, pi->name, value_buf);
+ callback(cookie, pi->name, value_buf, serial);
return;
}
}
@@ -1329,30 +1331,34 @@
}
// Wait for non-locked serial, and retrieve it with acquire semantics.
-unsigned int __system_property_serial(const prop_info* pi) {
+uint32_t __system_property_serial(const prop_info* pi) {
uint32_t serial = load_const_atomic(&pi->serial, memory_order_acquire);
while (SERIAL_DIRTY(serial)) {
- __futex_wait(const_cast<volatile void*>(reinterpret_cast<const void*>(&pi->serial)), serial,
- nullptr);
+ __futex_wait(const_cast<_Atomic(uint_least32_t)*>(&pi->serial), serial, nullptr);
serial = load_const_atomic(&pi->serial, memory_order_acquire);
}
return serial;
}
-unsigned int __system_property_wait_any(unsigned int serial) {
+uint32_t __system_property_wait_any(uint32_t old_serial) {
prop_area* pa = __system_property_area__;
- uint32_t my_serial;
+ if (!pa) return 0;
- if (!pa) {
- return 0;
- }
-
+ uint32_t new_serial;
do {
- __futex_wait(pa->serial(), serial, nullptr);
- my_serial = atomic_load_explicit(pa->serial(), memory_order_acquire);
- } while (my_serial == serial);
+ __futex_wait(pa->serial(), old_serial, nullptr);
+ new_serial = atomic_load_explicit(pa->serial(), memory_order_acquire);
+ } while (new_serial == old_serial);
+ return new_serial;
+}
- return my_serial;
+uint32_t __system_property_wait(const prop_info* pi, uint32_t old_serial) {
+ uint32_t new_serial;
+ do {
+ __futex_wait(const_cast<_Atomic(uint_least32_t)*>(&pi->serial), old_serial, nullptr);
+ new_serial = load_const_atomic(&pi->serial, memory_order_acquire);
+ } while (new_serial == old_serial);
+ return new_serial;
}
const prop_info* __system_property_find_nth(unsigned n) {
diff --git a/libc/include/sys/_system_properties.h b/libc/include/sys/_system_properties.h
index ffa6d2e..fa98d11 100644
--- a/libc/include/sys/_system_properties.h
+++ b/libc/include/sys/_system_properties.h
@@ -30,6 +30,7 @@
#define _INCLUDE_SYS__SYSTEM_PROPERTIES_H
#include <sys/cdefs.h>
+#include <stdint.h>
#ifndef _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
#error you should #include <sys/system_properties.h> instead
@@ -91,7 +92,7 @@
**
** Returns the serial number on success, -1 on error.
*/
-unsigned int __system_property_area_serial();
+uint32_t __system_property_area_serial();
/* Add a new system property. Can only be done by a single
** process that has write access to the property area, and
@@ -118,12 +119,22 @@
**
** Returns the serial number on success, -1 on error.
*/
-unsigned int __system_property_serial(const prop_info *pi);
+uint32_t __system_property_serial(const prop_info* pi);
-/* Wait for any system property to be updated. Caller must pass
-** in 0 the first time, and the previous return value on each
-** successive call. */
-unsigned int __system_property_wait_any(unsigned int serial);
+/*
+ * Waits for any system property to be updated past `old_serial`.
+ * If you don't know the current global serial number, use 0.
+ * Returns the new global serial number.
+ */
+uint32_t __system_property_wait_any(uint32_t old_serial);
+
+/*
+ * Waits for the specific system property identified by `pi` to be updated past `old_serial`.
+ * If you don't know the current serial, use 0.
+ * Returns the serial number for `pi` that caused the wake.
+ */
+uint32_t __system_property_wait(const prop_info* pi, uint32_t old_serial)
+ __INTRODUCED_IN_FUTURE;
/* Initialize the system properties area in read only mode.
* Should be done by all processes that need to read system
diff --git a/libc/include/sys/system_properties.h b/libc/include/sys/system_properties.h
index d80b2fe..fb90251 100644
--- a/libc/include/sys/system_properties.h
+++ b/libc/include/sys/system_properties.h
@@ -31,77 +31,52 @@
#include <sys/cdefs.h>
#include <stddef.h>
+#include <stdint.h>
__BEGIN_DECLS
typedef struct prop_info prop_info;
-#define PROP_NAME_MAX 32
#define PROP_VALUE_MAX 92
-/* Look up a system property by name, copying its value and a
-** \0 terminator to the provided pointer. The total bytes
-** copied will be no greater than PROP_VALUE_MAX. Returns
-** the string length of the value. A property that is not
-** defined is identical to a property with a length 0 value.
-*/
-int __system_property_get(const char *name, char *value);
-
-/* Set a system property by name.
-**/
+/*
+ * Sets system property `key` to `value`, creating the system property if it doesn't already exist.
+ */
int __system_property_set(const char* key, const char* value) __INTRODUCED_IN(12);
-/* Return a pointer to the system property named name, if it
-** exists, or NULL if there is no such property. Use
-** __system_property_read() to obtain the string value from
-** the returned prop_info pointer.
-**
-** It is safe to cache the prop_info pointer to avoid future
-** lookups. These returned pointers will remain valid for
-** the lifetime of the system.
-*/
-const prop_info *__system_property_find(const char *name);
+/*
+ * Returns a `prop_info` corresponding system property `name`, or nullptr if it doesn't exist.
+ * Use __system_property_read_callback to query the current value.
+ *
+ * Property lookup is expensive, so it can be useful to cache the result of this function.
+ */
+const prop_info* __system_property_find(const char* name);
-/* Read the value of a system property. Returns the length
-** of the value. Copies the value and \0 terminator into
-** the provided value pointer. Total length (including
-** terminator) will be no greater that PROP_VALUE_MAX for
-** __system_property_read.
-**
-** If name is nonzero, up to PROP_NAME_MAX bytes will be
-** copied into the provided name pointer. The name will
-** be \0 terminated.
-*/
-int __system_property_read(const prop_info *pi, char *name, char *value);
+/*
+ * Calls `callback` with a consistent trio of name, value, and serial number for property `pi`.
+ */
void __system_property_read_callback(const prop_info *pi,
- void (*)(void* cookie, const char *name, const char *value),
- void* cookie) __INTRODUCED_IN_FUTURE;
+ void (*callback)(void* cookie, const char *name, const char *value, uint32_t serial),
+ void* cookie) __INTRODUCED_IN_FUTURE;
-/* Return a prop_info for the nth system property, or NULL if
-** there is no nth property. Use __system_property_read() to
-** read the value of this property.
-**
-** Please do not call this method. It only exists to provide
-** backwards compatibility to NDK apps. Its implementation
-** is inefficient and order of results may change from call
-** to call.
-*/
-const prop_info *__system_property_find_nth(unsigned n)
- __REMOVED_IN(26);
-
-/* Pass a prop_info for each system property to the provided
-** callback. Use __system_property_read() to read the value
-** of this property.
-**
-** This method is for inspecting and debugging the property
-** system. Please use __system_property_find() instead.
-**
-** Order of results may change from call to call. This is
-** not a bug.
-*/
+/*
+ * Passes a `prop_info` for each system property to the provided
+ * callback. Use __system_property_read_callback() to read the value.
+ *
+ * This method is for inspecting and debugging the property system, and not generally useful.
+ */
int __system_property_foreach(void (*propfn)(const prop_info* pi, void* cookie), void* cookie)
__INTRODUCED_IN(19);
+/* Deprecated. In Android O and above, there's no limit on property name length. */
+#define PROP_NAME_MAX 32
+/* Deprecated. Use __system_property_read_callback instead. */
+int __system_property_read(const prop_info *pi, char *name, char *value);
+/* Deprecated. Use __system_property_read_callback instead. */
+int __system_property_get(const char *name, char *value);
+/* Deprecated. Use __system_property_foreach instead. Aborts in Android O and above. */
+const prop_info *__system_property_find_nth(unsigned n) __REMOVED_IN(26);
+
__END_DECLS
#endif
diff --git a/libc/libc.arm.map b/libc/libc.arm.map
index 4319429..33aedb2 100644
--- a/libc/libc.arm.map
+++ b/libc/libc.arm.map
@@ -1266,6 +1266,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
bsd_signal; # arm x86 mips versioned=26
catclose; # future
catgets; # future
diff --git a/libc/libc.arm64.map b/libc/libc.arm64.map
index f88c284..0d4fc2d 100644
--- a/libc/libc.arm64.map
+++ b/libc/libc.arm64.map
@@ -1189,6 +1189,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
catclose; # future
catgets; # future
catopen; # future
diff --git a/libc/libc.map.txt b/libc/libc.map.txt
index 46087e6..4d9ac57 100644
--- a/libc/libc.map.txt
+++ b/libc/libc.map.txt
@@ -1291,6 +1291,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
bsd_signal; # arm x86 mips versioned=26
catclose; # future
catgets; # future
diff --git a/libc/libc.mips.map b/libc/libc.mips.map
index 075746c..c526226 100644
--- a/libc/libc.mips.map
+++ b/libc/libc.mips.map
@@ -1250,6 +1250,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
bsd_signal; # arm x86 mips versioned=26
catclose; # future
catgets; # future
diff --git a/libc/libc.mips64.map b/libc/libc.mips64.map
index f88c284..0d4fc2d 100644
--- a/libc/libc.mips64.map
+++ b/libc/libc.mips64.map
@@ -1189,6 +1189,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
catclose; # future
catgets; # future
catopen; # future
diff --git a/libc/libc.x86.map b/libc/libc.x86.map
index 75c7757..130bbed 100644
--- a/libc/libc.x86.map
+++ b/libc/libc.x86.map
@@ -1248,6 +1248,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
bsd_signal; # arm x86 mips versioned=26
catclose; # future
catgets; # future
diff --git a/libc/libc.x86_64.map b/libc/libc.x86_64.map
index f88c284..0d4fc2d 100644
--- a/libc/libc.x86_64.map
+++ b/libc/libc.x86_64.map
@@ -1189,6 +1189,7 @@
LIBC_O {
global:
__system_property_read_callback; # future
+ __system_property_wait; # future
catclose; # future
catgets; # future
catopen; # future