[libc-commits] [libc] c4a3d18 - [libc] Replace `MutexLock` with `cpp::lock_guard` (#89340)
via libc-commits
libc-commits at lists.llvm.org
Thu May 9 00:06:22 PDT 2024
Author: Vlad Mishel
Date: 2024-05-09T09:06:18+02:00
New Revision: c4a3d184db5fdffe798208b8281dfe944616f9ed
URL: https://github.com/llvm/llvm-project/commit/c4a3d184db5fdffe798208b8281dfe944616f9ed
DIFF: https://github.com/llvm/llvm-project/commit/c4a3d184db5fdffe798208b8281dfe944616f9ed.diff
LOG: [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 <gchatelet at google.com>
Added:
libc/src/__support/CPP/mutex.h
libc/test/src/__support/CPP/mutex_test.cpp
Modified:
libc/src/__support/CPP/CMakeLists.txt
libc/src/__support/File/CMakeLists.txt
libc/src/__support/File/dir.cpp
libc/src/__support/threads/CMakeLists.txt
libc/src/__support/threads/fork_callbacks.cpp
libc/src/__support/threads/thread.cpp
libc/src/stdlib/CMakeLists.txt
libc/src/stdlib/atexit.cpp
libc/src/threads/linux/CMakeLists.txt
libc/src/threads/linux/CndVar.h
libc/test/src/__support/CPP/CMakeLists.txt
Removed:
################################################################################
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index 84d01fe045160..08661aba5b6b1 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/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
diff --git a/libc/src/__support/CPP/mutex.h b/libc/src/__support/CPP/mutex.h
new file mode 100644
index 0000000000000..c25c1155b7666
--- /dev/null
+++ b/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
diff --git a/libc/src/__support/File/CMakeLists.txt b/libc/src/__support/File/CMakeLists.txt
index b7c0612096aa9..0416ac2cc902e 100644
--- a/libc/src/__support/File/CMakeLists.txt
+++ b/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
diff --git a/libc/src/__support/File/dir.cpp b/libc/src/__support/File/dir.cpp
index 9ff639a777e23..e0f7695b39323 100644
--- a/libc/src/__support/File/dir.cpp
+++ b/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;
diff --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt
index 731adf6f9c8e4..34412be4dfed6 100644
--- a/libc/src/__support/threads/CMakeLists.txt
+++ b/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()
diff --git a/libc/src/__support/threads/fork_callbacks.cpp b/libc/src/__support/threads/fork_callbacks.cpp
index 54fda676f281e..6efaf62f135ae 100644
--- a/libc/src/__support/threads/fork_callbacks.cpp
+++ b/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)
diff --git a/libc/src/__support/threads/thread.cpp b/libc/src/__support/threads/thread.cpp
index c1785343671c5..7b02f8246e24b 100644
--- a/libc/src/__support/threads/thread.cpp
+++ b/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});
}
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index e526ba040befb..9b76a6a0f8575 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/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
diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index fa072b2fdf8d0..4f0497444773d 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/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;
diff --git a/libc/src/threads/linux/CMakeLists.txt b/libc/src/threads/linux/CMakeLists.txt
index d372bd9e18c4a..68b7106c2052f 100644
--- a/libc/src/threads/linux/CMakeLists.txt
+++ b/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
diff --git a/libc/src/threads/linux/CndVar.h b/libc/src/threads/linux/CndVar.h
index 525a8f0f2b534..c08ffa393856f 100644
--- a/libc/src/threads/linux/CndVar.h
+++ b/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;
diff --git a/libc/test/src/__support/CPP/CMakeLists.txt b/libc/test/src/__support/CPP/CMakeLists.txt
index 708548f812c66..cec13afc8dd12 100644
--- a/libc/test/src/__support/CPP/CMakeLists.txt
+++ b/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
diff --git a/libc/test/src/__support/CPP/mutex_test.cpp b/libc/test/src/__support/CPP/mutex_test.cpp
new file mode 100644
index 0000000000000..a68c84cfc78af
--- /dev/null
+++ b/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);
+}
More information about the libc-commits
mailing list