[libc-commits] [libc] 5db4177 - [libc] Add pthread_detach and thrd_detach.

Siva Chandra Reddy via libc-commits libc-commits at lists.llvm.org
Fri Jun 10 22:29:55 PDT 2022


Author: Siva Chandra Reddy
Date: 2022-06-11T05:29:40Z
New Revision: 5db4177817a8ba0f9a55184da12581abc4b65d7f

URL: https://github.com/llvm/llvm-project/commit/5db4177817a8ba0f9a55184da12581abc4b65d7f
DIFF: https://github.com/llvm/llvm-project/commit/5db4177817a8ba0f9a55184da12581abc4b65d7f.diff

LOG: [libc] Add pthread_detach and thrd_detach.

Tests for pthread_detach and thrd_detach have not been added. Instead, a
test for the underlying implementation has been added as it makes use of
an internal wait method to synchronize with detached threads.

Reviewed By: lntue, michaelrj

Differential Revision: https://reviews.llvm.org/D127479

Added: 
    libc/src/pthread/pthread_detach.cpp
    libc/src/pthread/pthread_detach.h
    libc/src/threads/thrd_detach.cpp
    libc/src/threads/thrd_detach.h
    libc/test/src/__support/threads/CMakeLists.txt
    libc/test/src/__support/threads/thread_detach_test.cpp

Modified: 
    libc/config/linux/x86_64/entrypoints.txt
    libc/spec/posix.td
    libc/spec/stdc.td
    libc/src/__support/threads/CMakeLists.txt
    libc/src/__support/threads/linux/thread.h
    libc/src/__support/threads/thread_attrib.h
    libc/src/pthread/CMakeLists.txt
    libc/src/threads/CMakeLists.txt
    libc/test/src/__support/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index ed2e248d7652c..6e8cb2c1f1112 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -237,6 +237,7 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.pthread.pthread_attr_setstacksize
     libc.src.pthread.pthread_attr_init
     libc.src.pthread.pthread_create
+    libc.src.pthread.pthread_detach
     libc.src.pthread.pthread_join
     libc.src.pthread.pthread_mutex_destroy
     libc.src.pthread.pthread_mutex_init
@@ -305,6 +306,7 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.threads.mtx_lock
     libc.src.threads.mtx_unlock
     libc.src.threads.thrd_create
+    libc.src.threads.thrd_detach
     libc.src.threads.thrd_join
 
     # time.h entrypoints

diff  --git a/libc/spec/posix.td b/libc/spec/posix.td
index e53942dd850c4..915a177913402 100644
--- a/libc/spec/posix.td
+++ b/libc/spec/posix.td
@@ -461,6 +461,11 @@ def POSIX : StandardSpec<"POSIX"> {
           RetValSpec<IntType>,
           [ArgSpec<PThreadTType>, ArgSpec<VoidPtrPtr>]
       >,
+      FunctionSpec<
+          "pthread_detach",
+          RetValSpec<IntType>,
+          [ArgSpec<PThreadTType>]
+      >,
       FunctionSpec<
           "pthread_mutexattr_init",
           RetValSpec<IntType>,

diff  --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index e9b9a2d3b42fd..90218443d0991 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -801,6 +801,11 @@ def StdC : StandardSpec<"stdc"> {
                   ArgSpec<ThrdTTypePtr>,
                   ArgSpec<IntPtr>,
               ]
+          >,
+          FunctionSpec<
+              "thrd_detach",
+              RetValSpec<IntType>,
+              [ArgSpec<ThrdTType>]
           >
       ]
   >;

diff  --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt
index 26c2387faad38..5dce96cfc2ab2 100644
--- a/libc/src/__support/threads/CMakeLists.txt
+++ b/libc/src/__support/threads/CMakeLists.txt
@@ -8,6 +8,9 @@ add_header_library(
   thread_attrib
   HDRS
     thread_attrib.h
+  DEPENDS
+    libc.src.__support.common
+    libc.src.__support.CPP.atomic
 )
 
 if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})

diff  --git a/libc/src/__support/threads/linux/thread.h b/libc/src/__support/threads/linux/thread.h
index a354f2d5528c5..b389b8a4c9816 100644
--- a/libc/src/__support/threads/linux/thread.h
+++ b/libc/src/__support/threads/linux/thread.h
@@ -143,7 +143,8 @@ template <typename ReturnType> struct Thread {
 
     attrib = reinterpret_cast<ThreadAttributes<ReturnType> *>(
         adjusted_stack + sizeof(StartArgs<ReturnType>));
-    attrib->detached = detached;
+    attrib->detach_state =
+        uint32_t(detached ? DetachState::DETACHED : DetachState::JOINABLE);
     attrib->stack = stack;
     attrib->stack_size = size;
     attrib->owned_stack = owned_stack;
@@ -189,6 +190,48 @@ template <typename ReturnType> struct Thread {
   }
 
   int join(ReturnType *retval) {
+    wait();
+
+    *retval = attrib->retval;
+    if (attrib->owned_stack)
+      free_stack(attrib->stack, attrib->stack_size);
+
+    return 0;
+  }
+
+  // Detach a joinable thread.
+  //
+  // This method does not have error return value. However, the type of detach
+  // is returned to help with testing.
+  int detach() {
+    uint32_t joinable_state = uint32_t(DetachState::JOINABLE);
+    if (attrib->detach_state.compare_exchange_strong(
+            joinable_state, uint32_t(DetachState::DETACHED))) {
+      return int(DetachType::SIMPLE);
+    }
+
+    // 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.
+    wait();
+
+    if (attrib->owned_stack)
+      free_stack(attrib->stack, attrib->stack_size);
+    return int(DetachType::CLEANUP);
+  }
+
+  // Wait for the thread to finish. This method can only be called
+  // if:
+  // 1. A detached thread is guaranteed to be running.
+  // 2. A joinable thread has not been detached or joined. As long as it has
+  //    not been detached or joined, wait can be called multiple times.
+  //
+  // Also, only one thread can wait and expect to get woken up when the thread
+  // finishes.
+  //
+  // NOTE: This function is to be used for testing only. There is no standard
+  // which requires exposing it via a public API.
+  void wait() {
     // The kernel should set the value at the clear tid address to zero.
     // If not, it is a spurious wake and we should continue to wait on
     // the futex.
@@ -198,12 +241,6 @@ template <typename ReturnType> struct Thread {
       __llvm_libc::syscall(SYS_futex, &clear_tid->val, FUTEX_WAIT,
                            CLEAR_TID_VALUE, nullptr);
     }
-
-    *retval = attrib->retval;
-    if (!attrib->detached)
-      free_stack(attrib->stack, attrib->stack_size);
-
-    return 0;
   }
 };
 
@@ -212,12 +249,18 @@ __attribute__((noinline)) void Thread<ReturnType>::start_thread() {
   auto *start_args =
       reinterpret_cast<StartArgs<ReturnType> *>(get_start_args_addr());
   auto *thread = start_args->thread;
-  thread->attrib->retval = start_args->func(start_args->arg);
+  ReturnType retval = thread->attrib->retval =
+      start_args->func(start_args->arg);
 
-  if (thread->attrib->detached && thread->attrib->owned_stack)
-    free_stack(thread->attrib->stack, thread->attrib->stack_size);
+  uint32_t joinable_state = uint32_t(DetachState::JOINABLE);
+  if (!thread->attrib->detach_state.compare_exchange_strong(
+          joinable_state, uint32_t(DetachState::EXITING))) {
+    // Thread is detached so cleanup the resources.
+    if (thread->attrib->owned_stack)
+      free_stack(thread->attrib->stack, thread->attrib->stack_size);
+  }
 
-  __llvm_libc::syscall(SYS_exit, thread->attrib->retval);
+  __llvm_libc::syscall(SYS_exit, retval);
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/__support/threads/thread_attrib.h b/libc/src/__support/threads/thread_attrib.h
index c88652163d4da..31d97acd3fb90 100644
--- a/libc/src/__support/threads/thread_attrib.h
+++ b/libc/src/__support/threads/thread_attrib.h
@@ -9,8 +9,11 @@
 #ifndef LLVM_LIBC_SRC_SUPPORT_THREADS_THREAD_ATTRIB_H
 #define LLVM_LIBC_SRC_SUPPORT_THREADS_THREAD_ATTRIB_H
 
+#include "src/__support/CPP/atomic.h"
 #include "src/__support/architectures.h"
 
+#include <stdint.h>
+
 namespace __llvm_libc {
 
 #if (defined(LLVM_LIBC_ARCH_AARCH64) || defined(LLVM_LIBC_ARCH_X86_64))
@@ -18,6 +21,22 @@ constexpr unsigned int STACK_ALIGNMENT = 16;
 #endif
 // TODO: Provide stack alignment requirements for other architectures.
 
+enum class DetachState : uint32_t {
+  JOINABLE = 0x11,
+  EXITING = 0x22,
+  DETACHED = 0x33
+};
+
+// Detach type is useful in testing the detach operation.
+enum class DetachType : int {
+  // Indicates that the detach operation just set the detach state to DETACHED
+  // and returned.
+  SIMPLE = 1,
+
+  // Indicates that the detach operation performed thread cleanup.
+  CLEANUP = 2
+};
+
 // A data type to hold common thread attributes which have to be stored as
 // thread state. Note that this is 
diff erent from public attribute types like
 // pthread_attr_t which might contain information which need not be saved as
@@ -27,7 +46,25 @@ constexpr unsigned int STACK_ALIGNMENT = 16;
 // for the target architecture.
 template <typename ReturnType>
 struct alignas(STACK_ALIGNMENT) ThreadAttributes {
-  bool detached;
+  // We want the "detach_state" attribute to be an atomic value as it could be
+  // updated by one thread while the self thread is reading it. It is a tristate
+  // variable with the following state transitions:
+  // 1. The a thread is created in a detached state, then user code should never
+  //    call a detach or join function. Calling either of them can lead to
+  //    undefined behavior.
+  //    The value of |detach_state| is expected to be DetachState::DETACHED for
+  //    its lifetime.
+  // 2. If a thread is created in a joinable state, |detach_state| will start
+  //    with the value DetachState::JOINABLE. Another thread can detach this
+  //    thread before it exits. The state transitions will as follows:
+  //      (a) If the detach method sees the state as JOINABLE, then it will
+  //          compare exchange to a state of DETACHED. The thread will clean
+  //          itself up after it finishes.
+  //      (b) If the detach method does not see JOINABLE in (a), then it will
+  //          conclude that the thread is EXITING and will wait until the thread
+  //          exits. It will clean up the thread resources once the thread
+  //          exits.
+  cpp::Atomic<uint32_t> detach_state;
   void *stack;                   // Pointer to the thread stack
   unsigned long long stack_size; // Size of the stack
   unsigned char owned_stack; // Indicates if the thread owns this stack memory

diff  --git a/libc/src/pthread/CMakeLists.txt b/libc/src/pthread/CMakeLists.txt
index d44eef057d320..d786a35155492 100644
--- a/libc/src/pthread/CMakeLists.txt
+++ b/libc/src/pthread/CMakeLists.txt
@@ -268,3 +268,14 @@ add_entrypoint_object(
     libc.include.pthread
     libc.src.__support.threads.thread
 )
+
+add_entrypoint_object(
+  pthread_detach
+  SRCS
+    pthread_detach.cpp
+  HDRS
+    pthread_detach.h
+  DEPENDS
+    libc.include.pthread
+    libc.src.__support.threads.thread
+)

diff  --git a/libc/src/pthread/pthread_detach.cpp b/libc/src/pthread/pthread_detach.cpp
new file mode 100644
index 0000000000000..c71b12fbdcda5
--- /dev/null
+++ b/libc/src/pthread/pthread_detach.cpp
@@ -0,0 +1,27 @@
+//===-- Implementation of the pthread_detach 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 "pthread_detach.h"
+
+#include "src/__support/common.h"
+#include "src/__support/threads/thread.h"
+
+#include <pthread.h> // For pthread_* type definitions.
+
+namespace __llvm_libc {
+
+static_assert(sizeof(pthread_t) == sizeof(__llvm_libc::Thread<void *>),
+              "Mismatch between pthread_t and internal Thread<void *>.");
+
+LLVM_LIBC_FUNCTION(int, pthread_detach, (pthread_t th)) {
+  auto *thread = reinterpret_cast<Thread<void *> *>(&th);
+  thread->detach();
+  return 0;
+}
+
+} // namespace __llvm_libc

diff  --git a/libc/src/pthread/pthread_detach.h b/libc/src/pthread/pthread_detach.h
new file mode 100644
index 0000000000000..2122ed6881a09
--- /dev/null
+++ b/libc/src/pthread/pthread_detach.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for pthread_detach 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_PTHREAD_DETACH_H
+#define LLVM_LIBC_SRC_THREADS_PTHREAD_DETACH_H
+
+#include <pthread.h>
+
+namespace __llvm_libc {
+
+int pthread_detach(pthread_t thread);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_THREADS_PTHREAD_DETACH_H

diff  --git a/libc/src/threads/CMakeLists.txt b/libc/src/threads/CMakeLists.txt
index bd63a53061f36..bdbb32d81e1c1 100644
--- a/libc/src/threads/CMakeLists.txt
+++ b/libc/src/threads/CMakeLists.txt
@@ -35,6 +35,17 @@ add_entrypoint_object(
     libc.src.__support.threads.thread
 )
 
+add_entrypoint_object(
+  thrd_detach
+  SRCS
+    thrd_detach.cpp
+  HDRS
+    thrd_detach.h
+  DEPENDS
+    libc.include.threads
+    libc.src.__support.threads.thread
+)
+
 add_entrypoint_object(
   mtx_init
   SRCS

diff  --git a/libc/src/threads/thrd_detach.cpp b/libc/src/threads/thrd_detach.cpp
new file mode 100644
index 0000000000000..e3c4301709cdf
--- /dev/null
+++ b/libc/src/threads/thrd_detach.cpp
@@ -0,0 +1,26 @@
+//===-- Linux implementation of the thrd_detach 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/thrd_detach.h"
+#include "src/__support/common.h"
+#include "src/__support/threads/thread.h"
+
+#include <threads.h> // For thrd_* type definitions.
+
+namespace __llvm_libc {
+
+static_assert(sizeof(thrd_t) == sizeof(__llvm_libc::Thread<int>),
+              "Mismatch between thrd_t and internal Thread<int>.");
+
+LLVM_LIBC_FUNCTION(int, thrd_detach, (thrd_t th)) {
+  auto *thread = reinterpret_cast<Thread<int> *>(&th);
+  thread->detach();
+  return 0;
+}
+
+} // namespace __llvm_libc

diff  --git a/libc/src/threads/thrd_detach.h b/libc/src/threads/thrd_detach.h
new file mode 100644
index 0000000000000..176770d59986e
--- /dev/null
+++ b/libc/src/threads/thrd_detach.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for thrd_detach 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_THRD_DETACH_H
+#define LLVM_LIBC_SRC_THREADS_THRD_DETACH_H
+
+#include "include/threads.h"
+
+namespace __llvm_libc {
+
+int thrd_detach(thrd_t thread);
+
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_THREADS_THRD_DETACH_H

diff  --git a/libc/test/src/__support/CMakeLists.txt b/libc/test/src/__support/CMakeLists.txt
index 64c014c6c4d58..6f0750cd5d404 100644
--- a/libc/test/src/__support/CMakeLists.txt
+++ b/libc/test/src/__support/CMakeLists.txt
@@ -77,3 +77,4 @@ add_custom_command(TARGET libc_str_to_float_comparison_test
 add_subdirectory(CPP)
 add_subdirectory(File)
 add_subdirectory(OSUtil)
+add_subdirectory(threads)

diff  --git a/libc/test/src/__support/threads/CMakeLists.txt b/libc/test/src/__support/threads/CMakeLists.txt
new file mode 100644
index 0000000000000..c713e8677378b
--- /dev/null
+++ b/libc/test/src/__support/threads/CMakeLists.txt
@@ -0,0 +1,18 @@
+if(NOT (TARGET libc.src.__support.threads.thread AND
+        TARGET libc.src.__support.threads.mutex))
+  return()
+endif()
+
+add_libc_unittest(
+  thread_detach_test
+  SUITE
+  SRCS
+    thread_detach_test.cpp
+  DEPENDS
+    libc.src.__support.threads.mutex
+    libc.src.__support.threads.thread
+    libc.src.__support.threads.thread_attrib
+  COMPILE_OPTIONS
+    -O3
+    -fno-omit-frame-pointer
+)

diff  --git a/libc/test/src/__support/threads/thread_detach_test.cpp b/libc/test/src/__support/threads/thread_detach_test.cpp
new file mode 100644
index 0000000000000..060ee08533796
--- /dev/null
+++ b/libc/test/src/__support/threads/thread_detach_test.cpp
@@ -0,0 +1,52 @@
+//===-- Unittests for thrd_t ----------------------------------------------===//
+//
+// 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/__support/threads/mutex.h"
+#include "src/__support/threads/thread.h"
+#include "src/__support/threads/thread_attrib.h"
+#include "utils/UnitTest/Test.h"
+
+__llvm_libc::Mutex mutex(false, false, false);
+
+int func(void *) {
+  mutex.lock();
+  mutex.unlock();
+  return 0;
+}
+
+TEST(LlvmLibcTestThreadTest, DetachSimpleTest) {
+  mutex.lock();
+  __llvm_libc::Thread<int> th;
+  th.run(func, nullptr, nullptr, 0);
+
+  // Since |mutex| is held by the current thread, we guarantee that
+  // th is running and hence it is safe to detach. Since the thread is
+  // still running, it should be simple detach.
+  ASSERT_EQ(th.detach(), int(__llvm_libc::DetachType::SIMPLE));
+
+  // We will release |mutex| now to let the thread finish an cleanup itself.
+  mutex.unlock();
+}
+
+TEST(LlvmLibcTestThreadTest, DetachCleanupTest) {
+  mutex.lock();
+  __llvm_libc::Thread<int> th;
+  ASSERT_EQ(0, th.run(func, nullptr, nullptr, 0));
+
+  // Since |mutex| is held by the current thread, we will release it
+  // to let |th| run.
+  mutex.unlock();
+
+  // We will wait for |th| to finish. Since it is a joinable thread,
+  // we can wait on it safely.
+  th.wait();
+
+  // Since |th| is now finished, detaching should cleanup the thread
+  // resources.
+  ASSERT_EQ(th.detach(), int(__llvm_libc::DetachType::CLEANUP));
+}


        


More information about the libc-commits mailing list