diff --git a/libc/bionic/system_properties.cpp b/libc/bionic/system_properties.cpp index a62ce14f4..32d1e3114 100644 --- a/libc/bionic/system_properties.cpp +++ b/libc/bionic/system_properties.cpp @@ -1102,7 +1102,7 @@ int __system_property_area_init() { 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 @@ int __system_property_read(const prop_info* pi, char* name, char* value) { } 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 @@ void __system_property_read_callback(const prop_info* pi, // 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 @@ int __system_property_add(const char* name, unsigned int namelen, const char* va } // 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(reinterpret_cast(&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 ffa6d2e73..fa98d11d6 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 +#include #ifndef _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ #error you should #include instead @@ -91,7 +92,7 @@ int __system_property_area_init(); ** ** 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 @@ int __system_property_update(prop_info *pi, const char *value, unsigned int len) ** ** 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 d80b2fee8..fb90251e5 100644 --- a/libc/include/sys/system_properties.h +++ b/libc/include/sys/system_properties.h @@ -31,77 +31,52 @@ #include #include +#include __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 43194295e..33aedb28e 100644 --- a/libc/libc.arm.map +++ b/libc/libc.arm.map @@ -1266,6 +1266,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce 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 f88c28425..0d4fc2d57 100644 --- a/libc/libc.arm64.map +++ b/libc/libc.arm64.map @@ -1189,6 +1189,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce 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 46087e606..4d9ac5705 100644 --- a/libc/libc.map.txt +++ b/libc/libc.map.txt @@ -1291,6 +1291,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce 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 075746c56..c526226cd 100644 --- a/libc/libc.mips.map +++ b/libc/libc.mips.map @@ -1250,6 +1250,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce 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 f88c28425..0d4fc2d57 100644 --- a/libc/libc.mips64.map +++ b/libc/libc.mips64.map @@ -1189,6 +1189,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce 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 75c77578b..130bbed4c 100644 --- a/libc/libc.x86.map +++ b/libc/libc.x86.map @@ -1248,6 +1248,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce 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 f88c28425..0d4fc2d57 100644 --- a/libc/libc.x86_64.map +++ b/libc/libc.x86_64.map @@ -1189,6 +1189,7 @@ LIBC_N { # introduced-arm64=24 introduced-mips=24 introduced-mips64=24 introduce LIBC_O { global: __system_property_read_callback; # future + __system_property_wait; # future catclose; # future catgets; # future catopen; # future diff --git a/tests/system_properties_test.cpp b/tests/system_properties_test.cpp index 6b037d8af..39734d752 100644 --- a/tests/system_properties_test.cpp +++ b/tests/system_properties_test.cpp @@ -20,7 +20,9 @@ #include #include #include + #include +#include #if defined(__BIONIC__) @@ -111,13 +113,11 @@ static void* PropertyWaitHelperFn(void* arg) { #endif // __BIONIC__ -TEST(properties, add) { +TEST(properties, __system_property_add) { #if defined(__BIONIC__) LocalPropertyTestState pa; ASSERT_TRUE(pa.valid); - char propvalue[PROP_VALUE_MAX]; - ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); @@ -132,6 +132,7 @@ TEST(properties, add) { name[sizeof(name)-1] = '\0'; ASSERT_EQ(0, __system_property_add(name, strlen(name), "value", 5)); + char propvalue[PROP_VALUE_MAX]; ASSERT_EQ(6, __system_property_get("property", propvalue)); ASSERT_STREQ(propvalue, "value1"); @@ -148,30 +149,28 @@ TEST(properties, add) { #endif // __BIONIC__ } -TEST(properties, update) { +TEST(properties, __system_property_update) { #if defined(__BIONIC__) LocalPropertyTestState pa; ASSERT_TRUE(pa.valid); - char propvalue[PROP_VALUE_MAX]; - prop_info *pi; - ASSERT_EQ(0, __system_property_add("property", 8, "oldvalue1", 9)); ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); - pi = (prop_info *)__system_property_find("property"); - ASSERT_NE((prop_info *)NULL, pi); - __system_property_update(pi, "value4", 6); + const prop_info* pi = __system_property_find("property"); + ASSERT_TRUE(pi != nullptr); + __system_property_update(const_cast(pi), "value4", 6); - pi = (prop_info *)__system_property_find("other_property"); - ASSERT_NE((prop_info *)NULL, pi); - __system_property_update(pi, "newvalue5", 9); + pi = __system_property_find("other_property"); + ASSERT_TRUE(pi != nullptr); + __system_property_update(const_cast(pi), "newvalue5", 9); - pi = (prop_info *)__system_property_find("property_other"); - ASSERT_NE((prop_info *)NULL, pi); - __system_property_update(pi, "value6", 6); + pi = __system_property_find("property_other"); + ASSERT_TRUE(pi != nullptr); + __system_property_update(const_cast(pi), "value6", 6); + char propvalue[PROP_VALUE_MAX]; ASSERT_EQ(6, __system_property_get("property", propvalue)); ASSERT_STREQ(propvalue, "value4"); @@ -230,16 +229,16 @@ TEST(properties, fill) { #endif // __BIONIC__ } -TEST(properties, foreach) { +TEST(properties, __system_property_foreach) { #if defined(__BIONIC__) LocalPropertyTestState pa; ASSERT_TRUE(pa.valid); - size_t count = 0; ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); ASSERT_EQ(0, __system_property_add("other_property", 14, "value2", 6)); ASSERT_EQ(0, __system_property_add("property_other", 14, "value3", 6)); + size_t count = 0; ASSERT_EQ(0, __system_property_foreach(foreach_test_callback, &count)); ASSERT_EQ(3U, count); #else // __BIONIC__ @@ -247,7 +246,7 @@ TEST(properties, foreach) { #endif // __BIONIC__ } -TEST(properties, find_nth) { +TEST(properties, __system_property_find_nth) { #if defined(__BIONIC__) LocalPropertyTestState pa; ASSERT_TRUE(pa.valid); @@ -342,47 +341,74 @@ TEST(properties, errors) { #endif // __BIONIC__ } -TEST(properties, serial) { +TEST(properties, __system_property_serial) { #if defined(__BIONIC__) LocalPropertyTestState pa; ASSERT_TRUE(pa.valid); - const prop_info *pi; - unsigned int serial; ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); - ASSERT_NE((const prop_info *)NULL, pi = __system_property_find("property")); - serial = __system_property_serial(pi); - ASSERT_EQ(0, __system_property_update((prop_info *)pi, "value2", 6)); + const prop_info* pi = __system_property_find("property"); + ASSERT_TRUE(pi != nullptr); + unsigned serial = __system_property_serial(pi); + ASSERT_EQ(0, __system_property_update(const_cast(pi), "value2", 6)); ASSERT_NE(serial, __system_property_serial(pi)); #else // __BIONIC__ GTEST_LOG_(INFO) << "This test does nothing.\n"; #endif // __BIONIC__ } -TEST(properties, wait) { +TEST(properties, __system_property_wait_any) { #if defined(__BIONIC__) LocalPropertyTestState pa; ASSERT_TRUE(pa.valid); - unsigned int serial; - prop_info *pi; - pthread_t t; - int flag = 0; ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); - serial = __system_property_wait_any(0); + unsigned serial = __system_property_wait_any(0); - pi = const_cast(__system_property_find("property")); + prop_info* pi = const_cast(__system_property_find("property")); ASSERT_TRUE(pi != nullptr); __system_property_update(pi, "value2", 6); serial = __system_property_wait_any(serial); + int flag = 0; + pthread_t t; ASSERT_EQ(0, pthread_create(&t, nullptr, PropertyWaitHelperFn, &flag)); ASSERT_EQ(flag, 0); serial = __system_property_wait_any(serial); ASSERT_EQ(flag, 1); - void* result; - ASSERT_EQ(0, pthread_join(t, &result)); + ASSERT_EQ(0, pthread_join(t, nullptr)); +#else // __BIONIC__ + GTEST_LOG_(INFO) << "This test does nothing.\n"; +#endif // __BIONIC__ +} + +TEST(properties, __system_property_wait) { +#if defined(__BIONIC__) + LocalPropertyTestState pa; + ASSERT_TRUE(pa.valid); + + ASSERT_EQ(0, __system_property_add("property", 8, "value1", 6)); + + prop_info* pi = const_cast(__system_property_find("property")); + ASSERT_TRUE(pi != nullptr); + + unsigned serial = __system_property_serial(pi); + + std::thread thread([]() { + prop_info* pi = const_cast(__system_property_find("property")); + ASSERT_TRUE(pi != nullptr); + + __system_property_update(pi, "value2", 6); + }); + + __system_property_wait(pi, serial); + + char value[PROP_VALUE_MAX]; + ASSERT_EQ(6, __system_property_get("property", value)); + ASSERT_STREQ("value2", value); + + thread.join(); #else // __BIONIC__ GTEST_LOG_(INFO) << "This test does nothing.\n"; #endif // __BIONIC__ diff --git a/tests/system_properties_test2.cpp b/tests/system_properties_test2.cpp index 056096001..e6e7ef25c 100644 --- a/tests/system_properties_test2.cpp +++ b/tests/system_properties_test2.cpp @@ -90,20 +90,22 @@ TEST(properties, smoke) { ASSERT_TRUE(pi != nullptr); std::string expected_name = property_name; - __system_property_read_callback(pi, [](void* cookie, const char *name, const char *value) { - const std::string* expected_name = static_cast(cookie); - ASSERT_EQ(*expected_name, name); - ASSERT_STREQ("value1-1", value); + __system_property_read_callback(pi, + [](void* cookie, const char* name, const char* value, unsigned /*serial*/) { + const std::string* expected_name = static_cast(cookie); + ASSERT_EQ(*expected_name, name); + ASSERT_STREQ("value1-1", value); }, &expected_name); pi = __system_property_find(long_property_name.c_str()); ASSERT_TRUE(pi != nullptr); expected_name = long_property_name; - __system_property_read_callback(pi, [](void* cookie, const char *name, const char *value) { - const std::string* expected_name = static_cast(cookie); - ASSERT_EQ(*expected_name, name); - ASSERT_STREQ("value2", value); + __system_property_read_callback(pi, + [](void* cookie, const char* name, const char* value, unsigned /*serial*/) { + const std::string* expected_name = static_cast(cookie); + ASSERT_EQ(*expected_name, name); + ASSERT_STREQ("value2", value); }, &expected_name); // Check that read() for long names still works but returns truncated version of the name