Skip to content

Commit

Permalink
[libc++] Fix hang in counting_semaphore::try_acquire
Browse files Browse the repository at this point in the history
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

(cherry picked from commit c92a253)
  • Loading branch information
Arthur O'Dwyer committed Nov 5, 2021
1 parent 00f64cc commit 216200a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
27 changes: 16 additions & 11 deletions libcxx/include/semaphore
Original file line number Diff line number Diff line change
Expand Up @@ -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<ptrdiff_t>::max())
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions libcxx/test/std/thread/thread.semaphore/try_acquire.pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,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;
}

0 comments on commit 216200a

Please sign in to comment.