From 13c79b34c243710f2fb259f9e9b3eabc28b57ca9 Mon Sep 17 00:00:00 2001 From: Peter Andreas Entschev Date: Mon, 4 Dec 2023 13:10:28 -0800 Subject: [PATCH 1/2] Use `notify_one()` in `CallbackNotifier` It is unclear why but for some reason `notify_all()` is causing futexes never to return in some situations. This occurs very frequently in CI and is also less frequently reproducible locally. --- cpp/src/utils/callback_notifier.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/utils/callback_notifier.cpp b/cpp/src/utils/callback_notifier.cpp index 6a786816..2d5812f9 100644 --- a/cpp/src/utils/callback_notifier.cpp +++ b/cpp/src/utils/callback_notifier.cpp @@ -46,7 +46,7 @@ void CallbackNotifier::set() // ordering. _flag.store(true, std::memory_order_relaxed); } - _conditionVariable.notify_all(); + _conditionVariable.notify_one(); } } From 54414d1aa01de13a288b87039f210f825a8511a9 Mon Sep 17 00:00:00 2001 From: Peter Andreas Entschev Date: Tue, 5 Dec 2023 02:13:25 -0800 Subject: [PATCH 2/2] Update `CallbackNotifier` docs to reflect change to `notify_one` --- cpp/include/ucxx/utils/callback_notifier.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/include/ucxx/utils/callback_notifier.h b/cpp/include/ucxx/utils/callback_notifier.h index 4c4e1cd4..ea5bea61 100644 --- a/cpp/include/ucxx/utils/callback_notifier.h +++ b/cpp/include/ucxx/utils/callback_notifier.h @@ -20,15 +20,15 @@ class CallbackNotifier { * @brief Construct a thread-safe notification object * * Construct a thread-safe notification object which can signal - * release of some shared state with `set()` while other threads - * block on `wait()` until the shared state is released. + * release of some shared state with `set()` while a single thread + * blocks on `wait()` until the shared state is released. * * If libc is glibc and the version is older than 2.25, the * implementation uses a spinlock otherwise it uses a condition * variable. * * When C++-20 is the minimum supported version, it should use - * atomic.wait + notify_all. + * atomic.wait + notify_one. */ CallbackNotifier() : _flag{false} {}; @@ -42,8 +42,8 @@ class CallbackNotifier { /** * @brief Notify waiting threads that we are done and they can proceed * - * Set the flag to true and notify others threads blocked by a call to `wait()`. - * See also `std::condition_variable::notify_all`. + * Set the flag to true and notify a single thread blocked by a call to `wait()`. + * See also `std::condition_variable::notify_one`. */ void set();