[libc][darwin] use os_sync_*_timeouts and clear tests

Author: bojle

libc/src/__support/threads/darwin/futex_utils.h

@@ -1,11 +1,10 @@
-//===--- Futex utils for Darwin -----------------------------------*- C++
-//-*-===//
+//===--- Futex utils for Darwin ------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
-//===----------------------------------------------------------------------===//
+//===------------------------------------------------------------===//
 
 #ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_DARWIN_FUTEX_UTILS_H
 #define LLVM_LIBC_SRC___SUPPORT_THREADS_DARWIN_FUTEX_UTILS_H
@@ -25,39 +24,52 @@ struct Futex : public cpp::Atomic<FutexWordType> {
   using cpp::Atomic<FutexWordType>::Atomic;
   using Timeout = internal::AbsTimeout;
 
-  // The Darwin futex API does not return a value on timeout, so we have to
-  // check for it manually. This means we can't use the return value to
-  // distinguish between a timeout and a successful wake-up.
-  int wait(FutexWordType val, cpp::optional<Timeout> timeout, bool) {
-    if (timeout) {
-      struct timespec now;
-      clock_gettime(timeout->is_realtime() ? CLOCK_REALTIME : CLOCK_MONOTONIC,
-                    &now);
-      const timespec &target_ts = timeout->get_timespec();
+  LIBC_INLINE long wait(FutexWordType val, cpp::optional<Timeout> timeout,
+                        bool /* is_shared */) {
+    // TODO(bojle): consider using OS_SYNC_WAIT_ON_ADDRESS_SHARED to sync
+    // betweeen processes. Catch: it is recommended to only be used by shared
+    // processes, not threads of a same process.
 
-      if (now.tv_sec > target_ts.tv_sec ||
-          (now.tv_sec == target_ts.tv_sec && now.tv_nsec >= target_ts.tv_nsec))
-        return ETIMEDOUT;
+    for (;;) {
+      if (this->load(cpp::MemoryOrder::RELAXED) != val)
+        return 0;
+      long ret = 0;
+      if (timeout) {
+        // Assuming, OS_CLOCK_MACH_ABSOLUTE_TIME is equivalent to CLOCK_REALTIME
+        uint64_t tnsec = timeout->get_timespec().tv_sec * 1000000000 +
+                         timeout->get_timespec().tv_nsec;
+        ret = os_sync_wait_on_address_with_timeout(
+            reinterpret_cast<void *>(this), static_cast<uint64_t>(val),
+            sizeof(FutexWordType), OS_SYNC_WAIT_ON_ADDRESS_NONE,
+            OS_CLOCK_MACH_ABSOLUTE_TIME, tnsec);
+      } else {
+        ret = os_sync_wait_on_address(
+            reinterpret_cast<void *>(this), static_cast<uint64_t>(val),
+            sizeof(FutexWordType), OS_SYNC_WAIT_ON_ADDRESS_NONE);
+      }
+      if ((ret < 0) && (errno == ETIMEDOUT)) {
+        return -ETIMEDOUT;
+      }
+      // case when os_sync returns early with an error. retry.
+      if ((ret < 0) && ((errno == EINTR) || (errno == EFAULT))) {
+        continue;
+      }
+      return ret;
     }
-
-    os_sync_wait_on_address(reinterpret_cast<void *>(this),
-                            static_cast<uint64_t>(val), sizeof(FutexWordType),
-                            OS_SYNC_WAIT_ON_ADDRESS_NONE);
-    return 0;
   }
 
-  void notify_one(bool) {
-    os_sync_wake_by_address_any(reinterpret_cast<void *>(this),
-                                sizeof(FutexWordType),
-                                OS_SYNC_WAKE_BY_ADDRESS_NONE);
+  LIBC_INLINE long notify_one(bool /* is_shared */) {
+    // TODO(bojle): deal with is_shared
+    return os_sync_wake_by_address_any(reinterpret_cast<void *>(this),
+                                       sizeof(FutexWordType),
+                                       OS_SYNC_WAKE_BY_ADDRESS_NONE);
   }
 
-  void notify_all(bool) {
-    // os_sync_wake_by_address_all is not available, so we use notify_one.
-    // This is not ideal, but it's the best we can do with the available API.
-    os_sync_wake_by_address_any(reinterpret_cast<void *>(this),
-                                sizeof(FutexWordType),
-                                OS_SYNC_WAKE_BY_ADDRESS_NONE);
+  LIBC_INLINE long notify_all(bool /* is_shared */) {
+    // TODO(bojle): deal with is_shared
+    return os_sync_wake_by_address_all(reinterpret_cast<void *>(this),
+                                       sizeof(FutexWordType),
+                                       OS_SYNC_WAKE_BY_ADDRESS_NONE);
   }
 };
 

libc/src/__support/threads/raw_mutex.h

@@ -18,6 +18,8 @@
 #include "src/__support/time/abs_timeout.h"
 #include "sys/errno.h"
 
+#include <stdio.h>
+
 #if defined(__linux__)
 #include "src/__support/threads/linux/futex_utils.h"
 #elif defined(__APPLE__)

libc/test/src/__support/threads/darwin/mutex_test.cpp

@@ -13,7 +13,6 @@
 
 TEST(LlvmLibcSupportThreadsMutexTest, SmokeTest) {
   LIBC_NAMESPACE::Mutex mutex(0, 0, 0, 0);
-  ;
   ASSERT_EQ(mutex.lock(), LIBC_NAMESPACE::MutexError::NONE);
   ASSERT_EQ(mutex.unlock(), LIBC_NAMESPACE::MutexError::NONE);
   ASSERT_EQ(mutex.try_lock(), LIBC_NAMESPACE::MutexError::NONE);
@@ -22,4 +21,25 @@ TEST(LlvmLibcSupportThreadsMutexTest, SmokeTest) {
   ASSERT_EQ(mutex.unlock(), LIBC_NAMESPACE::MutexError::UNLOCK_WITHOUT_LOCK);
 }
 
+TEST(LlvmLibcSupportThreadsRawMutexTest, Timeout) {
+  LIBC_NAMESPACE::RawMutex mutex;
+  ASSERT_TRUE(mutex.lock());
+  timespec ts;
+  LIBC_NAMESPACE::internal::clock_gettime(CLOCK_MONOTONIC, &ts);
+  ts.tv_sec += 1;
+  // Timeout will be respected when deadlock happens.
+  auto timeout = LIBC_NAMESPACE::internal::AbsTimeout::from_timespec(ts, false);
+  ASSERT_TRUE(timeout.has_value());
+  // The following will timeout
+  ASSERT_FALSE(mutex.lock(*timeout));
+  ASSERT_TRUE(mutex.unlock());
+  // Test that the mutex works after the timeout.
+  ASSERT_TRUE(mutex.lock());
+  ASSERT_TRUE(mutex.unlock());
+  // If a lock can be acquired directly, expired timeout will not count.
+  // Notice that the timeout is already reached during preivous deadlock.
+  ASSERT_TRUE(mutex.lock(*timeout));
+  ASSERT_TRUE(mutex.unlock());
+}
+
 // TODO(bojle): add other tests a la linux