[libc-commits] [libc] [libc][CndVar] switch to doubly-linked list and implement timeout (PR #191827)
via libc-commits
libc-commits at lists.llvm.org
Mon Apr 13 08:18:58 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Schrodinger ZHU Yifan (SchrodingerZhu)
<details>
<summary>Changes</summary>
Implement the cnd_timedwait entrypoint and annotate TODOs for further improvement.
---
Full diff: https://github.com/llvm/llvm-project/pull/191827.diff
14 Files Affected:
- (modified) libc/config/linux/aarch64/entrypoints.txt (+1)
- (modified) libc/config/linux/riscv/entrypoints.txt (+1)
- (modified) libc/config/linux/x86_64/entrypoints.txt (+1)
- (modified) libc/include/CMakeLists.txt (+1)
- (modified) libc/include/threads.yaml (+8)
- (modified) libc/src/__support/threads/CndVar.h (+33-7)
- (modified) libc/src/__support/threads/linux/CndVar.cpp (+75-29)
- (modified) libc/src/threads/CMakeLists.txt (+7)
- (added) libc/src/threads/cnd_timedwait.h (+22)
- (modified) libc/src/threads/linux/CMakeLists.txt (+12)
- (added) libc/src/threads/linux/cnd_timedwait.cpp (+53)
- (modified) libc/src/threads/linux/cnd_wait.cpp (+2-1)
- (modified) libc/test/integration/src/threads/CMakeLists.txt (+2)
- (modified) libc/test/integration/src/threads/cnd_test.cpp (+33)
``````````diff
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index c0d2f6c39f9f9..a8bf089a48001 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -1160,6 +1160,7 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.threads.cnd_destroy
libc.src.threads.cnd_init
libc.src.threads.cnd_signal
+ libc.src.threads.cnd_timedwait
libc.src.threads.cnd_wait
libc.src.threads.mtx_destroy
libc.src.threads.mtx_init
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index fe393241ce8d0..f901bad85f4d1 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -1283,6 +1283,7 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.threads.cnd_destroy
libc.src.threads.cnd_init
libc.src.threads.cnd_signal
+ libc.src.threads.cnd_timedwait
libc.src.threads.cnd_wait
libc.src.threads.mtx_destroy
libc.src.threads.mtx_init
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 184aa57016ce5..29a68e73fee8f 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -1351,6 +1351,7 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.threads.cnd_destroy
libc.src.threads.cnd_init
libc.src.threads.cnd_signal
+ libc.src.threads.cnd_timedwait
libc.src.threads.cnd_wait
libc.src.threads.mtx_destroy
libc.src.threads.mtx_init
diff --git a/libc/include/CMakeLists.txt b/libc/include/CMakeLists.txt
index 07a39c045178e..6543bd972a956 100644
--- a/libc/include/CMakeLists.txt
+++ b/libc/include/CMakeLists.txt
@@ -331,6 +331,7 @@ add_header_macro(
.llvm-libc-types.thrd_start_t
.llvm-libc-types.tss_t
.llvm-libc-types.tss_dtor_t
+ .llvm-libc-types.struct_timespec
)
add_header_macro(
diff --git a/libc/include/threads.yaml b/libc/include/threads.yaml
index 6f7796e090ab6..461be4eb03992 100644
--- a/libc/include/threads.yaml
+++ b/libc/include/threads.yaml
@@ -63,6 +63,14 @@ functions:
return_type: int
arguments:
- type: cnd_t *
+ - name: cnd_timedwait
+ standards:
+ - stdc
+ return_type: int
+ arguments:
+ - type: cnd_t *
+ - type: mtx_t *
+ - type: const struct timespec *
- name: cnd_wait
standards:
- stdc
diff --git a/libc/src/__support/threads/CndVar.h b/libc/src/__support/threads/CndVar.h
index 406bcf193704e..4aa6c9dbd24c3 100644
--- a/libc/src/__support/threads/CndVar.h
+++ b/libc/src/__support/threads/CndVar.h
@@ -10,43 +10,69 @@
#define LLVM_LIBC___SUPPORT_SRC_THREADS_LINUX_CNDVAR_H
#include "hdr/stdint_proxy.h" // uint32_t
+#include "src/__support/CPP/optional.h"
#include "src/__support/macros/config.h"
#include "src/__support/threads/linux/futex_utils.h" // Futex
#include "src/__support/threads/mutex.h" // Mutex
#include "src/__support/threads/raw_mutex.h" // RawMutex
+#include "src/__support/time/abs_timeout.h"
namespace LIBC_NAMESPACE_DECL {
+// TODO: this implementation seems to be contention heavy. We probably want to
+// use a more efficient implementation.
+// TODO: thread local stack is not accessible in shared mode. We need to either
+// fallback to single futex word under shared memory or use a totally
+// different implementation.
+// TODO: node clean up needs to be registered into pthread cancellation point
+// cleanup routine once that functionality is available.
class CndVar {
enum CndWaiterStatus : uint32_t {
WS_Waiting = 0xE,
WS_Signalled = 0x5,
};
- struct CndWaiter {
+ struct WQNode {
+ WQNode *prev;
+ WQNode *next;
+ };
+
+ struct CndWaiter : WQNode {
Futex futex_word = WS_Waiting;
- CndWaiter *next = nullptr;
};
- CndWaiter *waitq_front;
- CndWaiter *waitq_back;
+ WQNode waitq;
RawMutex qmtx;
public:
+ enum class Result {
+ Success,
+ MutexError,
+ Timeout,
+ };
+
LIBC_INLINE static int init(CndVar *cv) {
- cv->waitq_front = cv->waitq_back = nullptr;
+ cv->waitq.prev = cv->waitq.next = nullptr;
RawMutex::init(&cv->qmtx);
return 0;
}
LIBC_INLINE static void destroy(CndVar *cv) {
- cv->waitq_front = cv->waitq_back = nullptr;
+ cv->waitq.prev = cv->waitq.next = nullptr;
}
// Returns 0 on success, -1 on error.
- int wait(Mutex *m);
+ Result wait(Mutex *m,
+ cpp::optional<internal::AbsTimeout> timeout = cpp::nullopt);
void notify_one();
void broadcast();
+
+private:
+ void ensure_cyclic_queue();
+ void push_back(CndWaiter *w);
+ CndWaiter *pop_front();
+ void remove(CndWaiter *w);
+ CndWaiter *take_all();
};
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/threads/linux/CndVar.cpp b/libc/src/__support/threads/linux/CndVar.cpp
index 60424673e819c..d3b1230c164d0 100644
--- a/libc/src/__support/threads/linux/CndVar.cpp
+++ b/libc/src/__support/threads/linux/CndVar.cpp
@@ -13,12 +13,17 @@
#include "src/__support/threads/linux/futex_word.h" // FutexWordType
#include "src/__support/threads/mutex.h" // Mutex
#include "src/__support/threads/raw_mutex.h" // RawMutex
-
+#include "src/__support/time/monotonicity.h"
#include <sys/syscall.h> // For syscall numbers.
+#ifndef LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY
+#define LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY 1
+#endif
+
namespace LIBC_NAMESPACE_DECL {
-int CndVar::wait(Mutex *m) {
+CndVar::Result CndVar::wait(Mutex *m,
+ cpp::optional<internal::AbsTimeout> timeout) {
// The goal is to perform "unlock |m| and wait" in an
// atomic operation. However, it is not possible to do it
// in the true sense so we do it in spirit. Before unlocking
@@ -27,18 +32,10 @@ int CndVar::wait(Mutex *m) {
// the waiter before the waiter actually starts waiting, the
// wait operation will not begin at all and the waiter immediately
// returns.
-
CndWaiter waiter;
{
cpp::lock_guard ml(qmtx);
- CndWaiter *old_back = nullptr;
- if (waitq_front == nullptr) {
- waitq_front = waitq_back = &waiter;
- } else {
- old_back = waitq_back;
- waitq_back->next = &waiter;
- waitq_back = &waiter;
- }
+ push_back(&waiter);
if (m->unlock() != MutexError::NONE) {
// If we do not remove the queued up waiter before returning,
@@ -46,36 +43,40 @@ int CndVar::wait(Mutex *m) {
// waiter. Note also that we do this with |qmtx| locked. This
// ensures that another thread will not signal the withdrawing
// waiter.
- waitq_back = old_back;
- if (waitq_back == nullptr)
- waitq_front = nullptr;
- else
- waitq_back->next = nullptr;
+ remove(&waiter);
- return -1;
+ return Result::MutexError;
}
}
-
- waiter.futex_word.wait(WS_Waiting, cpp::nullopt, true);
+#if LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY
+ if (timeout.has_value())
+ internal::ensure_monotonicity(*timeout);
+#endif
+ Result res = Result::Success;
+ if (waiter.futex_word.wait(WS_Waiting, timeout, true) == -ETIMEDOUT) {
+ cpp::lock_guard ml(qmtx);
+ remove(&waiter);
+ // POSIX.1-2024 says the following:
+ // "When such timeouts occur, pthread_cond_clockwait() shall nonetheless
+ // release and re-acquire the mutex referenced by mutex, and may consume a
+ // condition signal directed concurrently at the condition variable."
+ res = Result::Timeout;
+ }
// At this point, if locking |m| fails, we can simply return as the
// queued up waiter would have been removed from the queue.
auto err = m->lock();
- return err == MutexError::NONE ? 0 : -1;
+ return err == MutexError::NONE ? res : Result::MutexError;
}
void CndVar::notify_one() {
// We don't use an RAII locker in this method as we want to unlock
// |qmtx| and signal the waiter using a single FUTEX_WAKE_OP signal.
qmtx.lock();
- if (waitq_front == nullptr)
+ CndWaiter *first = pop_front();
+ if (first == nullptr)
qmtx.unlock();
- CndWaiter *first = waitq_front;
- waitq_front = waitq_front->next;
- if (waitq_front == nullptr)
- waitq_back = nullptr;
-
qmtx.reset();
// this is a special WAKE_OP, so we use syscall directly
@@ -86,10 +87,12 @@ void CndVar::notify_one() {
}
void CndVar::broadcast() {
+ // TODO: currently, we need to hold lock until broadcast is done to avoid
+ // timeout race condition. We could mimic musl to alternate the list state
+ // first and then wake or requeue after releasing the lock.
cpp::lock_guard ml(qmtx);
+ CndWaiter *waiter = take_all();
uint32_t dummy_futex_word;
- CndWaiter *waiter = waitq_front;
- waitq_front = waitq_back = nullptr;
while (waiter != nullptr) {
// FUTEX_WAKE_OP is used instead of just FUTEX_WAKE as it allows us to
// atomically update the waiter status to WS_Signalled before waking
@@ -99,8 +102,51 @@ void CndVar::broadcast() {
FUTEX_SYSCALL_ID, &dummy_futex_word, FUTEX_WAKE_OP, 1, 1,
&waiter->futex_word.val,
FUTEX_OP(FUTEX_OP_SET, WS_Signalled, FUTEX_OP_CMP_EQ, WS_Waiting));
- waiter = waiter->next;
+ waiter = static_cast<CndWaiter *>(waiter->next);
}
}
+void CndVar::ensure_cyclic_queue() {
+ if (waitq.prev || waitq.next)
+ return;
+ waitq.prev = waitq.next = &waitq;
+}
+
+void CndVar::push_back(CndWaiter *w) {
+ ensure_cyclic_queue();
+ w->next = &waitq;
+ w->prev = waitq.prev;
+ w->next->prev = w;
+ w->prev->next = w;
+}
+
+CndVar::CndWaiter *CndVar::pop_front() {
+ ensure_cyclic_queue();
+ if (waitq.next == &waitq)
+ return nullptr;
+ CndWaiter *first = static_cast<CndWaiter *>(waitq.next);
+ remove(first);
+ return first;
+}
+
+CndVar::CndWaiter *CndVar::take_all() {
+ ensure_cyclic_queue();
+ if (waitq.next == &waitq)
+ return nullptr;
+ waitq.next->prev = nullptr;
+ waitq.prev->next = nullptr;
+ CndVar::CndWaiter *first = static_cast<CndWaiter *>(waitq.next);
+ waitq.next = waitq.prev = &waitq;
+ return first;
+}
+
+void CndVar::remove(CndWaiter *w) {
+ // node may have already been removed from the queue
+ if (w->next == nullptr || w->prev == nullptr)
+ return;
+ w->next->prev = w->prev;
+ w->prev->next = w->next;
+ w->next = w->prev = nullptr;
+}
+
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/threads/CMakeLists.txt b/libc/src/threads/CMakeLists.txt
index 17dea3920a07a..925f77235a6d1 100644
--- a/libc/src/threads/CMakeLists.txt
+++ b/libc/src/threads/CMakeLists.txt
@@ -193,6 +193,13 @@ add_entrypoint_object(
.${LIBC_TARGET_OS}.cnd_wait
)
+add_entrypoint_object(
+ cnd_timedwait
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.cnd_timedwait
+)
+
add_entrypoint_object(
cnd_signal
ALIAS
diff --git a/libc/src/threads/cnd_timedwait.h b/libc/src/threads/cnd_timedwait.h
new file mode 100644
index 0000000000000..3056b8bbddeaf
--- /dev/null
+++ b/libc/src/threads/cnd_timedwait.h
@@ -0,0 +1,22 @@
+//===-- Implementation header for cnd_timedwait function -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_THREADS_CND_TIMEDWAIT_H
+#define LLVM_LIBC_SRC_THREADS_CND_TIMEDWAIT_H
+
+#include "src/__support/macros/config.h"
+
+#include <threads.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+int cnd_timedwait(cnd_t *cond, mtx_t *mutex, const struct timespec *time_point);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_THREADS_CND_TIMEDWAIT_H
diff --git a/libc/src/threads/linux/CMakeLists.txt b/libc/src/threads/linux/CMakeLists.txt
index 3cbf2f85f3f9d..2311d4aa7c35f 100644
--- a/libc/src/threads/linux/CMakeLists.txt
+++ b/libc/src/threads/linux/CMakeLists.txt
@@ -47,6 +47,18 @@ add_entrypoint_object(
libc.src.__support.threads.CndVar
)
+add_entrypoint_object(
+ cnd_timedwait
+ SRCS
+ cnd_timedwait.cpp
+ HDRS
+ ../cnd_timedwait.h
+ DEPENDS
+ libc.include.threads
+ libc.src.__support.threads.mutex
+ libc.src.__support.threads.CndVar
+)
+
add_entrypoint_object(
cnd_signal
SRCS
diff --git a/libc/src/threads/linux/cnd_timedwait.cpp b/libc/src/threads/linux/cnd_timedwait.cpp
new file mode 100644
index 0000000000000..9e3b6455d52d9
--- /dev/null
+++ b/libc/src/threads/linux/cnd_timedwait.cpp
@@ -0,0 +1,53 @@
+//===-- Linux implementation of the cnd_timedwait function ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/threads/cnd_timedwait.h"
+
+#include "src/__support/common.h"
+#include "src/__support/libc_assert.h"
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/null_check.h"
+#include "src/__support/macros/optimization.h"
+#include "src/__support/threads/CndVar.h"
+#include "src/__support/threads/mutex.h"
+#include "src/__support/time/abs_timeout.h"
+
+#include <threads.h> // cnd_t, mtx_t, thrd_error, thrd_success, thrd_timedout
+
+namespace LIBC_NAMESPACE_DECL {
+
+static_assert(sizeof(CndVar) == sizeof(cnd_t));
+
+LLVM_LIBC_FUNCTION(int, cnd_timedwait,
+ (cnd_t * cond, mtx_t *mtx,
+ const struct timespec *abs_time)) {
+ LIBC_CRASH_ON_NULLPTR(cond);
+ LIBC_CRASH_ON_NULLPTR(mtx);
+ LIBC_CRASH_ON_NULLPTR(abs_time);
+
+ auto timeout =
+ internal::AbsTimeout::from_timespec(*abs_time, /*is_realtime=*/true);
+ if (LIBC_UNLIKELY(!timeout.has_value()))
+ return timeout.error() == internal::AbsTimeout::Error::Invalid
+ ? thrd_error
+ : thrd_timedout;
+
+ CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
+ Mutex *mutex = reinterpret_cast<Mutex *>(mtx);
+ switch (cndvar->wait(mutex, timeout.value())) {
+ case CndVar::Result::Success:
+ return thrd_success;
+ case CndVar::Result::Timeout:
+ return thrd_timedout;
+ case CndVar::Result::MutexError:
+ return thrd_error;
+ }
+ __builtin_unreachable();
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/threads/linux/cnd_wait.cpp b/libc/src/threads/linux/cnd_wait.cpp
index 3633cc85277b9..d7d9687a144a1 100644
--- a/libc/src/threads/linux/cnd_wait.cpp
+++ b/libc/src/threads/linux/cnd_wait.cpp
@@ -21,7 +21,8 @@ static_assert(sizeof(CndVar) == sizeof(cnd_t));
LLVM_LIBC_FUNCTION(int, cnd_wait, (cnd_t * cond, mtx_t *mtx)) {
CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
Mutex *mutex = reinterpret_cast<Mutex *>(mtx);
- return cndvar->wait(mutex) ? thrd_error : thrd_success;
+ return cndvar->wait(mutex) == CndVar::Result::Success ? thrd_success
+ : thrd_error;
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/test/integration/src/threads/CMakeLists.txt b/libc/test/integration/src/threads/CMakeLists.txt
index d74b923751ad6..378d5fb16ab6a 100644
--- a/libc/test/integration/src/threads/CMakeLists.txt
+++ b/libc/test/integration/src/threads/CMakeLists.txt
@@ -108,6 +108,7 @@ add_integration_test(
libc.src.threads.cnd_broadcast
libc.src.threads.cnd_signal
libc.src.threads.cnd_destroy
+ libc.src.threads.cnd_timedwait
libc.src.threads.cnd_wait
libc.src.threads.mtx_destroy
libc.src.threads.mtx_init
@@ -116,4 +117,5 @@ add_integration_test(
libc.src.threads.thrd_create
libc.src.threads.thrd_join
libc.src.threads.linux.threads_utils
+ libc.src.time.clock_gettime
)
diff --git a/libc/test/integration/src/threads/cnd_test.cpp b/libc/test/integration/src/threads/cnd_test.cpp
index 4eaab9ac08bc1..a7c5a2424e484 100644
--- a/libc/test/integration/src/threads/cnd_test.cpp
+++ b/libc/test/integration/src/threads/cnd_test.cpp
@@ -11,6 +11,7 @@
#include "src/threads/cnd_destroy.h"
#include "src/threads/cnd_init.h"
#include "src/threads/cnd_signal.h"
+#include "src/threads/cnd_timedwait.h"
#include "src/threads/cnd_wait.h"
#include "src/threads/mtx_destroy.h"
#include "src/threads/mtx_init.h"
@@ -18,9 +19,12 @@
#include "src/threads/mtx_unlock.h"
#include "src/threads/thrd_create.h"
#include "src/threads/thrd_join.h"
+#include "src/time/clock_gettime.h"
#include "test/IntegrationTest/test.h"
+#include "hdr/time_macros.h"
+
#include <threads.h>
namespace wait_notify_broadcast_test {
@@ -146,8 +150,37 @@ void single_waiter_test() {
} // namespace single_waiter_test
+namespace timedwait_test {
+
+void timedwait_test() {
+ cnd_t cnd;
+ mtx_t mtx;
+
+ ASSERT_EQ(LIBC_NAMESPACE::cnd_init(&cnd), int(thrd_success));
+ ASSERT_EQ(LIBC_NAMESPACE::mtx_init(&mtx, mtx_plain), int(thrd_success));
+ ASSERT_EQ(LIBC_NAMESPACE::mtx_lock(&mtx), int(thrd_success));
+
+ timespec timeout;
+ ASSERT_EQ(LIBC_NAMESPACE::clock_gettime(CLOCK_REALTIME, &timeout), 0);
+ timeout.tv_nsec -= 1;
+ if (timeout.tv_nsec < 0) {
+ timeout.tv_nsec = 999999999;
+ timeout.tv_sec -= 1;
+ }
+
+ ASSERT_EQ(LIBC_NAMESPACE::cnd_timedwait(&cnd, &mtx, &timeout),
+ int(thrd_timedout));
+ ASSERT_EQ(LIBC_NAMESPACE::mtx_unlock(&mtx), int(thrd_success));
+
+ LIBC_NAMESPACE::mtx_destroy(&mtx);
+ LIBC_NAMESPACE::cnd_destroy(&cnd);
+}
+
+} // namespace timedwait_test
+
TEST_MAIN() {
wait_notify_broadcast_test::wait_notify_broadcast_test();
single_waiter_test::single_waiter_test();
+ timedwait_test::timedwait_test();
return 0;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/191827
More information about the libc-commits
mailing list