From 03edc9f7647b3ac9db8a4743acc98b0238b846fb Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Thu, 16 Feb 2017 17:14:10 -0800
Subject: [PATCH] Add timeout support to android::base::WaitForProperty.

Bug: http://b/35201172
Test: ran tests
Change-Id: I025aa0217dc94fabf0eb076b285a84866b00e741
---
 base/include/android-base/properties.h |  9 ++++--
 base/properties.cpp                    | 43 +++++++++++++++++++++++---
 base/properties_test.cpp               | 15 +++++++--
 3 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/base/include/android-base/properties.h b/base/include/android-base/properties.h
index e275fa2a4..e2324b785 100644
--- a/base/include/android-base/properties.h
+++ b/base/include/android-base/properties.h
@@ -23,6 +23,7 @@
 #error Only bionic supports system properties.
 #endif
 
+#include <chrono>
 #include <limits>
 #include <string>
 
@@ -58,8 +59,12 @@ template <typename T> T GetUintProperty(const std::string& key,
 // tell you whether or not your call succeeded. A `false` return value definitely means failure.
 bool SetProperty(const std::string& key, const std::string& value);
 
-// Waits for the system property `key` to have the value `expected_value`, .
-void WaitForProperty(const std::string& key, const std::string& expected_value);
+// Waits for the system property `key` to have the value `expected_value`.
+// Times out after `relative_timeout`.
+// Returns true on success, false on timeout.
+bool WaitForProperty(const std::string& key,
+                     const std::string& expected_value,
+                     std::chrono::milliseconds relative_timeout);
 
 } // namespace base
 } // namespace android
diff --git a/base/properties.cpp b/base/properties.cpp
index 0f0f6382f..acd6c0fbf 100644
--- a/base/properties.cpp
+++ b/base/properties.cpp
@@ -21,10 +21,14 @@
 #include <sys/system_properties.h>
 #include <sys/_system_properties.h>
 
+#include <algorithm>
+#include <chrono>
 #include <string>
 
 #include <android-base/parseint.h>
 
+using namespace std::chrono_literals;
+
 namespace android {
 namespace base {
 
@@ -96,14 +100,41 @@ static void WaitForPropertyCallback(void* data_ptr, const char*, const char* val
   }
 }
 
-void WaitForProperty(const std::string& key, const std::string& expected_value) {
+// TODO: chrono_utils?
+static void DurationToTimeSpec(timespec& ts, std::chrono::nanoseconds d) {
+  auto s = std::chrono::duration_cast<std::chrono::seconds>(d);
+  auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(d - s);
+  ts.tv_sec = s.count();
+  ts.tv_nsec = ns.count();
+}
+
+static void UpdateTimeSpec(timespec& ts,
+                           const std::chrono::time_point<std::chrono::steady_clock>& timeout) {
+  auto now = std::chrono::steady_clock::now();
+  auto remaining_timeout = std::chrono::duration_cast<std::chrono::nanoseconds>(timeout - now);
+  if (remaining_timeout < 0ns) {
+    ts = { 0, 0 };
+  } else {
+    DurationToTimeSpec(ts, remaining_timeout);
+  }
+}
+
+bool WaitForProperty(const std::string& key,
+                     const std::string& expected_value,
+                     std::chrono::milliseconds relative_timeout) {
+  // TODO: boot_clock?
+  auto now = std::chrono::steady_clock::now();
+  std::chrono::time_point<std::chrono::steady_clock> absolute_timeout = now + relative_timeout;
+  timespec ts;
+
   // Find the property's prop_info*.
   const prop_info* pi;
   unsigned global_serial = 0;
   while ((pi = __system_property_find(key.c_str())) == nullptr) {
     // The property doesn't even exist yet.
     // Wait for a global change and then look again.
-    global_serial = __system_property_wait_any(global_serial);
+    UpdateTimeSpec(ts, absolute_timeout);
+    if (!__system_property_wait(nullptr, global_serial, &global_serial, &ts)) return false;
   }
 
   WaitForPropertyData data;
@@ -112,10 +143,12 @@ void WaitForProperty(const std::string& key, const std::string& expected_value)
   while (true) {
     // Check whether the property has the value we're looking for?
     __system_property_read_callback(pi, WaitForPropertyCallback, &data);
-    if (data.done) return;
+    if (data.done) return true;
 
-    // It didn't so wait for it to change before checking again.
-    __system_property_wait(pi, data.last_read_serial);
+    // It didn't, so wait for the property to change before checking again.
+    UpdateTimeSpec(ts, absolute_timeout);
+    uint32_t unused;
+    if (!__system_property_wait(pi, data.last_read_serial, &unused, &ts)) return false;
   }
 }
 
diff --git a/base/properties_test.cpp b/base/properties_test.cpp
index d8186be6e..c68c2f805 100644
--- a/base/properties_test.cpp
+++ b/base/properties_test.cpp
@@ -134,8 +134,19 @@ TEST(properties, WaitForProperty) {
     android::base::SetProperty("debug.libbase.WaitForProperty_test", "b");
   });
 
-  android::base::WaitForProperty("debug.libbase.WaitForProperty_test", "a");
+  ASSERT_TRUE(android::base::WaitForProperty("debug.libbase.WaitForProperty_test", "a", 1s));
   flag = true;
-  android::base::WaitForProperty("debug.libbase.WaitForProperty_test", "b");
+  ASSERT_TRUE(android::base::WaitForProperty("debug.libbase.WaitForProperty_test", "b", 1s));
   thread.join();
 }
+
+TEST(properties, WaitForProperty_timeout) {
+  auto t0 = std::chrono::steady_clock::now();
+  ASSERT_FALSE(android::base::WaitForProperty("debug.libbase.WaitForProperty_timeout_test", "a",
+                                              200ms));
+  auto t1 = std::chrono::steady_clock::now();
+
+  ASSERT_GE(std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0), 200ms);
+  // Upper bounds on timing are inherently flaky, but let's try...
+  ASSERT_LT(std::chrono::duration_cast<std::chrono::milliseconds>(t1 - t0), 600ms);
+}