[libcxx-commits] [PATCH] D98334: [libcxx] Fix hang in try_acquire with __atomic_semaphore_base

[Pablo Busse via Phabricator via libcxx-commits]

pabusse created this revision.
pabusse added reviewers: __simt__, mclow.lists, ldionne.
Herald added a subscriber: jfb.
pabusse requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

On platforms with _LIBCPP_NO_NATIVE_SEMAPHORES, 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 when __atomic_semaphore_base is used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98334

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


Index: libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
===================================================================
--- libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
+++ libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
@@ -34,14 +34,17 @@
   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;
 }
Index: libcxx/include/semaphore
===================================================================
--- libcxx/include/semaphore
+++ libcxx/include/semaphore
@@ -108,17 +108,22 @@
     _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     bool try_acquire_for(chrono::duration<Rep, Period> const& __rel_time)
     {
-        auto const __test_fn = [=]() -> 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 = [=]() -> bool { 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(1) {
+            if (__old == 0)
+                return false;
+            if(__a.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed))
+                return true;
+        }
+    }
 };
 
 #ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
@@ -161,6 +166,11 @@
     {
         return __libcpp_semaphore_wait_timed(&__semaphore, __rel_time);
     }
+    _LIBCPP_INLINE_VISIBILITY
+    bool try_acquire()
+    {
+        return try_acquire_for(chrono::nanoseconds::zero());
+    }
 };
 
 template<ptrdiff_t __least_max_value>
@@ -215,7 +225,7 @@
     _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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98334.329614.patch
Type: text/x-patch
Size: 2650 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20210310/69c87cf8/attachment.bin>