Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 24 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
42f47af
[libc] Added lock_guard
vmishelcs Apr 18, 2024
0d98c20
[libc] Added test file
vmishelcs Apr 18, 2024
f75fba7
[libc] Changed lock_guard typename for clarity
vmishelcs Apr 18, 2024
5c4e4dd
[libc] Added basic lock_guard test
vmishelcs Apr 19, 2024
30d9cb6
[libc] Added template argument in mutex_test.cpp
vmishelcs Apr 19, 2024
5cba525
[libc] Improved mutex_test.cpp
vmishelcs Apr 19, 2024
f17ff1c
[libc] Replace references to `MutexLock` with `cpp::lock_guard`
vmishelcs Apr 19, 2024
5572fd7
[libc] Ran `clang-format`
vmishelcs Apr 19, 2024
c6528ed
[libc] `clang-format` on mutex_test.cpp
vmishelcs Apr 19, 2024
9bc3554
[libc] More `clang-format`
vmishelcs Apr 19, 2024
5f7c3bc
[libc] Implemented `lock_guard` constructor for locked mutexes
vmishelcs Apr 19, 2024
4ef858e
[libc] `clang-format` on mutex.h
vmishelcs Apr 19, 2024
8db29ee
[libc] Formatting fix in mutex_test.cpp
vmishelcs Apr 19, 2024
e1e1a14
[libc] Fixed comment in mutex_test.cpp
vmishelcs Apr 19, 2024
64ef921
[libc] Moved lock_guard data member to top of class definition
vmishelcs Apr 20, 2024
8008975
[libc] Adjusted comments related to lock_guard implementation
vmishelcs Apr 20, 2024
9c2a9eb
[libc] Simplified mutex_test.cpp
vmishelcs Apr 20, 2024
88b27de
[libc] Formatting
vmishelcs Apr 20, 2024
5b7270c
[libc] Removed type arguments from `cpp::lock_guard`
vmishelcs Apr 20, 2024
afb6a45
[libc] Removed some unnecessary code
vmishelcs Apr 23, 2024
9a7d1f4
[libc] clang-format
vmishelcs Apr 23, 2024
8daef4c
[libc] Small changes to mutex_test.cpp
vmishelcs May 3, 2024
d84a202
Merge branch 'main' into issue-89002-add-lock-guard
gchatelet May 7, 2024
7ba0feb
Fix formatting
gchatelet May 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
47 changes: 47 additions & 0 deletions libc/src/__support/CPP/mutex.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//===--- 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(); }

private:
vmishelcs marked this conversation as resolved.
Show resolved Hide resolved
// 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 "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_word_type
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/OSUtil/syscall.h" // For syscall functions.
#include "src/__support/threads/linux/futex_word.h"
#include "src/__support/threads/mutex.h"
Expand Down Expand Up @@ -58,7 +59,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 @@ -117,7 +118,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;

// Flag to show if this mutex has been double locked.
bool double_locked;

// Flag to show if this mutex has been double unlocked.
bool double_unlocked;

Mutex() : locked(false), double_locked(false), double_unlocked(false) {}
vmishelcs marked this conversation as resolved.
Show resolved Hide resolved

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);
}
Loading