Skip to content

Commit 8256588

Browse files
authored
[libc++] fix counting_semaphore lost wakeups (#79265)
Fixes #77659 Fixes #46357 Picked up from https://reviews.llvm.org/D114119
1 parent 992d852 commit 8256588

File tree

3 files changed

+108
-20
lines changed

3 files changed

+108
-20
lines changed

libcxx/include/__atomic/atomic_sync.h

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,17 @@ __libcpp_atomic_monitor(__cxx_atomic_contention_t const volatile*);
4343
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_EXPORTED_FROM_ABI void
4444
__libcpp_atomic_wait(__cxx_atomic_contention_t const volatile*, __cxx_contention_t);
4545

46-
template <class _Atp, class _Fn>
46+
template <class _Atp, class _BackoffTest>
4747
struct __libcpp_atomic_wait_backoff_impl {
48-
_Atp* __a;
49-
_Fn __test_fn;
48+
_Atp* __a_;
49+
_BackoffTest __backoff_test_;
5050
_LIBCPP_AVAILABILITY_SYNC
5151
_LIBCPP_HIDE_FROM_ABI bool operator()(chrono::nanoseconds __elapsed) const {
5252
if (__elapsed > chrono::microseconds(64)) {
53-
auto const __monitor = std::__libcpp_atomic_monitor(__a);
54-
if (__test_fn())
53+
auto __monitor = std::__libcpp_atomic_monitor(__a_);
54+
if (__backoff_test_(__monitor))
5555
return true;
56-
std::__libcpp_atomic_wait(__a, __monitor);
56+
std::__libcpp_atomic_wait(__a_, __monitor);
5757
} else if (__elapsed > chrono::microseconds(4))
5858
__libcpp_thread_yield();
5959
else {
@@ -62,10 +62,26 @@ struct __libcpp_atomic_wait_backoff_impl {
6262
}
6363
};
6464

65-
template <class _Atp, class _Fn>
66-
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool __cxx_atomic_wait(_Atp* __a, _Fn&& __test_fn) {
67-
__libcpp_atomic_wait_backoff_impl<_Atp, __decay_t<_Fn> > __backoff_fn = {__a, __test_fn};
68-
return std::__libcpp_thread_poll_with_backoff(__test_fn, __backoff_fn);
65+
template <class _Atp, class _Poll, class _BackoffTest>
66+
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool
67+
__cxx_atomic_wait(_Atp* __a, _Poll&& __poll, _BackoffTest&& __backoff_test) {
68+
__libcpp_atomic_wait_backoff_impl<_Atp, __decay_t<_BackoffTest> > __backoff_fn = {__a, __backoff_test};
69+
return std::__libcpp_thread_poll_with_backoff(__poll, __backoff_fn);
70+
}
71+
72+
template <class _Poll>
73+
struct __libcpp_atomic_wait_poll_as_backoff_test {
74+
_Poll __poll_;
75+
76+
_LIBCPP_AVAILABILITY_SYNC
77+
_LIBCPP_HIDE_FROM_ABI bool operator()(__cxx_contention_t&) const { return __poll_(); }
78+
};
79+
80+
template <class _Atp, class _Poll>
81+
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool __cxx_atomic_wait(_Atp* __a, _Poll&& __poll) {
82+
__libcpp_atomic_wait_backoff_impl<_Atp, __libcpp_atomic_wait_poll_as_backoff_test<_Poll&> > __backoff_fn = {
83+
__a, {__poll}};
84+
return std::__libcpp_thread_poll_with_backoff(__poll, __backoff_fn);
6985
}
7086

7187
#else // _LIBCPP_HAS_NO_THREADS

libcxx/include/semaphore

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ using binary_semaphore = counting_semaphore<1>;
5454
#include <__assert> // all public C++ headers provide the assertion handler
5555
#include <__atomic/atomic_base.h>
5656
#include <__atomic/atomic_sync.h>
57+
#include <__atomic/contention_t.h>
5758
#include <__atomic/memory_order.h>
5859
#include <__availability>
5960
#include <__chrono/time_point.h>
@@ -94,31 +95,38 @@ public:
9495
auto __old = __a_.fetch_add(__update, memory_order_release);
9596
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
9697
__update <= _LIBCPP_SEMAPHORE_MAX - __old, "update is greater than the expected value");
97-
98-
if (__old > 0) {
99-
// Nothing to do
100-
} else if (__update > 1)
98+
if (__old == 0) {
10199
__a_.notify_all();
102-
else
103-
__a_.notify_one();
100+
}
104101
}
105102
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void acquire() {
106-
auto const __test_fn = [this]() -> bool {
103+
auto const __poll_fn = [this]() -> bool {
107104
auto __old = __a_.load(memory_order_relaxed);
108105
return (__old != 0) && __a_.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed);
109106
};
110-
__cxx_atomic_wait(&__a_.__a_, __test_fn);
107+
auto const __backoff_test = [this](__cxx_contention_t& __monitor) -> bool {
108+
ptrdiff_t __old = __monitor;
109+
bool __r = __try_acquire_impl(__old);
110+
__monitor = static_cast<__cxx_contention_t>(__old);
111+
return __r;
112+
};
113+
__cxx_atomic_wait(&__a_.__a_, __poll_fn, __backoff_test);
111114
}
112115
template <class _Rep, class _Period>
113116
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool
114117
try_acquire_for(chrono::duration<_Rep, _Period> const& __rel_time) {
115118
if (__rel_time == chrono::duration<_Rep, _Period>::zero())
116119
return try_acquire();
117-
auto const __test_fn = [this]() { return try_acquire(); };
118-
return std::__libcpp_thread_poll_with_backoff(__test_fn, __libcpp_timed_backoff_policy(), __rel_time);
120+
auto const __poll_fn = [this]() { return try_acquire(); };
121+
return std::__libcpp_thread_poll_with_backoff(__poll_fn, __libcpp_timed_backoff_policy(), __rel_time);
119122
}
120123
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool try_acquire() {
121124
auto __old = __a_.load(memory_order_acquire);
125+
return __try_acquire_impl(__old);
126+
}
127+
128+
private:
129+
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool __try_acquire_impl(ptrdiff_t& __old) {
122130
while (true) {
123131
if (__old == 0)
124132
return false;
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
// UNSUPPORTED: no-threads
10+
// UNSUPPORTED: c++03, c++11, c++14, c++17
11+
12+
// XFAIL: availability-synchronization_library-missing
13+
14+
// This is a regression test for https://llvm.org/PR47013.
15+
16+
// <semaphore>
17+
18+
#include <barrier>
19+
#include <semaphore>
20+
#include <thread>
21+
#include <vector>
22+
23+
#include "make_test_thread.h"
24+
25+
static std::counting_semaphore<> s(0);
26+
static std::barrier<> b(8 + 1);
27+
28+
void acquire() {
29+
for (int i = 0; i < 10'000; ++i) {
30+
s.acquire();
31+
b.arrive_and_wait();
32+
}
33+
}
34+
35+
void release() {
36+
for (int i = 0; i < 10'000; ++i) {
37+
s.release(1);
38+
s.release(1);
39+
s.release(1);
40+
s.release(1);
41+
42+
s.release(1);
43+
s.release(1);
44+
s.release(1);
45+
s.release(1);
46+
47+
b.arrive_and_wait();
48+
}
49+
}
50+
51+
int main(int, char**) {
52+
for (int run = 0; run < 20; ++run) {
53+
std::vector<std::thread> threads;
54+
for (int i = 0; i < 8; ++i)
55+
threads.push_back(support::make_test_thread(acquire));
56+
57+
threads.push_back(support::make_test_thread(release));
58+
59+
for (auto& thread : threads)
60+
thread.join();
61+
}
62+
63+
return 0;
64+
}

0 commit comments

Comments
 (0)