From 289b60430e10514dc1ba98ec2aea3226251f20ec Mon Sep 17 00:00:00 2001 From: Schrodinger ZHU Yifan Date: Wed, 29 Apr 2026 17:51:17 -0400 Subject: [PATCH] [libc][thread] detect self-join and mutual-join deadlock (#194891) Fix #194034. Detect the deadlock cases of mutual thread joining. Required by `libcxx/test/std/thread/thread.jthread/join.deadlock.pass.cpp` Assisted-by: Codex with gpt-5.5 high fast --- libc/docs/dev/undefined_behavior.rst | 19 ++++ .../__support/threads/linux/CMakeLists.txt | 1 + libc/src/__support/threads/linux/thread.cpp | 22 +++++ libc/src/__support/threads/thread.h | 3 +- .../integration/src/pthread/CMakeLists.txt | 3 + .../src/pthread/pthread_join_test.cpp | 89 +++++++++++++++++-- 6 files changed, 130 insertions(+), 7 deletions(-) diff --git a/libc/docs/dev/undefined_behavior.rst b/libc/docs/dev/undefined_behavior.rst index 59eafa4d86ff..c777e5c371a6 100644 --- a/libc/docs/dev/undefined_behavior.rst +++ b/libc/docs/dev/undefined_behavior.rst @@ -180,3 +180,22 @@ For LLVM-libc, `tdelete` returns bit-casted `uintptr_t`'s maximum value. The standard requires that ``twalk``, ``twalk_r``, and ``tdestroy`` to be used with a valid function pointer. LLVM-libc follows the behavior of configured via the `LIBC_ADD_NULL_CHECKS` option. + +Invalid Thread Joining Behavior +------------------------------------------------------ +POSIX standard does not demand accurate deadlock detection and leaves +repeated/concurrent joining as undefined behavior. In the following, +we discuss the related behaviors explicitly. + +Self joining is always rejected with ``EDEADLK``. + +At least one of the two threads in a mutual joining will detect the deadlock and +return ``EDEADLK``. If joining requester gets ``EDEADLK``, the joining target is +recovered to joinable state. However, the joining target may already be waiting +if it does not see ``EDEADLK``. + +Cyclic joining with more than two threads is not detected. + +Concurrent and repeated joinings on the same thread are faulty behaviors, because +target thread's TLS may already be torn down. ``EINVAL`` may be returned if +multiple joinings occur on the same thread but it is not guaranteed to observe. diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt index 5bd67c60ad88..b9273b42bb5f 100644 --- a/libc/src/__support/threads/linux/CMakeLists.txt +++ b/libc/src/__support/threads/linux/CMakeLists.txt @@ -33,6 +33,7 @@ add_object_library( libc.config.app_h libc.include.sys_syscall libc.hdr.fcntl_macros + libc.hdr.errno_macros libc.src.errno.errno libc.src.__support.CPP.atomic libc.src.__support.CPP.stringstream diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp index 4f1e9110457d..bdbb8aefeef0 100644 --- a/libc/src/__support/threads/linux/thread.cpp +++ b/libc/src/__support/threads/linux/thread.cpp @@ -22,6 +22,7 @@ #include #endif +#include "hdr/errno_macros.h" #include "hdr/fcntl_macros.h" #include "hdr/stdint_proxy.h" #include // For EXEC_PAGESIZE. @@ -282,6 +283,7 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack, attrib->owned_stack = owned_stack; attrib->tls = tls.addr; attrib->tls_size = tls.size; + attrib->joiner = nullptr; start_args->thread_attrib = attrib; start_args->runner = runner; @@ -340,6 +342,26 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack, } int Thread::join(ThreadReturnValue &retval) { + if (self.attrib) { + // Reject self join. + if (self.attrib == attrib) + return EDEADLK; + + // Do a best-effort check of concurrent/repeated join. + // This cmpxchg establishes exclusive joiner role by setting the joiner + // field iff there is no previous joiner + ThreadAttributes *expected = nullptr; + if (!attrib->joiner.compare_exchange_strong(expected, self.attrib, + cpp::MemoryOrder::ACQ_REL)) + return EINVAL; + + // Reject mutual join. + if (self.attrib->joiner.load(cpp::MemoryOrder::ACQUIRE) == attrib) { + attrib->joiner.store(nullptr, cpp::MemoryOrder::RELEASE); + return EDEADLK; + } + } + wait(); if (attrib->style == ThreadStyle::POSIX) diff --git a/libc/src/__support/threads/thread.h b/libc/src/__support/threads/thread.h index 6806098653b2..232b300bbba5 100644 --- a/libc/src/__support/threads/thread.h +++ b/libc/src/__support/threads/thread.h @@ -109,12 +109,13 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes { ThreadReturnValue retval; ThreadAtExitCallbackMgr *atexit_callback_mgr; void *platform_data; + cpp::Atomic joiner; LIBC_INLINE constexpr ThreadAttributes() : detach_state(uint32_t(DetachState::DETACHED)), stack(nullptr), stacksize(0), guardsize(0), tls(0), tls_size(0), owned_stack(false), tid(-1), style(ThreadStyle::POSIX), retval(), - atexit_callback_mgr(nullptr), platform_data(nullptr) {} + atexit_callback_mgr(nullptr), platform_data(nullptr), joiner(nullptr) {} }; using TSSDtor = void(void *); diff --git a/libc/test/integration/src/pthread/CMakeLists.txt b/libc/test/integration/src/pthread/CMakeLists.txt index eceebb42f18c..32ff223ffa98 100644 --- a/libc/test/integration/src/pthread/CMakeLists.txt +++ b/libc/test/integration/src/pthread/CMakeLists.txt @@ -245,11 +245,14 @@ add_integration_test( SRCS pthread_join_test.cpp DEPENDS + libc.src.__support.CPP.atomic + libc.src.__support.threads.thread libc.include.pthread libc.include.stdio libc.src.errno.errno libc.src.pthread.pthread_create libc.src.pthread.pthread_join + libc.src.pthread.pthread_self ) add_integration_test( diff --git a/libc/test/integration/src/pthread/pthread_join_test.cpp b/libc/test/integration/src/pthread/pthread_join_test.cpp index 6dea99de1a64..30b1cc93f4a5 100644 --- a/libc/test/integration/src/pthread/pthread_join_test.cpp +++ b/libc/test/integration/src/pthread/pthread_join_test.cpp @@ -8,24 +8,101 @@ #include "src/pthread/pthread_create.h" #include "src/pthread/pthread_join.h" +#include "src/pthread/pthread_self.h" +#include "src/__support/CPP/atomic.h" +#include "src/__support/threads/thread.h" #include "test/IntegrationTest/test.h" #include #include -static void *simpleFunc(void *) { return nullptr; } -static void nullJoinTest() { - pthread_t Tid; - ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&Tid, nullptr, simpleFunc, nullptr), +static void *simple_func(void *) { return nullptr; } + +static void null_join_test() { + pthread_t tid; + ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&tid, nullptr, simple_func, nullptr), 0); ASSERT_ERRNO_SUCCESS(); - ASSERT_EQ(LIBC_NAMESPACE::pthread_join(Tid, nullptr), 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_join(tid, nullptr), 0); ASSERT_ERRNO_SUCCESS(); } +static void self_join_test() { + ASSERT_EQ( + LIBC_NAMESPACE::pthread_join(LIBC_NAMESPACE::pthread_self(), nullptr), + EDEADLK); +} + +struct MutualJoinArgs { + pthread_t *peer; + LIBC_NAMESPACE::cpp::Atomic *ready_count; + LIBC_NAMESPACE::cpp::Atomic *start; + LIBC_NAMESPACE::cpp::Atomic *result; + int start_value; +}; + +static void *mutual_join_func(void *arg) { + auto *args = reinterpret_cast(arg); + args->ready_count->fetch_add(1); + while (args->start->load() < args->start_value) + ; // Spin until this thread is released to join its peer. + + args->result->store(LIBC_NAMESPACE::pthread_join(*args->peer, nullptr)); + return nullptr; +} + +static bool is_joining(pthread_t joiner, pthread_t target) { + auto *joiner_thread = reinterpret_cast(&joiner); + auto *target_thread = reinterpret_cast(&target); + return target_thread->attrib->joiner.load() == joiner_thread->attrib; +} + +static void mutual_join_test() { + pthread_t thread1; + pthread_t thread2; + LIBC_NAMESPACE::cpp::Atomic ready_count(0); + LIBC_NAMESPACE::cpp::Atomic start(0); + LIBC_NAMESPACE::cpp::Atomic result1(-1); + LIBC_NAMESPACE::cpp::Atomic result2(-1); + + MutualJoinArgs args1{&thread2, &ready_count, &start, &result1, 1}; + MutualJoinArgs args2{&thread1, &ready_count, &start, &result2, 2}; + + ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&thread1, nullptr, mutual_join_func, + &args1), + 0); + ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&thread2, nullptr, mutual_join_func, + &args2), + 0); + + while (ready_count.load() != 2) + ; // Spin until both threads are ready to join each other. + start.store(1); + while (!is_joining(thread1, thread2)) + ; // Spin until thread1 has started joining thread2. + start.store(2); + + while (result1.load() == -1 || result2.load() == -1) + ; // Spin until the successful joiner and deadlock loser have both exited. + + // A thread is recovered to joinable state if its joining requester gets + // EDEADLK. + bool thread1_joinable = result2.load() == EDEADLK; + bool thread2_joinable = result1.load() == EDEADLK; + + ASSERT_TRUE(thread1_joinable || thread2_joinable); + + if (thread1_joinable) + ASSERT_EQ(LIBC_NAMESPACE::pthread_join(thread1, nullptr), 0); + if (thread2_joinable) + ASSERT_EQ(LIBC_NAMESPACE::pthread_join(thread2, nullptr), 0); +} + TEST_MAIN() { errno = 0; - nullJoinTest(); + null_join_test(); + self_join_test(); + mutual_join_test(); return 0; }