Skip to content

Commit

Permalink
Use std::mutex instead of base lock_* functions
Browse files Browse the repository at this point in the history
Use `std::lock_guard` instead of `CLockScope`.

Rename `lock` variables to `mutex`.
  • Loading branch information
Robyt3 committed Nov 9, 2023
1 parent fa8640b commit 7fbe77b
Show file tree
Hide file tree
Showing 19 changed files with 121 additions and 330 deletions.
10 changes: 5 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ if(NOT MSVC AND NOT HAIKU)
add_cxx_compiler_flag_if_supported(OUR_FLAGS_LINK -Wl,--no-insert-timestamp)
endif()

if(TARGET_OS STREQUAL "mac")
add_cxx_compiler_flag_if_supported(OUR_FLAGS -stdlib=libc++)
endif()
# Force usage of libc++ to ensure thread-safety annotations are supported for std::mutex
add_cxx_compiler_flag_if_supported(OUR_FLAGS -stdlib=libc++)
add_linker_flag_if_supported(OUR_FLAGS_LINK -stdlib=libc++)

if(EXCEPTION_HANDLING)
add_cxx_compiler_flag_if_supported(OUR_FLAGS -DCONF_EXCEPTION_HANDLING)
Expand All @@ -323,7 +323,7 @@ if(NOT MSVC AND NOT HAIKU)
add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wshadow-all) # clang
add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wshadow=global) # gcc
add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety)
add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety-negative)
# add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety-negative) # Causes false positives at the moment
add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wsuggest-override)
add_linker_flag_if_supported(OUR_FLAGS_LINK -Wno-alloc-size-larger-than) # save.cpp with LTO
# add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wdouble-promotion) # Many occurrences
Expand Down Expand Up @@ -1844,7 +1844,6 @@ set_src(BASE GLOB_RECURSE src/base
hash_ctxt.h
hash_libtomcrypt.cpp
hash_openssl.cpp
lock_scope.h
log.cpp
log.h
logger.h
Expand Down Expand Up @@ -3285,6 +3284,7 @@ foreach(target ${TARGETS})
if(ENABLE_IPO)
set_property(TARGET ${target} PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
endif()
target_compile_definitions(${target} PRIVATE _LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS) # Enable thread-safety annotations for std::mutex
endforeach()

foreach(target ${TARGETS_LINK})
Expand Down
24 changes: 0 additions & 24 deletions src/base/lock_scope.h

This file was deleted.

6 changes: 4 additions & 2 deletions src/base/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#define BASE_LOGGER_H

#include "log.h"
#include "system.h"

#include <atomic>
#include <memory>
#include <mutex>
Expand Down Expand Up @@ -233,8 +235,8 @@ class CFutureLogger : public ILogger
* Replace the `CFutureLogger` instance with the given logger. It'll
* receive all log messages sent to the `CFutureLogger` so far.
*/
void Set(std::shared_ptr<ILogger> pLogger);
void Log(const CLogMessage *pMessage) override;
void Set(std::shared_ptr<ILogger> pLogger) EXCLUDES(m_PendingLock);
void Log(const CLogMessage *pMessage) override EXCLUDES(m_PendingLock);
void GlobalFinish() override;
void OnFilterChange() override;
};
Expand Down
152 changes: 23 additions & 129 deletions src/base/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,22 @@
#include <atomic>
#include <cctype>
#include <charconv>
#include <chrono>
#include <cinttypes>
#include <cmath>
#include <cstdarg>
#include <cstdio>
#include <cstring>
#include <iterator> // std::size
#include <mutex>
#include <string_view>

#include "system.h"

#include "lock_scope.h"
#include "logger.h"

#include <sys/types.h>

#include <chrono>

#include <cinttypes>

#if defined(CONF_WEBSOCKETS)
#include <engine/shared/websockets.h>
#endif
Expand Down Expand Up @@ -477,7 +475,7 @@ int io_sync(IOHANDLE io)
// TODO: Use Thread Safety Analysis when this file is converted to C++
struct ASYNCIO
{
LOCK lock;
std::mutex mutex;
IOHANDLE io;
SEMAPHORE sphore;
void *thread;
Expand Down Expand Up @@ -524,27 +522,26 @@ static void buffer_ptrs(ASYNCIO *aio, struct BUFFERS *buffers)
}
}

static void aio_handle_free_and_unlock(ASYNCIO *aio) RELEASE(aio->lock)
static void aio_handle_free_and_unlock(ASYNCIO *aio) RELEASE(aio->mutex)
{
int do_free;
aio->refcount--;

do_free = aio->refcount == 0;
lock_unlock(aio->lock);
aio->mutex.unlock();
if(do_free)
{
free(aio->buffer);
sphore_destroy(&aio->sphore);
lock_destroy(aio->lock);
free(aio);
delete aio;
}
}

static void aio_thread(void *user)
{
ASYNCIO *aio = (ASYNCIO *)user;

lock_wait(aio->lock);
aio->mutex.lock();
while(true)
{
struct BUFFERS buffers;
Expand All @@ -563,9 +560,9 @@ static void aio_thread(void *user)
aio_handle_free_and_unlock(aio);
break;
}
lock_unlock(aio->lock);
aio->mutex.unlock();
sphore_wait(&aio->sphore);
lock_wait(aio->lock);
aio->mutex.lock();
continue;
}

Expand All @@ -589,35 +586,33 @@ static void aio_thread(void *user)
}
}
aio->read_pos = (aio->read_pos + buffers.len1 + buffers.len2) % aio->buffer_size;
lock_unlock(aio->lock);
aio->mutex.unlock();

io_write(aio->io, local_buffer, local_buffer_len);
io_flush(aio->io);
result_io_error = io_error(aio->io);

lock_wait(aio->lock);
aio->mutex.lock();
aio->error = result_io_error;
}
}

ASYNCIO *aio_new(IOHANDLE io)
{
ASYNCIO *aio = (ASYNCIO *)malloc(sizeof(*aio));
ASYNCIO *aio = new ASYNCIO;
if(!aio)
{
return 0;
}
aio->io = io;
aio->lock = lock_create();
sphore_init(&aio->sphore);
aio->thread = 0;

aio->buffer = (unsigned char *)malloc(ASYNC_BUFSIZE);
if(!aio->buffer)
{
sphore_destroy(&aio->sphore);
lock_destroy(aio->lock);
free(aio);
delete aio;
return 0;
}
aio->buffer_size = ASYNC_BUFSIZE;
Expand All @@ -632,8 +627,7 @@ ASYNCIO *aio_new(IOHANDLE io)
{
free(aio->buffer);
sphore_destroy(&aio->sphore);
lock_destroy(aio->lock);
free(aio);
delete aio;
return 0;
}
return aio;
Expand All @@ -660,14 +654,14 @@ static unsigned int next_buffer_size(unsigned int cur_size, unsigned int need_si
return cur_size;
}

void aio_lock(ASYNCIO *aio) ACQUIRE(aio->lock)
void aio_lock(ASYNCIO *aio) ACQUIRE(aio->mutex)
{
lock_wait(aio->lock);
aio->mutex.lock();
}

void aio_unlock(ASYNCIO *aio) RELEASE(aio->lock)
void aio_unlock(ASYNCIO *aio) RELEASE(aio->mutex)
{
lock_unlock(aio->lock);
aio->mutex.unlock();
sphore_signal(&aio->sphore);
}

Expand Down Expand Up @@ -746,13 +740,13 @@ void aio_write_newline(ASYNCIO *aio)

int aio_error(ASYNCIO *aio)
{
CLockScope ls(aio->lock);
const std::lock_guard<std::mutex> lock_guard(aio->mutex);
return aio->error;
}

void aio_free(ASYNCIO *aio)
{
lock_wait(aio->lock);
aio->mutex.lock();
if(aio->thread)
{
thread_detach(aio->thread);
Expand All @@ -764,7 +758,7 @@ void aio_free(ASYNCIO *aio)
void aio_close(ASYNCIO *aio)
{
{
CLockScope ls(aio->lock);
const std::lock_guard<std::mutex> lock_guard(aio->mutex);
aio->finish = ASYNCIO_CLOSE;
}
sphore_signal(&aio->sphore);
Expand All @@ -774,7 +768,7 @@ void aio_wait(ASYNCIO *aio)
{
void *thread;
{
CLockScope ls(aio->lock);
const std::lock_guard<std::mutex> lock_guard(aio->mutex);
thread = aio->thread;
aio->thread = 0;
if(aio->finish == ASYNCIO_RUNNING)
Expand Down Expand Up @@ -877,106 +871,6 @@ void thread_detach(void *thread)
#endif
}

bool thread_init_and_detach(void (*threadfunc)(void *), void *u, const char *name)
{
void *thread = thread_init(threadfunc, u, name);
if(thread)
thread_detach(thread);
return thread != nullptr;
}

#if defined(CONF_FAMILY_UNIX)
typedef pthread_mutex_t LOCKINTERNAL;
#elif defined(CONF_FAMILY_WINDOWS)
typedef CRITICAL_SECTION LOCKINTERNAL;
#else
#error not implemented on this platform
#endif

LOCK lock_create()
{
LOCKINTERNAL *lock = (LOCKINTERNAL *)malloc(sizeof(*lock));
#if defined(CONF_FAMILY_UNIX)
int result;
#endif

if(!lock)
return 0;

#if defined(CONF_FAMILY_UNIX)
result = pthread_mutex_init(lock, 0x0);
if(result != 0)
{
dbg_msg("lock", "init failed: %d", result);
free(lock);
return 0;
}
#elif defined(CONF_FAMILY_WINDOWS)
InitializeCriticalSection((LPCRITICAL_SECTION)lock);
#else
#error not implemented on this platform
#endif
return (LOCK)lock;
}

void lock_destroy(LOCK lock)
{
#if defined(CONF_FAMILY_UNIX)
int result = pthread_mutex_destroy((LOCKINTERNAL *)lock);
if(result != 0)
dbg_msg("lock", "destroy failed: %d", result);
#elif defined(CONF_FAMILY_WINDOWS)
DeleteCriticalSection((LPCRITICAL_SECTION)lock);
#else
#error not implemented on this platform
#endif
free(lock);
}

int lock_trylock(LOCK lock)
{
#if defined(CONF_FAMILY_UNIX)
return pthread_mutex_trylock((LOCKINTERNAL *)lock);
#elif defined(CONF_FAMILY_WINDOWS)
return !TryEnterCriticalSection((LPCRITICAL_SECTION)lock);
#else
#error not implemented on this platform
#endif
}

#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wthread-safety-analysis"
#endif
void lock_wait(LOCK lock)
{
#if defined(CONF_FAMILY_UNIX)
int result = pthread_mutex_lock((LOCKINTERNAL *)lock);
if(result != 0)
dbg_msg("lock", "lock failed: %d", result);
#elif defined(CONF_FAMILY_WINDOWS)
EnterCriticalSection((LPCRITICAL_SECTION)lock);
#else
#error not implemented on this platform
#endif
}

void lock_unlock(LOCK lock)
{
#if defined(CONF_FAMILY_UNIX)
int result = pthread_mutex_unlock((LOCKINTERNAL *)lock);
if(result != 0)
dbg_msg("lock", "unlock failed: %d", result);
#elif defined(CONF_FAMILY_WINDOWS)
LeaveCriticalSection((LPCRITICAL_SECTION)lock);
#else
#error not implemented on this platform
#endif
}
#ifdef __clang__
#pragma clang diagnostic pop
#endif

#if defined(CONF_FAMILY_WINDOWS)
void sphore_init(SEMAPHORE *sem)
{
Expand Down
Loading

0 comments on commit 7fbe77b

Please sign in to comment.