Skip to content

Commit

Permalink
[libc] Replace MutexLock with cpp::lock_guard (#89340)
Browse files Browse the repository at this point in the history
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@google.com>
  • Loading branch information
vmishelcs and gchatelet committed May 9, 2024
1 parent aacea0d commit c4a3d18
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 14 deletions.
6 changes: 6 additions & 0 deletions libc/src/__support/CPP/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions libc/src/__support/CPP/mutex.h
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions libc/src/__support/File/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions libc/src/__support/File/dir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions libc/src/__support/threads/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()
Expand Down
9 changes: 5 additions & 4 deletions libc/src/__support/threads/fork_callbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
11 changes: 6 additions & 5 deletions libc/src/__support/threads/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
};
Expand Down Expand Up @@ -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});
}

Expand Down
1 change: 1 addition & 0 deletions libc/src/stdlib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion libc/src/stdlib/atexit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions libc/src/threads/linux/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions libc/src/threads/linux/CndVar.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 10 additions & 0 deletions libc/test/src/__support/CPP/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
79 changes: 79 additions & 0 deletions libc/test/src/__support/CPP/mutex_test.cpp
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit c4a3d18

Please sign in to comment.