From fd4c9d7958110f40daa4e9ad500a1c04bb89904c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Thu, 9 Nov 2023 00:08:54 +0100 Subject: [PATCH] Use `std::mutex` instead of base `lock_*` functions Use `std::lock_guard` instead of `CLockScope`. Rename `lock` variables to `mutex`. --- CMakeLists.txt | 19 ++- src/base/lock_scope.h | 24 ---- src/base/log.cpp | 10 +- src/base/logger.h | 6 +- src/base/system.cpp | 152 ++++------------------- src/base/system.h | 37 ------ src/engine/client/serverbrowser_http.cpp | 27 ++-- src/engine/client/sound.h | 2 +- src/engine/client/updater.cpp | 19 ++- src/engine/client/updater.h | 17 +-- src/engine/client/video.cpp | 23 ++-- src/engine/client/video.h | 12 +- src/engine/server/register.cpp | 52 ++++---- src/engine/server/server_logger.h | 2 +- src/engine/shared/http.cpp | 18 ++- src/engine/shared/jobs.cpp | 9 +- src/engine/shared/jobs.h | 9 +- src/game/client/components/console.cpp | 9 +- src/game/client/components/console.h | 6 +- src/test/thread.cpp | 17 --- 20 files changed, 127 insertions(+), 343 deletions(-) delete mode 100644 src/base/lock_scope.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 59ad0054332..e2f3f8ef77e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -296,9 +296,11 @@ 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++) + add_linker_flag_if_supported(OUR_FLAGS_LINK -static) + add_linker_flag_if_supported(OUR_FLAGS_LINK -no-pie) if(EXCEPTION_HANDLING) add_cxx_compiler_flag_if_supported(OUR_FLAGS -DCONF_EXCEPTION_HANDLING) @@ -323,7 +325,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 @@ -1844,7 +1846,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 @@ -3285,6 +3286,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}) @@ -3292,15 +3294,8 @@ foreach(target ${TARGETS_LINK}) set_property(TARGET ${target} APPEND PROPERTY LINK_FLAGS /SAFESEH:NO) # Disable SafeSEH because the shipped libraries don't support it (would cause error LNK2026 otherwise). endif() if(TARGET_OS STREQUAL "mac") - target_link_libraries(${target} -stdlib=libc++) target_link_libraries(${target} "-framework SystemConfiguration") # Required by curl 7.79.0 endif() - if((MINGW OR TARGET_OS STREQUAL "linux") AND PREFER_BUNDLED_LIBS) - # Statically link the standard libraries with on MinGW/Linux so we don't - # have to ship them as DLLs. - target_link_libraries(${target} -static-libgcc) - target_link_libraries(${target} -static-libstdc++) - endif() endforeach() foreach(target ${TARGETS_OWN}) diff --git a/src/base/lock_scope.h b/src/base/lock_scope.h deleted file mode 100644 index ce7ea63fc6a..00000000000 --- a/src/base/lock_scope.h +++ /dev/null @@ -1,24 +0,0 @@ -#ifndef BASE_LOCK_SCOPE_H -#define BASE_LOCK_SCOPE_H - -#include "system.h" - -class SCOPED_CAPABILITY CLockScope -{ -public: - CLockScope(LOCK Lock) ACQUIRE(Lock, m_Lock) : - m_Lock(Lock) - { - lock_wait(m_Lock); - } - - ~CLockScope() RELEASE() REQUIRES(m_Lock) - { - lock_unlock(m_Lock); - } - -private: - LOCK m_Lock; -}; - -#endif diff --git a/src/base/log.cpp b/src/base/log.cpp index 6788969c57d..fe2af422887 100644 --- a/src/base/log.cpp +++ b/src/base/log.cpp @@ -447,9 +447,9 @@ std::unique_ptr log_logger_windows_debugger() void CFutureLogger::Set(std::shared_ptr pLogger) { - std::shared_ptr null; - m_PendingLock.lock(); - if(!std::atomic_compare_exchange_strong_explicit(&m_pLogger, &null, pLogger, std::memory_order_acq_rel, std::memory_order_acq_rel)) + const std::lock_guard LockGuard(m_PendingLock); + std::shared_ptr pNullLogger; + if(!std::atomic_compare_exchange_strong_explicit(&m_pLogger, &pNullLogger, pLogger, std::memory_order_acq_rel, std::memory_order_acq_rel)) { dbg_assert(false, "future logger has already been set and can only be set once"); } @@ -461,7 +461,6 @@ void CFutureLogger::Set(std::shared_ptr pLogger) } m_vPending.clear(); m_vPending.shrink_to_fit(); - m_PendingLock.unlock(); } void CFutureLogger::Log(const CLogMessage *pMessage) @@ -472,7 +471,7 @@ void CFutureLogger::Log(const CLogMessage *pMessage) pLogger->Log(pMessage); return; } - m_PendingLock.lock(); + const std::lock_guard LockGuard(m_PendingLock); pLogger = std::atomic_load_explicit(&m_pLogger, std::memory_order_relaxed); if(pLogger) { @@ -480,7 +479,6 @@ void CFutureLogger::Log(const CLogMessage *pMessage) return; } m_vPending.push_back(*pMessage); - m_PendingLock.unlock(); } void CFutureLogger::GlobalFinish() diff --git a/src/base/logger.h b/src/base/logger.h index 1324d84d122..f1b08b8b5d1 100644 --- a/src/base/logger.h +++ b/src/base/logger.h @@ -2,6 +2,8 @@ #define BASE_LOGGER_H #include "log.h" +#include "system.h" + #include #include #include @@ -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 pLogger); - void Log(const CLogMessage *pMessage) override; + void Set(std::shared_ptr pLogger) EXCLUDES(m_PendingLock); + void Log(const CLogMessage *pMessage) override EXCLUDES(m_PendingLock); void GlobalFinish() override; void OnFilterChange() override; }; diff --git a/src/base/system.cpp b/src/base/system.cpp index d737a0998c7..181fbde5cf5 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -3,24 +3,22 @@ #include #include #include +#include +#include #include #include #include #include #include // std::size +#include #include #include "system.h" -#include "lock_scope.h" #include "logger.h" #include -#include - -#include - #if defined(CONF_WEBSOCKETS) #include #endif @@ -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; @@ -524,19 +522,18 @@ 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; } } @@ -544,7 +541,7 @@ static void aio_thread(void *user) { ASYNCIO *aio = (ASYNCIO *)user; - lock_wait(aio->lock); + aio->mutex.lock(); while(true) { struct BUFFERS buffers; @@ -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; } @@ -589,26 +586,25 @@ 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; @@ -616,8 +612,7 @@ ASYNCIO *aio_new(IOHANDLE io) if(!aio->buffer) { sphore_destroy(&aio->sphore); - lock_destroy(aio->lock); - free(aio); + delete aio; return 0; } aio->buffer_size = ASYNC_BUFSIZE; @@ -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; @@ -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); } @@ -746,13 +740,13 @@ void aio_write_newline(ASYNCIO *aio) int aio_error(ASYNCIO *aio) { - CLockScope ls(aio->lock); + const std::lock_guard 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); @@ -764,7 +758,7 @@ void aio_free(ASYNCIO *aio) void aio_close(ASYNCIO *aio) { { - CLockScope ls(aio->lock); + const std::lock_guard lock_guard(aio->mutex); aio->finish = ASYNCIO_CLOSE; } sphore_signal(&aio->sphore); @@ -774,7 +768,7 @@ void aio_wait(ASYNCIO *aio) { void *thread; { - CLockScope ls(aio->lock); + const std::lock_guard lock_guard(aio->mutex); thread = aio->thread; aio->thread = 0; if(aio->finish == ASYNCIO_RUNNING) @@ -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) { diff --git a/src/base/system.h b/src/base/system.h index 0347df22b57..0c9e45ac6d5 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -594,19 +594,6 @@ void thread_yield(); */ void thread_detach(void *thread); -/** - * Creates a new thread and if it succeeded detaches it. - * - * @ingroup Threads - * - * @param threadfunc Entry point for the new thread. - * @param user Pointer to pass to the thread. - * @param name Name describing the use of the thread - * - * @return true on success, false on failure. - */ -bool thread_init_and_detach(void (*threadfunc)(void *), void *user, const char *name); - // Enable thread safety attributes only with clang. // The attributes can be safely erased when compiling with other compilers. #if defined(__clang__) && (!defined(SWIG)) @@ -683,30 +670,6 @@ bool thread_init_and_detach(void (*threadfunc)(void *), void *user, const char * * @see Threads */ -typedef CAPABILITY("mutex") void *LOCK; - -/** - * @ingroup Locks - */ -LOCK lock_create(); -/** - * @ingroup Locks - */ -void lock_destroy(LOCK lock); - -/** - * @ingroup Locks - */ -int lock_trylock(LOCK lock) TRY_ACQUIRE(1, lock); -/** - * @ingroup Locks - */ -void lock_wait(LOCK lock) ACQUIRE(lock); -/** - * @ingroup Locks - */ -void lock_unlock(LOCK lock) RELEASE(lock); - /* Group: Semaphores */ #if defined(CONF_FAMILY_WINDOWS) typedef void *SEMAPHORE; diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index 978ac9e16c7..5590dd7f924 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -1,5 +1,7 @@ #include "serverbrowser_http.h" +#include + #include #include #include @@ -10,14 +12,10 @@ #include #include -#include -#include - +#include #include #include -#include - using namespace std::chrono_literals; class CChooseMaster @@ -51,17 +49,16 @@ class CChooseMaster }; class CJob : public IJob { - LOCK m_Lock; + std::mutex m_Mutex; std::shared_ptr m_pData; - std::unique_ptr m_pHead PT_GUARDED_BY(m_Lock); - std::unique_ptr m_pGet PT_GUARDED_BY(m_Lock); - void Run() override REQUIRES(!m_Lock); + std::unique_ptr m_pHead PT_GUARDED_BY(m_Mutex); + std::unique_ptr m_pGet PT_GUARDED_BY(m_Mutex); + void Run() override EXCLUDES(m_Mutex); public: CJob(std::shared_ptr pData) : - m_pData(std::move(pData)) { m_Lock = lock_create(); } - ~CJob() override { lock_destroy(m_Lock); } - void Abort() REQUIRES(!m_Lock); + m_pData(std::move(pData)) {} + void Abort() EXCLUDES(m_Mutex); }; IEngine *m_pEngine; @@ -134,7 +131,7 @@ void CChooseMaster::Refresh() void CChooseMaster::CJob::Abort() { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); if(m_pHead != nullptr) { m_pHead->Abort(); @@ -176,7 +173,7 @@ void CChooseMaster::CJob::Run() pHead->Timeout(Timeout); pHead->LogProgress(HTTPLOG::FAILURE); { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); m_pHead = std::unique_ptr(pHead); } IEngine::RunJobBlocking(pHead); @@ -194,7 +191,7 @@ void CChooseMaster::CJob::Run() pGet->Timeout(Timeout); pGet->LogProgress(HTTPLOG::FAILURE); { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); m_pGet = std::unique_ptr(pGet); } IEngine::RunJobBlocking(pGet); diff --git a/src/engine/client/sound.h b/src/engine/client/sound.h index 7ea40cca3f8..f1e951be761 100644 --- a/src/engine/client/sound.h +++ b/src/engine/client/sound.h @@ -120,7 +120,7 @@ class CSound : public IEngineSound void StopVoice(CVoiceHandle Voice) override; bool IsPlaying(int SampleID) override; - void Mix(short *pFinalOut, unsigned Frames) override; + void Mix(short *pFinalOut, unsigned Frames) override EXCLUDES(m_SoundLock); void PauseAudioDevice() override; void UnpauseAudioDevice() override; }; diff --git a/src/engine/client/updater.cpp b/src/engine/client/updater.cpp index 16a03048b35..386536bb799 100644 --- a/src/engine/client/updater.cpp +++ b/src/engine/client/updater.cpp @@ -1,6 +1,7 @@ #include "updater.h" -#include + #include + #include #include #include @@ -55,7 +56,7 @@ CUpdaterFetchTask::CUpdaterFetchTask(CUpdater *pUpdater, const char *pFile, cons void CUpdaterFetchTask::OnProgress() { - CLockScope ls(m_pUpdater->m_Lock); + const std::lock_guard LockGuard(m_pUpdater->m_Mutex); str_copy(m_pUpdater->m_aStatus, Dest()); m_pUpdater->m_Percent = Progress(); } @@ -94,7 +95,6 @@ CUpdater::CUpdater() m_pEngine = NULL; m_State = CLEAN; m_Percent = 0; - m_Lock = lock_create(); IStorage::FormatTmpPath(m_aClientExecTmp, sizeof(m_aClientExecTmp), CLIENT_EXEC); IStorage::FormatTmpPath(m_aServerExecTmp, sizeof(m_aServerExecTmp), SERVER_EXEC); @@ -107,32 +107,27 @@ void CUpdater::Init() m_pEngine = Kernel()->RequestInterface(); } -CUpdater::~CUpdater() -{ - lock_destroy(m_Lock); -} - void CUpdater::SetCurrentState(int NewState) { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); m_State = NewState; } int CUpdater::GetCurrentState() { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); return m_State; } void CUpdater::GetCurrentFile(char *pBuf, int BufSize) { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); str_copy(pBuf, m_aStatus, BufSize); } int CUpdater::GetCurrentPercent() { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); return m_Percent; } diff --git a/src/engine/client/updater.h b/src/engine/client/updater.h index 7571d58675e..694a6c52307 100644 --- a/src/engine/client/updater.h +++ b/src/engine/client/updater.h @@ -2,7 +2,9 @@ #define ENGINE_CLIENT_UPDATER_H #include + #include +#include #include #define CLIENT_EXEC "DDNet" @@ -39,11 +41,11 @@ class CUpdater : public IUpdater class IStorage *m_pStorage; class IEngine *m_pEngine; - LOCK m_Lock; + std::mutex m_Mutex; int m_State; - char m_aStatus[256] GUARDED_BY(m_Lock); - int m_Percent GUARDED_BY(m_Lock); + char m_aStatus[256] GUARDED_BY(m_Mutex); + int m_Percent GUARDED_BY(m_Mutex); char m_aLastFile[256]; char m_aClientExecTmp[64]; char m_aServerExecTmp[64]; @@ -64,15 +66,14 @@ class CUpdater : public IUpdater bool ReplaceClient(); bool ReplaceServer(); - void SetCurrentState(int NewState) REQUIRES(!m_Lock); + void SetCurrentState(int NewState) EXCLUDES(m_Mutex); public: CUpdater(); - ~CUpdater(); - int GetCurrentState() override REQUIRES(!m_Lock); - void GetCurrentFile(char *pBuf, int BufSize) override REQUIRES(!m_Lock); - int GetCurrentPercent() override REQUIRES(!m_Lock); + int GetCurrentState() override EXCLUDES(m_Mutex); + void GetCurrentFile(char *pBuf, int BufSize) override EXCLUDES(m_Mutex); + int GetCurrentPercent() override EXCLUDES(m_Mutex); void InitiateUpdate() override; void Init(); diff --git a/src/engine/client/video.cpp b/src/engine/client/video.cpp index 8fe839a8f59..0bc955dc995 100644 --- a/src/engine/client/video.cpp +++ b/src/engine/client/video.cpp @@ -1,11 +1,11 @@ #if defined(CONF_VIDEORECORDER) -#include -#include +#include "video.h" -#include #include +#include #include +#include extern "C" { #include @@ -14,12 +14,9 @@ extern "C" { #include }; +#include #include #include - -#include "video.h" - -#include #include using namespace std::chrono_literals; @@ -35,7 +32,7 @@ using namespace std::chrono_literals; #endif const size_t FORMAT_GL_NCHANNELS = 4; -LOCK g_WriteLock = 0; +std::mutex g_WriteMutex; CVideo::CVideo(CGraphics_Threaded *pGraphics, ISound *pSound, IStorage *pStorage, int Width, int Height, const char *pName) : m_pGraphics(pGraphics), @@ -66,13 +63,11 @@ CVideo::CVideo(CGraphics_Threaded *pGraphics, ISound *pSound, IStorage *pStorage ms_TickTime = time_freq() / m_FPS; ms_pCurrentVideo = this; - g_WriteLock = lock_create(); } CVideo::~CVideo() { ms_pCurrentVideo = 0; - lock_destroy(g_WriteLock); } void CVideo::Start() @@ -165,7 +160,7 @@ void CVideo::Start() for(size_t i = 0; i < m_VideoThreads; ++i) { std::unique_lock Lock(m_vVideoThreads[i]->m_Mutex); - m_vVideoThreads[i]->m_Thread = std::thread([this, i]() REQUIRES(!g_WriteLock) { RunVideoThread(i == 0 ? (m_VideoThreads - 1) : (i - 1), i); }); + m_vVideoThreads[i]->m_Thread = std::thread([this, i]() EXCLUDES(g_WriteMutex) { RunVideoThread(i == 0 ? (m_VideoThreads - 1) : (i - 1), i); }); m_vVideoThreads[i]->m_Cond.wait(Lock, [this, i]() -> bool { return m_vVideoThreads[i]->m_Started; }); } @@ -177,7 +172,7 @@ void CVideo::Start() for(size_t i = 0; i < m_AudioThreads; ++i) { std::unique_lock Lock(m_vAudioThreads[i]->m_Mutex); - m_vAudioThreads[i]->m_Thread = std::thread([this, i]() REQUIRES(!g_WriteLock) { RunAudioThread(i == 0 ? (m_AudioThreads - 1) : (i - 1), i); }); + m_vAudioThreads[i]->m_Thread = std::thread([this, i]() EXCLUDES(g_WriteMutex) { RunAudioThread(i == 0 ? (m_AudioThreads - 1) : (i - 1), i); }); m_vAudioThreads[i]->m_Cond.wait(Lock, [this, i]() -> bool { return m_vAudioThreads[i]->m_Started; }); } @@ -476,7 +471,7 @@ void CVideo::RunAudioThread(size_t ParentThreadIndex, size_t ThreadIndex) std::unique_lock LockAudio(pThreadData->m_AudioFillMutex); { - CLockScope ls(g_WriteLock); + const std::lock_guard LockGuard(g_WriteMutex); m_AudioStream.m_vpFrames[ThreadIndex]->pts = av_rescale_q(pThreadData->m_SampleCountStart, AVRational{1, m_AudioStream.pEnc->sample_rate}, m_AudioStream.pEnc->time_base); WriteFrame(&m_AudioStream, ThreadIndex); } @@ -557,7 +552,7 @@ void CVideo::RunVideoThread(size_t ParentThreadIndex, size_t ThreadIndex) { std::unique_lock LockVideo(pThreadData->m_VideoFillMutex); { - CLockScope ls(g_WriteLock); + const std::lock_guard LockGuard(g_WriteMutex); m_VideoStream.m_vpFrames[ThreadIndex]->pts = (int64_t)m_VideoStream.pEnc->FRAME_NUM; WriteFrame(&m_VideoStream, ThreadIndex); } diff --git a/src/engine/client/video.h b/src/engine/client/video.h index 38a4ec02c69..682fe03ed3d 100644 --- a/src/engine/client/video.h +++ b/src/engine/client/video.h @@ -21,7 +21,7 @@ class CGraphics_Threaded; class ISound; class IStorage; -extern LOCK g_WriteLock; +extern std::mutex g_WriteMutex; // a wrapper around a single output AVStream struct OutputStream @@ -47,7 +47,7 @@ class CVideo : public IVideo CVideo(CGraphics_Threaded *pGraphics, ISound *pSound, IStorage *pStorage, int Width, int Height, const char *pName); ~CVideo(); - void Start() override REQUIRES(!g_WriteLock); + void Start() override EXCLUDES(g_WriteMutex); void Stop() override; void Pause(bool Pause) override; bool IsRecording() override { return m_Recording; } @@ -63,11 +63,11 @@ class CVideo : public IVideo static void Init() { av_log_set_level(AV_LOG_DEBUG); } private: - void RunVideoThread(size_t ParentThreadIndex, size_t ThreadIndex) REQUIRES(!g_WriteLock); - void FillVideoFrame(size_t ThreadIndex) REQUIRES(!g_WriteLock); + void RunVideoThread(size_t ParentThreadIndex, size_t ThreadIndex) EXCLUDES(g_WriteMutex); + void FillVideoFrame(size_t ThreadIndex) EXCLUDES(g_WriteMutex); void ReadRGBFromGL(size_t ThreadIndex); - void RunAudioThread(size_t ParentThreadIndex, size_t ThreadIndex) REQUIRES(!g_WriteLock); + void RunAudioThread(size_t ParentThreadIndex, size_t ThreadIndex) EXCLUDES(g_WriteMutex); void FillAudioFrame(size_t ThreadIndex); bool OpenVideo(); @@ -75,7 +75,7 @@ class CVideo : public IVideo AVFrame *AllocPicture(enum AVPixelFormat PixFmt, int Width, int Height); AVFrame *AllocAudioFrame(enum AVSampleFormat SampleFmt, uint64_t ChannelLayout, int SampleRate, int NbSamples); - void WriteFrame(OutputStream *pStream, size_t ThreadIndex) REQUIRES(g_WriteLock); + void WriteFrame(OutputStream *pStream, size_t ThreadIndex) REQUIRES(g_WriteMutex); void FinishFrames(OutputStream *pStream); void CloseStream(OutputStream *pStream); diff --git a/src/engine/server/register.cpp b/src/engine/server/register.cpp index b262b77aec9..94c7395019c 100644 --- a/src/engine/server/register.cpp +++ b/src/engine/server/register.cpp @@ -1,7 +1,7 @@ #include "register.h" -#include #include + #include #include #include @@ -12,6 +12,8 @@ #include #include +#include + class CRegister : public IRegister { enum @@ -40,14 +42,9 @@ class CRegister : public IRegister class CGlobal { public: - ~CGlobal() - { - lock_destroy(m_Lock); - } - - LOCK m_Lock = lock_create(); - int m_InfoSerial GUARDED_BY(m_Lock) = -1; - int m_LatestSuccessfulInfoSerial GUARDED_BY(m_Lock) = -1; + std::mutex m_Mutex; + int m_InfoSerial GUARDED_BY(m_Mutex) = -1; + int m_LatestSuccessfulInfoSerial GUARDED_BY(m_Mutex) = -1; }; class CProtocol @@ -59,16 +56,12 @@ class CRegister : public IRegister m_pGlobal(std::move(pGlobal)) { } - ~CShared() - { - lock_destroy(m_Lock); - } std::shared_ptr m_pGlobal; - LOCK m_Lock = lock_create(); - int m_NumTotalRequests GUARDED_BY(m_Lock) = 0; - int m_LatestResponseStatus GUARDED_BY(m_Lock) = STATUS_NONE; - int m_LatestResponseIndex GUARDED_BY(m_Lock) = -1; + std::mutex m_Mutex; + int m_NumTotalRequests GUARDED_BY(m_Mutex) = 0; + int m_LatestResponseStatus GUARDED_BY(m_Mutex) = STATUS_NONE; + int m_LatestResponseIndex GUARDED_BY(m_Mutex) = -1; }; class CJob : public IJob @@ -274,7 +267,7 @@ void CRegister::CProtocol::SendRegister() bool SendInfo; { - CLockScope ls(m_pShared->m_pGlobal->m_Lock); + const std::lock_guard LockGuard(m_pShared->m_pGlobal->m_Mutex); InfoSerial = m_pShared->m_pGlobal->m_InfoSerial; SendInfo = InfoSerial > m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial; } @@ -309,7 +302,7 @@ void CRegister::CProtocol::SendRegister() int RequestIndex; { - CLockScope ls(m_pShared->m_Lock); + const std::lock_guard LockGuard(m_pShared->m_Mutex); if(m_pShared->m_LatestResponseStatus != STATUS_OK) { log_info(ProtocolToSystem(m_Protocol), "registering..."); @@ -326,13 +319,12 @@ void CRegister::CProtocol::SendRegister() void CRegister::CProtocol::SendDeleteIfRegistered(bool Shutdown) { - lock_wait(m_pShared->m_Lock); - bool ShouldSendDelete = m_pShared->m_LatestResponseStatus == STATUS_OK; - m_pShared->m_LatestResponseStatus = STATUS_NONE; - lock_unlock(m_pShared->m_Lock); - if(!ShouldSendDelete) { - return; + const std::lock_guard LockGuard(m_pShared->m_Mutex); + const bool ShouldSendDelete = m_pShared->m_LatestResponseStatus == STATUS_OK; + m_pShared->m_LatestResponseStatus = STATUS_NONE; + if(!ShouldSendDelete) + return; } char aAddress[64]; @@ -369,7 +361,7 @@ CRegister::CProtocol::CProtocol(CRegister *pParent, int Protocol) : void CRegister::CProtocol::CheckChallengeStatus() { - CLockScope ls(m_pShared->m_Lock); + const std::lock_guard LockGuard(m_pShared->m_Mutex); // No requests in flight? if(m_pShared->m_LatestResponseIndex == m_pShared->m_NumTotalRequests - 1) { @@ -444,7 +436,7 @@ void CRegister::CProtocol::CJob::Run() return; } { - CLockScope ls(m_pShared->m_Lock); + const std::lock_guard LockGuard(m_pShared->m_Mutex); if(Status != STATUS_OK || Status != m_pShared->m_LatestResponseStatus) { log_debug(ProtocolToSystem(m_Protocol), "status: %s", (const char *)StatusString); @@ -463,7 +455,7 @@ void CRegister::CProtocol::CJob::Run() } if(Status == STATUS_OK) { - CLockScope ls(m_pShared->m_pGlobal->m_Lock); + const std::lock_guard LockGuard(m_pShared->m_pGlobal->m_Mutex); if(m_InfoSerial > m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial) { m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial = m_InfoSerial; @@ -471,7 +463,7 @@ void CRegister::CProtocol::CJob::Run() } else if(Status == STATUS_NEEDINFO) { - CLockScope ls(m_pShared->m_pGlobal->m_Lock); + const std::lock_guard LockGuard(m_pShared->m_pGlobal->m_Mutex); if(m_InfoSerial == m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial) { // Tell other requests that they need to send the info again. @@ -692,7 +684,7 @@ void CRegister::OnNewInfo(const char *pInfo) m_GotServerInfo = true; str_copy(m_aServerInfo, pInfo); { - CLockScope ls(m_pGlobal->m_Lock); + const std::lock_guard LockGuard(m_pGlobal->m_Mutex); m_pGlobal->m_InfoSerial += 1; } diff --git a/src/engine/server/server_logger.h b/src/engine/server/server_logger.h index 86406d3c0b0..a998058d36b 100644 --- a/src/engine/server/server_logger.h +++ b/src/engine/server/server_logger.h @@ -15,7 +15,7 @@ class CServerLogger : public ILogger public: CServerLogger(CServer *pServer); - void Log(const CLogMessage *pMessage) override; + void Log(const CLogMessage *pMessage) override EXCLUDES(m_PendingLock); // Must be called from the main thread! void OnServerDeletion(); }; diff --git a/src/engine/shared/http.cpp b/src/engine/shared/http.cpp index d024ba5e95f..f1421230dcc 100644 --- a/src/engine/shared/http.cpp +++ b/src/engine/shared/http.cpp @@ -12,15 +12,17 @@ #include #endif +#include + #define WIN32_LEAN_AND_MEAN #include // TODO: Non-global pls? static CURLSH *gs_pShare; -static LOCK gs_aLocks[CURL_LOCK_DATA_LAST + 1]; +static std::mutex gs_aMutexes[CURL_LOCK_DATA_LAST + 1]; static bool gs_Initialized = false; -static int GetLockIndex(int Data) +static int GetMutexIndex(int Data) { if(!(0 <= Data && Data < CURL_LOCK_DATA_LAST)) { @@ -29,19 +31,19 @@ static int GetLockIndex(int Data) return Data; } -static void CurlLock(CURL *pHandle, curl_lock_data Data, curl_lock_access Access, void *pUser) ACQUIRE(gs_aLocks[GetLockIndex(Data)]) +static void CurlLock(CURL *pHandle, curl_lock_data Data, curl_lock_access Access, void *pUser) ACQUIRE(gs_aMutexes[GetMutexIndex(Data)]) { (void)pHandle; (void)Access; (void)pUser; - lock_wait(gs_aLocks[GetLockIndex(Data)]); + gs_aMutexes[GetMutexIndex(Data)].lock(); } -static void CurlUnlock(CURL *pHandle, curl_lock_data Data, void *pUser) RELEASE(gs_aLocks[GetLockIndex(Data)]) +static void CurlUnlock(CURL *pHandle, curl_lock_data Data, void *pUser) RELEASE(gs_aMutexes[GetMutexIndex(Data)]) { (void)pHandle; (void)pUser; - lock_unlock(gs_aLocks[GetLockIndex(Data)]); + gs_aMutexes[GetMutexIndex(Data)].unlock(); } int CurlDebug(CURL *pHandle, curl_infotype Type, char *pData, size_t DataSize, void *pUser) @@ -88,10 +90,6 @@ bool HttpInit(IStorage *pStorage) dbg_msg("http", "libcurl version %s (compiled = " LIBCURL_VERSION ")", pVersion->version); } - for(auto &Lock : gs_aLocks) - { - Lock = lock_create(); - } curl_share_setopt(gs_pShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); curl_share_setopt(gs_pShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); curl_share_setopt(gs_pShare, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); diff --git a/src/engine/shared/jobs.cpp b/src/engine/shared/jobs.cpp index 36ee17d8d11..a25a0f4f0f5 100644 --- a/src/engine/shared/jobs.cpp +++ b/src/engine/shared/jobs.cpp @@ -1,8 +1,7 @@ /* (c) Magnus Auvinen. See licence.txt in the root of the distribution for more information. */ /* If you are missing that file, acquire a complete release at teeworlds.com. */ -#include "jobs.h" -#include +#include "jobs.h" IJob::IJob() : m_Status(STATE_PENDING) @@ -20,7 +19,6 @@ CJobPool::CJobPool() { // empty the pool m_Shutdown = false; - m_Lock = lock_create(); sphore_init(&m_Semaphore); m_pFirstJob = 0; m_pLastJob = 0; @@ -45,7 +43,7 @@ void CJobPool::WorkerThread(void *pUser) // fetch job from queue sphore_wait(&pPool->m_Semaphore); { - CLockScope ls(pPool->m_Lock); + const std::lock_guard LockGuard(pPool->m_Mutex); if(pPool->m_pFirstJob) { pJob = pPool->m_pFirstJob; @@ -85,14 +83,13 @@ void CJobPool::Destroy() for(void *pThread : m_vpThreads) thread_wait(pThread); m_vpThreads.clear(); - lock_destroy(m_Lock); sphore_destroy(&m_Semaphore); } void CJobPool::Add(std::shared_ptr pJob) { { - CLockScope ls(m_Lock); + const std::lock_guard LockGuard(m_Mutex); // add job to queue if(m_pLastJob) m_pLastJob->m_pNext = pJob; diff --git a/src/engine/shared/jobs.h b/src/engine/shared/jobs.h index 2a24efbdcc0..78c4fa943d0 100644 --- a/src/engine/shared/jobs.h +++ b/src/engine/shared/jobs.h @@ -7,6 +7,7 @@ #include #include +#include #include class CJobPool; @@ -41,10 +42,10 @@ class CJobPool std::vector m_vpThreads; std::atomic m_Shutdown; - LOCK m_Lock; + std::mutex m_Mutex; SEMAPHORE m_Semaphore; - std::shared_ptr m_pFirstJob GUARDED_BY(m_Lock); - std::shared_ptr m_pLastJob GUARDED_BY(m_Lock); + std::shared_ptr m_pFirstJob GUARDED_BY(m_Mutex); + std::shared_ptr m_pLastJob GUARDED_BY(m_Mutex); static void WorkerThread(void *pUser) NO_THREAD_SAFETY_ANALYSIS; @@ -54,7 +55,7 @@ class CJobPool void Init(int NumThreads); void Destroy(); - void Add(std::shared_ptr pJob) REQUIRES(!m_Lock); + void Add(std::shared_ptr pJob) EXCLUDES(m_Mutex); static void RunBlocking(IJob *pJob); }; #endif diff --git a/src/game/client/components/console.cpp b/src/game/client/components/console.cpp index 99754dbfc72..cfe555b3a65 100644 --- a/src/game/client/components/console.cpp +++ b/src/game/client/components/console.cpp @@ -119,22 +119,20 @@ void CGameConsole::CInstance::Init(CGameConsole *pGameConsole) void CGameConsole::CInstance::ClearBacklog() { - m_BacklogLock.lock(); + const std::lock_guard LockGuard(m_BacklogLock); m_Backlog.Init(); m_BacklogCurPage = 0; - m_BacklogLock.unlock(); } void CGameConsole::CInstance::ClearBacklogYOffsets() { - m_BacklogLock.lock(); + const std::lock_guard LockGuard(m_BacklogLock); auto *pEntry = m_Backlog.First(); while(pEntry) { pEntry->m_YOffset = -1.0f; pEntry = m_Backlog.Next(pEntry); } - m_BacklogLock.unlock(); } void CGameConsole::CInstance::ClearHistory() @@ -385,13 +383,12 @@ bool CGameConsole::CInstance::OnInput(const IInput::CEvent &Event) void CGameConsole::CInstance::PrintLine(const char *pLine, int Len, ColorRGBA PrintColor) { - m_BacklogLock.lock(); + const std::lock_guard LockGuard(m_BacklogLock); CBacklogEntry *pEntry = m_Backlog.Allocate(sizeof(CBacklogEntry) + Len); pEntry->m_YOffset = -1.0f; pEntry->m_PrintColor = PrintColor; str_copy(pEntry->m_aText, pLine, Len + 1); m_NewLineCounter++; - m_BacklogLock.unlock(); } CGameConsole::CGameConsole() : diff --git a/src/game/client/components/console.h b/src/game/client/components/console.h index ba734a84935..98be595bba3 100644 --- a/src/game/client/components/console.h +++ b/src/game/client/components/console.h @@ -76,15 +76,15 @@ class CGameConsole : public CComponent CInstance(int t); void Init(CGameConsole *pGameConsole); - void ClearBacklog(); - void ClearBacklogYOffsets(); + void ClearBacklog() EXCLUDES(m_BacklogLock); + void ClearBacklogYOffsets() EXCLUDES(m_BacklogLock); void ClearHistory(); void Reset(); void ExecuteLine(const char *pLine); bool OnInput(const IInput::CEvent &Event); - void PrintLine(const char *pLine, int Len, ColorRGBA PrintColor); + void PrintLine(const char *pLine, int Len, ColorRGBA PrintColor) EXCLUDES(m_BacklogLock); const char *GetString() const { return m_Input.GetString(); } static void PossibleCommandsCompleteCallback(int Index, const char *pStr, void *pUser); diff --git a/src/test/thread.cpp b/src/test/thread.cpp index e0e242b4394..20109e71207 100644 --- a/src/test/thread.cpp +++ b/src/test/thread.cpp @@ -79,20 +79,3 @@ TEST(Thread, SemaphoreMultiThreaded) thread_wait(pThread); sphore_destroy(&Semaphore); } - -static void LockThread(void *pUser) -{ - LOCK *pLock = (LOCK *)pUser; - lock_wait(*pLock); - lock_unlock(*pLock); -} - -TEST(Thread, Lock) -{ - LOCK Lock = lock_create(); - lock_wait(Lock); - void *pThread = thread_init(LockThread, &Lock, "lock"); - lock_unlock(Lock); - thread_wait(pThread); - lock_destroy(Lock); -}