[libcxx-commits] [libcxx] c92a253 - [libc++] Fix hang in counting_semaphore::try_acquire

[Arthur O'Dwyer via libcxx-commits]

Author: Arthur O'Dwyer
Date: 2021-11-05T15:57:46-04:00
New Revision: c92a253cf0ddcf905707b4e9265b42570ce409d9

URL: https://github.com/llvm/llvm-project/commit/c92a253cf0ddcf905707b4e9265b42570ce409d9
DIFF: https://github.com/llvm/llvm-project/commit/c92a253cf0ddcf905707b4e9265b42570ce409d9.diff

LOG: [libc++] Fix hang in counting_semaphore::try_acquire

Before this patch, `try_acquire` blocks instead of returning false.
This is because `__libcpp_thread_poll_with_backoff` interprets zero
as meaning infinite, causing `try_acquire` to wait indefinitely.

Thanks to Pablo Busse (pabusse) for the patch!

Differential Revision: https://reviews.llvm.org/D98334

Added: 
    

Modified: 
    libcxx/include/semaphore
    libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/semaphore b/libcxx/include/semaphore
index 1128b36d9527d..db03fb967ed17 100644
--- a/libcxx/include/semaphore
+++ b/libcxx/include/semaphore
@@ -105,17 +105,22 @@ public:
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     bool try_acquire_for(chrono::duration<Rep, Period> const& __rel_time)
     {
-        auto const __test_fn = [this]() -> bool {
-            auto __old = __a.load(memory_order_acquire);
-            while(1) {
-                if (__old == 0)
-                    return false;
-                if(__a.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed))
-                    return true;
-            }
-        };
+        if (__rel_time == chrono::duration<Rep, Period>::zero())
+            return try_acquire();
+        auto const __test_fn = [this]() { return try_acquire(); };
         return __libcpp_thread_poll_with_backoff(__test_fn, __libcpp_timed_backoff_policy(), __rel_time);
     }
+    _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
+    bool try_acquire()
+    {
+        auto __old = __a.load(memory_order_acquire);
+        while (true) {
+            if (__old == 0)
+                return false;
+            if (__a.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed))
+                return true;
+        }
+    }
 };
 
 #define _LIBCPP_SEMAPHORE_MAX (numeric_limits<ptr
diff _t>::max())
@@ -156,14 +161,14 @@ public:
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     bool try_acquire()
     {
-        return try_acquire_for(chrono::nanoseconds::zero());
+        return __semaphore.try_acquire();
     }
     template <class Clock, class Duration>
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     bool try_acquire_until(chrono::time_point<Clock, Duration> const& __abs_time)
     {
         auto const current = Clock::now();
-        if(current >= __abs_time)
+        if (current >= __abs_time)
             return try_acquire();
         else
             return try_acquire_for(__abs_time - current);

diff  --git a/libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp b/libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
index 974e3c366e906..c15b0515a345b 100644
--- a/libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
+++ b/libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
@@ -30,14 +30,17 @@ int main(int, char**)
   std::counting_semaphore<> s(1);
 
   assert(s.try_acquire());
+  assert(!s.try_acquire());
   s.release();
   assert(s.try_acquire());
+  assert(!s.try_acquire());
   s.release(2);
   std::thread t = support::make_test_thread([&](){
     assert(s.try_acquire());
   });
   t.join();
   assert(s.try_acquire());
+  assert(!s.try_acquire());
 
   return 0;
 }