Skip to content

Commit c358979

Browse files
authored
[libc++] fix atomic::wait memory order (#146267)
Fixes #109290 See the GH issue for the details. In conclusion, we have two issues in the `atomic<T>::wait` when `T` does not match our `contention_t`: - We don't have a total single order which can leads to missed notification based on the Herd7 simulation on relaxed architectural like PowerPC - We assumed the platform wait (`futex_wait`/`__ulock_wait`) has seq_cst barrier internally but there is no such guarantee
1 parent ba7d78a commit c358979

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

libcxx/src/atomic.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ __libcpp_contention_monitor_for_wait(__cxx_atomic_contention_t volatile* /*__con
151151
static void __libcpp_contention_wait(__cxx_atomic_contention_t volatile* __contention_state,
152152
__cxx_atomic_contention_t const volatile* __platform_state,
153153
__cxx_contention_t __old_value) {
154-
__cxx_atomic_fetch_add(__contention_state, __cxx_contention_t(1), memory_order_seq_cst);
154+
__cxx_atomic_fetch_add(__contention_state, __cxx_contention_t(1), memory_order_relaxed);
155+
// https://github.com/llvm/llvm-project/issues/109290
156+
// There are no platform guarantees of a memory barrier in the platform wait implementation
157+
__cxx_atomic_thread_fence(memory_order_seq_cst);
155158
// We sleep as long as the monitored value hasn't changed.
156159
__libcpp_platform_wait_on_address(__platform_state, __old_value);
157160
__cxx_atomic_fetch_sub(__contention_state, __cxx_contention_t(1), memory_order_release);
@@ -163,7 +166,7 @@ static void __libcpp_contention_wait(__cxx_atomic_contention_t volatile* __conte
163166
static void __libcpp_atomic_notify(void const volatile* __location) {
164167
auto const __entry = __libcpp_contention_state(__location);
165168
// The value sequence laundering happens on the next line below.
166-
__cxx_atomic_fetch_add(&__entry->__platform_state, __cxx_contention_t(1), memory_order_release);
169+
__cxx_atomic_fetch_add(&__entry->__platform_state, __cxx_contention_t(1), memory_order_seq_cst);
167170
__libcpp_contention_notify(
168171
&__entry->__contention_state,
169172
&__entry->__platform_state,

0 commit comments

Comments
 (0)