diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp --- a/libc/src/__support/threads/linux/thread.cpp +++ b/libc/src/__support/threads/linux/thread.cpp @@ -323,6 +323,10 @@ } int Thread::join(ThreadReturnValue &retval) { + // Its UB either way, but we might as well make it fast :) + if (attrib->detach_state.load() == uint32_t(DetachState::DETACHED)) + return EINVAL; + wait(); if (attrib->style == ThreadStyle::POSIX) @@ -345,6 +349,7 @@ // If the thread was already detached, then the detach method should not // be called at all. If the thread is exiting, then we wait for it to exit // and free up resources. + // TODO: Should we handle this less destructively? Maybe just return EINVAL? wait(); cleanup_thread_resources(attrib); diff --git a/libc/src/__support/threads/thread.h b/libc/src/__support/threads/thread.h --- a/libc/src/__support/threads/thread.h +++ b/libc/src/__support/threads/thread.h @@ -169,7 +169,8 @@ int status = join(retval); if (status != 0) return status; - *val = retval.stdc_retval; + if (val != nullptr) + *val = retval.stdc_retval; return 0; } @@ -178,7 +179,8 @@ int status = join(retval); if (status != 0) return status; - *val = retval.posix_retval; + if (val != nullptr) + *val = retval.posix_retval; return 0; } diff --git a/libc/test/integration/src/pthread/CMakeLists.txt b/libc/test/integration/src/pthread/CMakeLists.txt --- a/libc/test/integration/src/pthread/CMakeLists.txt +++ b/libc/test/integration/src/pthread/CMakeLists.txt @@ -147,3 +147,23 @@ libc.src.__support.CPP.array libc.src.__support.CPP.new ) + +add_integration_test( + pthread_join_test + SUITE + libc-pthread-integration-tests + SRCS + pthread_join_test.cpp + DEPENDS + libc.include.pthread + libc.include.errno + libc.include.stdio + libc.src.pthread.pthread_create + libc.src.pthread.pthread_join + libc.src.pthread.pthread_attr_setdetachstate + libc.src.pthread.pthread_attr_init + libc.src.pthread.pthread_attr_destroy + libc.src.sys.random.getrandom + libc.src.__support.CPP.atomic + libc.src.__support.CPP.new +) diff --git a/libc/test/integration/src/pthread/pthread_join_test.cpp b/libc/test/integration/src/pthread/pthread_join_test.cpp new file mode 100644 --- /dev/null +++ b/libc/test/integration/src/pthread/pthread_join_test.cpp @@ -0,0 +1,130 @@ +//===-- Tests for pthread_create ------------------------------------------===// +// +// 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/pthread/pthread_attr_destroy.h" +#include "src/pthread/pthread_attr_init.h" +#include "src/pthread/pthread_attr_setdetachstate.h" +#include "src/pthread/pthread_create.h" +#include "src/pthread/pthread_join.h" + +#include "src/sys/random/getrandom.h" + +#include "src/__support/CPP/atomic.h" +#include "src/__support/CPP/new.h" + +#include "src/__support/threads/thread.h" +#include "src/errno/libc_errno.h" + +#include "test/IntegrationTest/test.h" + +#include // For EXEC_PAGESIZE. +#include + +struct test_thread_args { + pthread_t tid; + int detached; + volatile int done; + size_t spin_cnt; + void *ret; +}; +static __llvm_libc::AllocChecker ac; + +static void *func(void *arg) { + test_thread_args *th_arg = reinterpret_cast(arg); + volatile size_t spin_cnt = th_arg->spin_cnt; + void *ret = th_arg->ret; + + while (spin_cnt) { + --spin_cnt; + } + + th_arg->done = 1; + return ret; +} + +static void *simple_func(void *) { return nullptr; } + +static test_thread_args * +initialize_test_thread_args(size_t nthreads, unsigned detached_percent) { + test_thread_args *th_args = new (ac) test_thread_args[nthreads]; + for (size_t i = 0; i < nthreads; ++i) { + unsigned rval; + ASSERT_EQ(__llvm_libc::getrandom(&rval, sizeof(rval), 0), + static_cast(sizeof(rval))); + rval %= 100; + th_args[i].detached = detached_percent > rval; + th_args[i].done = 0; + th_args[i].spin_cnt = rval % (1000 * 1000); + + ASSERT_EQ(__llvm_libc::getrandom(&th_args[i].ret, sizeof(void *), 0), + static_cast(sizeof(void *))); + } + + pthread_attr_t attr; + ASSERT_EQ(__llvm_libc::pthread_attr_init(&attr), 0); + for (size_t i = 0; i < nthreads; ++i) { + ASSERT_EQ(__llvm_libc::pthread_attr_setdetachstate( + &attr, th_args[i].detached ? PTHREAD_CREATE_DETACHED + : PTHREAD_CREATE_JOINABLE), + 0); + + ASSERT_EQ( + __llvm_libc::pthread_create(&th_args[i].tid, &attr, func, + reinterpret_cast(th_args + i)), + 0); + } + return th_args; +} + +static void run_join_tests(size_t nthreads, bool join_as_completed) { + test_thread_args *th_args = initialize_test_thread_args(nthreads, 0); + for (size_t join_cnt = 0; join_cnt < nthreads;) { + for (size_t i = 0; i < nthreads; ++i) { + if (th_args[i].detached) { + if (th_args[i].done == 1) { + th_args[i].done = 2; + ++join_cnt; + } + } else { + if (!join_as_completed || th_args[i].done == 1) { + void *th_ret; + ASSERT_EQ(__llvm_libc::pthread_join(th_args[i].tid, &th_ret), 0); + ASSERT_EQ(libc_errno, 0); + + ASSERT_EQ(th_args[i].done, 1); + ASSERT_EQ(th_ret, th_args[i].ret); + + th_args[i].done = 2; + ++join_cnt; + } + } + } + } + delete th_args; +} + +static void null_join_test() { + pthread_t tid; + ASSERT_EQ(__llvm_libc::pthread_create(&tid, nullptr, simple_func, nullptr), + 0); + ASSERT_EQ(libc_errno, 0); + ASSERT_EQ(__llvm_libc::pthread_join(tid, nullptr), 0); + ASSERT_EQ(libc_errno, 0); +} + +TEST_MAIN() { + libc_errno = 0; + null_join_test(); + + for (size_t nthreads = 1; nthreads < 8; ++nthreads) { + run_join_tests(nthreads, true); + run_join_tests(nthreads, false); + } + + return 0; +}