Add optional timeout to PropertyWatcher::wait. am: 9349cf0598 am: 6bf2d6abc3 am: 7168a32209 am: 091f367b37 am: c4daacacc3 am: 2ba2eb9c29
Original change: https://android-review.googlesource.com/c/platform/system/librustutils/+/2584503
Change-Id: I56bdc442c16d6bb3bdfa9b6d16342c067e3710e6
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/Android.bp b/Android.bp
index 90f5f47..fcc53b5 100644
--- a/Android.bp
+++ b/Android.bp
@@ -8,7 +8,8 @@
rustlibs: [
"libanyhow",
"libcutils_bindgen",
- "libsystem_properties_bindgen",
+ "liblibc",
+ "libsystem_properties_bindgen_sys",
"libthiserror",
],
}
@@ -40,6 +41,33 @@
auto_gen_config: true,
}
+// Build a separate rust_library rather than depending directly on libsystem_properties_bindgen,
+// to work around the fact that rust_bindgen targets only produce rlibs and not dylibs, which would
+// result in duplicate conflicting versions of liblibc. This will hopefully be fixed in the build
+// system, at which point we can delete this target and go back to using libsystem_properties_bindgen
+// directly.
+rust_library {
+ name: "libsystem_properties_bindgen_sys",
+ crate_name: "system_properties_bindgen",
+ srcs: [
+ ":libsystem_properties_bindgen",
+ ],
+ rustlibs: [
+ "liblibc",
+ ],
+ vendor_available: true,
+ apex_available: [
+ "//apex_available:platform",
+ "com.android.btservices",
+ "com.android.compos",
+ "com.android.uwb",
+ "com.android.virt",
+ ],
+ min_sdk_version: "29",
+ lints: "none",
+ clippy_lints: "none",
+}
+
rust_bindgen {
name: "libsystem_properties_bindgen",
wrapper_src: "bindgen/system_properties.h",
@@ -53,6 +81,12 @@
"--allowlist-function=__system_property_read_callback",
"--allowlist-function=__system_property_set",
"--allowlist-function=__system_property_wait",
+ "--blocklist-type=timespec",
+ "--raw-line",
+ "use libc::timespec;",
+ ],
+ rustlibs: [
+ "liblibc",
],
vendor_available: true,
apex_available: [
@@ -95,6 +129,9 @@
srcs: [":libsystem_properties_bindgen"],
crate_name: "system_properties_bindgen_test",
test_suites: ["general-tests"],
+ rustlibs: [
+ "liblibc",
+ ],
auto_gen_config: true,
clippy_lints: "none",
lints: "none",
diff --git a/system_properties.rs b/system_properties.rs
index f2ceeeb..6edef94 100644
--- a/system_properties.rs
+++ b/system_properties.rs
@@ -16,11 +16,13 @@
//! in Android system properties.
use anyhow::Context;
+use libc::timespec;
use std::os::raw::c_char;
use std::ptr::null;
use std::{
ffi::{c_uint, c_void, CStr, CString},
str::Utf8Error,
+ time::{Duration, Instant},
};
use system_properties_bindgen::prop_info as PropInfo;
use thiserror::Error;
@@ -37,7 +39,7 @@
/// System properties are not initialized
#[error("System properties are not initialized.")]
Uninitialized,
- /// __system_property_wait timed out despite being given no timeout.
+ /// __system_property_wait timed out.
#[error("Wait failed")]
WaitFailed,
/// read callback was not called
@@ -159,12 +161,13 @@
// Waits for the property that self is watching to be created. Returns immediately if the
// property already exists.
- fn wait_for_property_creation(&mut self) -> Result<()> {
+ fn wait_for_property_creation_until(&mut self, until: Option<Instant>) -> Result<()> {
let mut global_serial = 0;
loop {
match self.get_prop_info() {
Some(_) => return Ok(()),
None => {
+ let remaining_timeout = remaining_time_until(until);
// Unsafe call for FFI. The function modifies only global_serial, and has
// no side-effects.
if !unsafe {
@@ -174,7 +177,11 @@
null(),
global_serial,
&mut global_serial,
- null(),
+ if let Some(remaining_timeout) = &remaining_timeout {
+ remaining_timeout
+ } else {
+ null()
+ },
)
} {
return Err(PropertyWatcherError::WaitFailed);
@@ -184,16 +191,17 @@
}
}
- /// Wait for the system property to change. This
- /// records the serial number of the last change, so
- /// race conditions are avoided.
- pub fn wait(&mut self) -> Result<()> {
+ /// Waits until the system property changes, or `until` is reached.
+ ///
+ /// This records the serial number of the last change, so race conditions are avoided.
+ fn wait_for_property_change_until(&mut self, until: Option<Instant>) -> Result<()> {
// If the property is null, then wait for it to be created. Subsequent waits will
// skip this step and wait for our specific property to change.
if self.prop_info.is_null() {
- return self.wait_for_property_creation();
+ return self.wait_for_property_creation_until(None);
}
+ let remaining_timeout = remaining_time_until(until);
let mut new_serial = self.serial;
// Unsafe block to call __system_property_wait.
// All arguments are private to PropertyWatcher so we
@@ -203,7 +211,11 @@
self.prop_info,
self.serial,
&mut new_serial,
- null(),
+ if let Some(remaining_timeout) = &remaining_timeout {
+ remaining_timeout
+ } else {
+ null()
+ },
)
} {
return Err(PropertyWatcherError::WaitFailed);
@@ -211,6 +223,14 @@
self.serial = new_serial;
Ok(())
}
+
+ /// Waits for the system property to change, or the timeout to elapse.
+ ///
+ /// This records the serial number of the last change, so race conditions are avoided.
+ pub fn wait(&mut self, timeout: Option<Duration>) -> Result<()> {
+ let until = timeout.map(|timeout| Instant::now() + timeout);
+ self.wait_for_property_change_until(until)
+ }
}
/// Reads a system property.
@@ -232,6 +252,27 @@
}
}
+/// Returns the duration remaining until the given instant.
+///
+/// Returns `None` if `None` is passed in, or `Some(0)` if `until` is in the past.
+fn remaining_time_until(until: Option<Instant>) -> Option<timespec> {
+ until.map(|until| {
+ duration_to_timespec(
+ until
+ .checked_duration_since(Instant::now())
+ .unwrap_or_default(),
+ )
+ })
+}
+
+/// Converts the given `Duration` to a C `timespec`.
+fn duration_to_timespec(duration: Duration) -> timespec {
+ timespec {
+ tv_sec: duration.as_secs().try_into().unwrap(),
+ tv_nsec: duration.subsec_nanos().try_into().unwrap(),
+ }
+}
+
/// Returns true if the system property `name` has the value "1", "y", "yes", "on", or "true",
/// false for "0", "n", "no", "off", or "false", or `default_value` otherwise.
pub fn read_bool(name: &str, default_value: bool) -> Result<bool> {
diff --git a/system_properties_fuzzer.rs b/system_properties_fuzzer.rs
index 5950ac2..a6d1e05 100644
--- a/system_properties_fuzzer.rs
+++ b/system_properties_fuzzer.rs
@@ -98,7 +98,7 @@
// Spawn a thread that will wait for a change to the property.
let waiter = thread::spawn(move || {
let result = match system_properties::PropertyWatcher::new(prop_str) {
- Ok(mut watcher) => watcher.wait(),
+ Ok(mut watcher) => watcher.wait(None),
Err(e) => Err(e),
};
waited_clone.store(true, Ordering::Relaxed);
