[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
This commit is contained in:
committed by
GitHub
parent
8f45c1e718
commit
289b60430e
@@ -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``
|
The standard requires that ``twalk``, ``twalk_r``, and ``tdestroy``
|
||||||
to be used with a valid function pointer. LLVM-libc follows the behavior of
|
to be used with a valid function pointer. LLVM-libc follows the behavior of
|
||||||
configured via the `LIBC_ADD_NULL_CHECKS` option.
|
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.
|
||||||
|
|||||||
@@ -33,6 +33,7 @@ add_object_library(
|
|||||||
libc.config.app_h
|
libc.config.app_h
|
||||||
libc.include.sys_syscall
|
libc.include.sys_syscall
|
||||||
libc.hdr.fcntl_macros
|
libc.hdr.fcntl_macros
|
||||||
|
libc.hdr.errno_macros
|
||||||
libc.src.errno.errno
|
libc.src.errno.errno
|
||||||
libc.src.__support.CPP.atomic
|
libc.src.__support.CPP.atomic
|
||||||
libc.src.__support.CPP.stringstream
|
libc.src.__support.CPP.stringstream
|
||||||
|
|||||||
@@ -22,6 +22,7 @@
|
|||||||
#include <arm_acle.h>
|
#include <arm_acle.h>
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
#include "hdr/errno_macros.h"
|
||||||
#include "hdr/fcntl_macros.h"
|
#include "hdr/fcntl_macros.h"
|
||||||
#include "hdr/stdint_proxy.h"
|
#include "hdr/stdint_proxy.h"
|
||||||
#include <linux/param.h> // For EXEC_PAGESIZE.
|
#include <linux/param.h> // For EXEC_PAGESIZE.
|
||||||
@@ -282,6 +283,7 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
|
|||||||
attrib->owned_stack = owned_stack;
|
attrib->owned_stack = owned_stack;
|
||||||
attrib->tls = tls.addr;
|
attrib->tls = tls.addr;
|
||||||
attrib->tls_size = tls.size;
|
attrib->tls_size = tls.size;
|
||||||
|
attrib->joiner = nullptr;
|
||||||
|
|
||||||
start_args->thread_attrib = attrib;
|
start_args->thread_attrib = attrib;
|
||||||
start_args->runner = runner;
|
start_args->runner = runner;
|
||||||
@@ -340,6 +342,26 @@ int Thread::run(ThreadStyle style, ThreadRunner runner, void *arg, void *stack,
|
|||||||
}
|
}
|
||||||
|
|
||||||
int Thread::join(ThreadReturnValue &retval) {
|
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();
|
wait();
|
||||||
|
|
||||||
if (attrib->style == ThreadStyle::POSIX)
|
if (attrib->style == ThreadStyle::POSIX)
|
||||||
|
|||||||
@@ -109,12 +109,13 @@ struct alignas(STACK_ALIGNMENT) ThreadAttributes {
|
|||||||
ThreadReturnValue retval;
|
ThreadReturnValue retval;
|
||||||
ThreadAtExitCallbackMgr *atexit_callback_mgr;
|
ThreadAtExitCallbackMgr *atexit_callback_mgr;
|
||||||
void *platform_data;
|
void *platform_data;
|
||||||
|
cpp::Atomic<ThreadAttributes *> joiner;
|
||||||
|
|
||||||
LIBC_INLINE constexpr ThreadAttributes()
|
LIBC_INLINE constexpr ThreadAttributes()
|
||||||
: detach_state(uint32_t(DetachState::DETACHED)), stack(nullptr),
|
: detach_state(uint32_t(DetachState::DETACHED)), stack(nullptr),
|
||||||
stacksize(0), guardsize(0), tls(0), tls_size(0), owned_stack(false),
|
stacksize(0), guardsize(0), tls(0), tls_size(0), owned_stack(false),
|
||||||
tid(-1), style(ThreadStyle::POSIX), retval(),
|
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 *);
|
using TSSDtor = void(void *);
|
||||||
|
|||||||
@@ -245,11 +245,14 @@ add_integration_test(
|
|||||||
SRCS
|
SRCS
|
||||||
pthread_join_test.cpp
|
pthread_join_test.cpp
|
||||||
DEPENDS
|
DEPENDS
|
||||||
|
libc.src.__support.CPP.atomic
|
||||||
|
libc.src.__support.threads.thread
|
||||||
libc.include.pthread
|
libc.include.pthread
|
||||||
libc.include.stdio
|
libc.include.stdio
|
||||||
libc.src.errno.errno
|
libc.src.errno.errno
|
||||||
libc.src.pthread.pthread_create
|
libc.src.pthread.pthread_create
|
||||||
libc.src.pthread.pthread_join
|
libc.src.pthread.pthread_join
|
||||||
|
libc.src.pthread.pthread_self
|
||||||
)
|
)
|
||||||
|
|
||||||
add_integration_test(
|
add_integration_test(
|
||||||
|
|||||||
@@ -8,24 +8,101 @@
|
|||||||
|
|
||||||
#include "src/pthread/pthread_create.h"
|
#include "src/pthread/pthread_create.h"
|
||||||
#include "src/pthread/pthread_join.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 "test/IntegrationTest/test.h"
|
||||||
|
|
||||||
#include <errno.h>
|
#include <errno.h>
|
||||||
#include <pthread.h>
|
#include <pthread.h>
|
||||||
|
|
||||||
static void *simpleFunc(void *) { return nullptr; }
|
static void *simple_func(void *) { return nullptr; }
|
||||||
static void nullJoinTest() {
|
|
||||||
pthread_t Tid;
|
static void null_join_test() {
|
||||||
ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&Tid, nullptr, simpleFunc, nullptr),
|
pthread_t tid;
|
||||||
|
ASSERT_EQ(LIBC_NAMESPACE::pthread_create(&tid, nullptr, simple_func, nullptr),
|
||||||
0);
|
0);
|
||||||
ASSERT_ERRNO_SUCCESS();
|
ASSERT_ERRNO_SUCCESS();
|
||||||
ASSERT_EQ(LIBC_NAMESPACE::pthread_join(Tid, nullptr), 0);
|
ASSERT_EQ(LIBC_NAMESPACE::pthread_join(tid, nullptr), 0);
|
||||||
ASSERT_ERRNO_SUCCESS();
|
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<int> *ready_count;
|
||||||
|
LIBC_NAMESPACE::cpp::Atomic<int> *start;
|
||||||
|
LIBC_NAMESPACE::cpp::Atomic<int> *result;
|
||||||
|
int start_value;
|
||||||
|
};
|
||||||
|
|
||||||
|
static void *mutual_join_func(void *arg) {
|
||||||
|
auto *args = reinterpret_cast<MutualJoinArgs *>(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<LIBC_NAMESPACE::Thread *>(&joiner);
|
||||||
|
auto *target_thread = reinterpret_cast<LIBC_NAMESPACE::Thread *>(&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<int> ready_count(0);
|
||||||
|
LIBC_NAMESPACE::cpp::Atomic<int> start(0);
|
||||||
|
LIBC_NAMESPACE::cpp::Atomic<int> result1(-1);
|
||||||
|
LIBC_NAMESPACE::cpp::Atomic<int> 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() {
|
TEST_MAIN() {
|
||||||
errno = 0;
|
errno = 0;
|
||||||
nullJoinTest();
|
null_join_test();
|
||||||
|
self_join_test();
|
||||||
|
mutual_join_test();
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user