[libc] Replace MutexLock with cpp::lock_guard (#89340)

This PR address issue #89002.

#### Changes in this PR

* Added a simple implementation of `cpp::lock_guard` (an equivalent of
`std::lock_guard`) in libc/src/__support/CPP inspired by the libstdc++
implementation
* Added tests for `cpp::lock_guard` in
/libc/test/src/__support/CPP/mutex_test.cpp
* Replaced all references to `MutexLock` with `cpp::lock_guard`

---------

Co-authored-by: Guillaume Chatelet <[email protected]>

Author: vmishelcs

libc/src/__support/CPP/CMakeLists.txt

@@ -51,6 +51,12 @@ add_header_library(
     libc.src.__support.macros.properties.types
 )
 
+add_header_library(
+  mutex
+  HDRS
+    mutex.h
+)
+
 add_header_library(
   span
   HDRS

libc/src/__support/CPP/mutex.h

@@ -0,0 +1,46 @@
+//===--- A self contained equivalent of std::mutex --------------*- 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___SUPPORT_CPP_MUTEX_H
+#define LLVM_LIBC_SRC___SUPPORT_CPP_MUTEX_H
+
+namespace LIBC_NAMESPACE {
+namespace cpp {
+
+// Assume the calling thread has already obtained mutex ownership.
+struct adopt_lock_t {
+  explicit adopt_lock_t() = default;
+};
+
+// Tag used to make a scoped lock take ownership of a locked mutex.
+constexpr adopt_lock_t adopt_lock{};
+
+// An RAII class for easy locking and unlocking of mutexes.
+template <typename MutexType> class lock_guard {
+  MutexType &mutex;
+
+public:
+  // Calls `m.lock()` upon resource acquisition.
+  explicit lock_guard(MutexType &m) : mutex(m) { mutex.lock(); }
+
+  // Acquires ownership of the mutex object `m` without attempting to lock
+  // it. The behavior is undefined if the current thread does not hold the
+  // lock on `m`. Does not call `m.lock()` upon resource acquisition.
+  lock_guard(MutexType &m, adopt_lock_t t) : mutex(m) {}
+
+  ~lock_guard() { mutex.unlock(); }
+
+  // non-copyable
+  lock_guard &operator=(const lock_guard &) = delete;
+  lock_guard(const lock_guard &) = delete;
+};
+
+} // namespace cpp
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC___SUPPORT_CPP_MUTEX_H

libc/src/__support/File/CMakeLists.txt

@@ -25,6 +25,7 @@ add_object_library(
   HDRS
     dir.h
   DEPENDS
+    libc.src.__support.CPP.mutex
     libc.src.__support.CPP.new
     libc.src.__support.CPP.span
     libc.src.__support.threads.mutex

libc/src/__support/File/dir.cpp

@@ -8,6 +8,7 @@
 
 #include "dir.h"
 
+#include "src/__support/CPP/mutex.h" // lock_guard
 #include "src/__support/CPP/new.h"
 #include "src/__support/error_or.h"
 #include "src/errno/libc_errno.h" // For error macros
@@ -27,7 +28,7 @@ ErrorOr<Dir *> Dir::open(const char *path) {
 }
 
 ErrorOr<struct ::dirent *> Dir::read() {
-  MutexLock lock(&mutex);
+  cpp::lock_guard lock(mutex);
   if (readptr >= fillsize) {
     auto readsize = platform_fetch_dirents(fd, buffer);
     if (!readsize)
@@ -51,7 +52,7 @@ ErrorOr<struct ::dirent *> Dir::read() {
 
 int Dir::close() {
   {
-    MutexLock lock(&mutex);
+    cpp::lock_guard lock(mutex);
     int retval = platform_closedir(fd);
     if (retval != 0)
       return retval;

libc/src/__support/threads/CMakeLists.txt

@@ -31,6 +31,7 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.mutex)
       fork_callbacks.h
     DEPENDS
       .mutex
+      libc.src.__support.CPP.mutex
   )
 endif()
 
@@ -57,6 +58,7 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.thread)
       libc.src.__support.common
       libc.src.__support.fixedvector
       libc.src.__support.CPP.array
+      libc.src.__support.CPP.mutex
       libc.src.__support.CPP.optional
   )
 endif()

libc/src/__support/threads/fork_callbacks.cpp

@@ -8,6 +8,7 @@
 
 #include "fork_callbacks.h"
 
+#include "src/__support/CPP/mutex.h" // lock_guard
 #include "src/__support/threads/mutex.h"
 
 #include <stddef.h> // For size_t
@@ -35,7 +36,7 @@ class AtForkCallbackManager {
   constexpr AtForkCallbackManager() : mtx(false, false, false), next_index(0) {}
 
   bool register_triple(const ForkCallbackTriple &triple) {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     if (next_index >= CALLBACK_SIZE)
       return false;
     list[next_index] = triple;
@@ -44,7 +45,7 @@ class AtForkCallbackManager {
   }
 
   void invoke_prepare() {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     for (size_t i = 0; i < next_index; ++i) {
       auto prepare = list[i].prepare;
       if (prepare)
@@ -53,7 +54,7 @@ class AtForkCallbackManager {
   }
 
   void invoke_parent() {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     for (size_t i = 0; i < next_index; ++i) {
       auto parent = list[i].parent;
       if (parent)
@@ -62,7 +63,7 @@ class AtForkCallbackManager {
   }
 
   void invoke_child() {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     for (size_t i = 0; i < next_index; ++i) {
       auto child = list[i].child;
       if (child)

libc/src/__support/threads/thread.cpp

@@ -10,6 +10,7 @@
 #include "src/__support/threads/mutex.h"
 
 #include "src/__support/CPP/array.h"
+#include "src/__support/CPP/mutex.h" // lock_guard
 #include "src/__support/CPP/optional.h"
 #include "src/__support/fixedvector.h"
 #include "src/__support/macros/attributes.h"
@@ -56,7 +57,7 @@ class TSSKeyMgr {
   constexpr TSSKeyMgr() : mtx(false, false, false) {}
 
   cpp::optional<unsigned int> new_key(TSSDtor *dtor) {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     for (unsigned int i = 0; i < TSS_KEY_COUNT; ++i) {
       TSSKeyUnit &u = units[i];
       if (!u.active) {
@@ -70,20 +71,20 @@ class TSSKeyMgr {
   TSSDtor *get_dtor(unsigned int key) {
     if (key >= TSS_KEY_COUNT)
       return nullptr;
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     return units[key].dtor;
   }
 
   bool remove_key(unsigned int key) {
     if (key >= TSS_KEY_COUNT)
       return false;
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     units[key].reset();
     return true;
   }
 
   bool is_valid_key(unsigned int key) {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     return units[key].active;
   }
 };
@@ -113,7 +114,7 @@ class ThreadAtExitCallbackMgr {
   constexpr ThreadAtExitCallbackMgr() : mtx(false, false, false) {}
 
   int add_callback(AtExitCallback *callback, void *obj) {
-    MutexLock lock(&mtx);
+    cpp::lock_guard lock(mtx);
     return callback_list.push_back({callback, obj});
   }
 

libc/src/stdlib/CMakeLists.txt

@@ -414,6 +414,7 @@ add_entrypoint_object(
   CXX_STANDARD
     20 # For constinit of the atexit callback list.
   DEPENDS
+    libc.src.__support.CPP.mutex
     libc.src.__support.CPP.new
     libc.src.__support.OSUtil.osutil
     libc.src.__support.blockstore

libc/src/stdlib/atexit.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/stdlib/atexit.h"
+#include "src/__support/CPP/mutex.h" // lock_guard
 #include "src/__support/blockstore.h"
 #include "src/__support/common.h"
 #include "src/__support/fixedvector.h"
@@ -68,7 +69,7 @@ void call_exit_callbacks() {
 }
 
 int add_atexit_unit(const AtExitUnit &unit) {
-  MutexLock lock(&handler_list_mtx);
+  cpp::lock_guard lock(handler_list_mtx);
   if (exit_callbacks.push_back(unit))
     return 0;
   return -1;

libc/src/threads/linux/CMakeLists.txt

@@ -7,6 +7,7 @@ add_header_library(
     libc.include.sys_syscall
     libc.include.threads
     libc.src.__support.CPP.atomic
+    libc.src.__support.CPP.mutex
     libc.src.__support.OSUtil.osutil
     libc.src.__support.threads.mutex
     libc.src.__support.threads.linux.futex_utils

libc/src/threads/linux/CndVar.h

@@ -10,6 +10,7 @@
 #define LLVM_LIBC_SRC_THREADS_LINUX_CNDVAR_H
 
 #include "src/__support/CPP/atomic.h"
+#include "src/__support/CPP/mutex.h" // lock_guard
 #include "src/__support/CPP/optional.h"
 #include "src/__support/OSUtil/syscall.h" // For syscall functions.
 #include "src/__support/threads/linux/futex_utils.h"
@@ -59,7 +60,7 @@ struct CndVar {
 
     CndWaiter waiter;
     {
-      MutexLock ml(&qmtx);
+      cpp::lock_guard ml(qmtx);
       CndWaiter *old_back = nullptr;
       if (waitq_front == nullptr) {
         waitq_front = waitq_back = &waiter;
@@ -118,7 +119,7 @@ struct CndVar {
   }
 
   int broadcast() {
-    MutexLock ml(&qmtx);
+    cpp::lock_guard ml(qmtx);
     uint32_t dummy_futex_word;
     CndWaiter *waiter = waitq_front;
     waitq_front = waitq_back = nullptr;

libc/test/src/__support/CPP/CMakeLists.txt

@@ -64,6 +64,16 @@ add_libc_test(
     libc.src.__support.macros.properties.types
 )
 
+add_libc_test(
+  mutex_test
+  SUITE
+    libc-cpp-utils-tests
+  SRCS
+    mutex_test.cpp
+  DEPENDS
+    libc.src.__support.CPP.mutex
+)
+
 add_libc_test(
   int_seq_test
   SUITE

libc/test/src/__support/CPP/mutex_test.cpp

@@ -0,0 +1,79 @@
+//===-- Unittests for mutex -----------------------------------------------===//
+//
+// 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/CPP/mutex.h"
+#include "test/UnitTest/Test.h"
+
+using LIBC_NAMESPACE::cpp::adopt_lock;
+using LIBC_NAMESPACE::cpp::lock_guard;
+
+// Simple struct for testing cpp::lock_guard. It defines methods 'lock' and
+// 'unlock' which are required for the cpp::lock_guard class template.
+struct Mutex {
+  // Flag to show whether this mutex is locked.
+  bool locked = false;
+
+  // Flag to show if this mutex has been double locked.
+  bool double_locked = false;
+
+  // Flag to show if this mutex has been double unlocked.
+  bool double_unlocked = false;
+
+  Mutex() {}
+
+  void lock() {
+    if (locked)
+      double_locked = true;
+
+    locked = true;
+  }
+
+  void unlock() {
+    if (!locked)
+      double_unlocked = true;
+
+    locked = false;
+  }
+};
+
+TEST(LlvmLibcMutexTest, Basic) {
+  Mutex m;
+  ASSERT_FALSE(m.locked);
+  ASSERT_FALSE(m.double_locked);
+  ASSERT_FALSE(m.double_unlocked);
+
+  {
+    lock_guard lg(m);
+    ASSERT_TRUE(m.locked);
+    ASSERT_FALSE(m.double_locked);
+  }
+
+  ASSERT_FALSE(m.locked);
+  ASSERT_FALSE(m.double_unlocked);
+}
+
+TEST(LlvmLibcMutexTest, AcquireLocked) {
+  Mutex m;
+  ASSERT_FALSE(m.locked);
+  ASSERT_FALSE(m.double_locked);
+  ASSERT_FALSE(m.double_unlocked);
+
+  // Lock the mutex before placing a lock guard on it.
+  m.lock();
+  ASSERT_TRUE(m.locked);
+  ASSERT_FALSE(m.double_locked);
+
+  {
+    lock_guard lg(m, adopt_lock);
+    ASSERT_TRUE(m.locked);
+    ASSERT_FALSE(m.double_locked);
+  }
+
+  ASSERT_FALSE(m.locked);
+  ASSERT_FALSE(m.double_unlocked);
+}