diff --git a/CMakeLists.txt b/CMakeLists.txt index 0b40dc9ba..38dae4940 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1683,6 +1683,7 @@ 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 diff --git a/src/base/lock_scope.h b/src/base/lock_scope.h new file mode 100644 index 000000000..130ab1f4e --- /dev/null +++ b/src/base/lock_scope.h @@ -0,0 +1,24 @@ +#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) REQUIRES(!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/system.cpp b/src/base/system.cpp index 6dbd07c67..ae12c834b 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -13,6 +13,7 @@ #include "system.h" +#include "lock_scope.h" #include "logger.h" #include @@ -624,11 +625,8 @@ void aio_write_newline(ASYNCIO *aio) int aio_error(ASYNCIO *aio) { - int result; - lock_wait(aio->lock); - result = aio->error; - lock_unlock(aio->lock); - return result; + CLockScope ls(aio->lock); + return aio->error; } void aio_free(ASYNCIO *aio) @@ -644,23 +642,25 @@ void aio_free(ASYNCIO *aio) void aio_close(ASYNCIO *aio) { - lock_wait(aio->lock); - aio->finish = ASYNCIO_CLOSE; - lock_unlock(aio->lock); + { + CLockScope ls(aio->lock); + aio->finish = ASYNCIO_CLOSE; + } sphore_signal(&aio->sphore); } void aio_wait(ASYNCIO *aio) { void *thread; - lock_wait(aio->lock); - thread = aio->thread; - aio->thread = 0; - if(aio->finish == ASYNCIO_RUNNING) { - aio->finish = ASYNCIO_EXIT; + CLockScope ls(aio->lock); + thread = aio->thread; + aio->thread = 0; + if(aio->finish == ASYNCIO_RUNNING) + { + aio->finish = ASYNCIO_EXIT; + } } - lock_unlock(aio->lock); sphore_signal(&aio->sphore); thread_wait(thread); } diff --git a/src/base/tl/threading.h b/src/base/tl/threading.h index 84ed6361a..bcedd27fc 100644 --- a/src/base/tl/threading.h +++ b/src/base/tl/threading.h @@ -28,44 +28,4 @@ public: } }; -class SCOPED_CAPABILITY CLock -{ - LOCK m_Lock; - -public: - CLock() ACQUIRE(m_Lock) - { - m_Lock = lock_create(); - } - - ~CLock() RELEASE() - { - lock_destroy(m_Lock); - } - - CLock(const CLock &) = delete; - - void Take() ACQUIRE(m_Lock) REQUIRES(!m_Lock) { lock_wait(m_Lock); } - void Release() RELEASE() REQUIRES(m_Lock) { lock_unlock(m_Lock); } -}; - -class CScopeLock -{ - CLock *m_pLock; - -public: - CScopeLock(CLock *pLock) - { - m_pLock = pLock; - m_pLock->Take(); - } - - ~CScopeLock() - { - m_pLock->Release(); - } - - CScopeLock(const CScopeLock &) = delete; -}; - #endif // BASE_TL_THREADING_H diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index 082882833..56f503724 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -133,7 +134,7 @@ void CChooseMaster::Refresh() void CChooseMaster::CJob::Abort() { - lock_wait(m_Lock); + CLockScope ls(m_Lock); if(m_pHead != nullptr) { m_pHead->Abort(); @@ -143,7 +144,6 @@ void CChooseMaster::CJob::Abort() { m_pGet->Abort(); } - lock_unlock(m_Lock); } void CChooseMaster::CJob::Run() @@ -175,9 +175,10 @@ void CChooseMaster::CJob::Run() CHttpRequest *pHead = HttpHead(pUrl).release(); pHead->Timeout(Timeout); pHead->LogProgress(HTTPLOG::FAILURE); - lock_wait(m_Lock); - m_pHead = std::unique_ptr(pHead); - lock_unlock(m_Lock); + { + CLockScope ls(m_Lock); + m_pHead = std::unique_ptr(pHead); + } IEngine::RunJobBlocking(pHead); if(pHead->State() == HTTP_ABORTED) { @@ -192,9 +193,10 @@ void CChooseMaster::CJob::Run() CHttpRequest *pGet = HttpGet(pUrl).release(); pGet->Timeout(Timeout); pGet->LogProgress(HTTPLOG::FAILURE); - lock_wait(m_Lock); - m_pGet = std::unique_ptr(pGet); - lock_unlock(m_Lock); + { + CLockScope ls(m_Lock); + m_pGet = std::unique_ptr(pGet); + } IEngine::RunJobBlocking(pGet); auto Time = std::chrono::duration_cast(tw::time_get() - StartTime); if(pHead->State() == HTTP_ABORTED) diff --git a/src/engine/client/updater.cpp b/src/engine/client/updater.cpp index 636fa3ee1..6df1ea2b2 100644 --- a/src/engine/client/updater.cpp +++ b/src/engine/client/updater.cpp @@ -1,4 +1,5 @@ #include "updater.h" +#include #include #include #include @@ -52,10 +53,9 @@ CUpdaterFetchTask::CUpdaterFetchTask(CUpdater *pUpdater, const char *pFile, cons void CUpdaterFetchTask::OnProgress() { - lock_wait(m_pUpdater->m_Lock); + CLockScope ls(m_pUpdater->m_Lock); str_copy(m_pUpdater->m_aStatus, Dest(), sizeof(m_pUpdater->m_aStatus)); m_pUpdater->m_Percent = Progress(); - lock_unlock(m_pUpdater->m_Lock); } int CUpdaterFetchTask::OnCompletion(int State) @@ -112,32 +112,26 @@ CUpdater::~CUpdater() void CUpdater::SetCurrentState(int NewState) { - lock_wait(m_Lock); + CLockScope ls(m_Lock); m_State = NewState; - lock_unlock(m_Lock); } int CUpdater::GetCurrentState() { - lock_wait(m_Lock); - int Result = m_State; - lock_unlock(m_Lock); - return Result; + CLockScope ls(m_Lock); + return m_State; } void CUpdater::GetCurrentFile(char *pBuf, int BufSize) { - lock_wait(m_Lock); + CLockScope ls(m_Lock); str_copy(pBuf, m_aStatus, BufSize); - lock_unlock(m_Lock); } int CUpdater::GetCurrentPercent() { - lock_wait(m_Lock); - int Result = m_Percent; - lock_unlock(m_Lock); - return Result; + CLockScope ls(m_Lock); + return m_Percent; } void CUpdater::FetchFile(const char *pFile, const char *pDestPath) diff --git a/src/engine/client/video.cpp b/src/engine/client/video.cpp index 315db0908..8c3320e49 100644 --- a/src/engine/client/video.cpp +++ b/src/engine/client/video.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -467,10 +468,11 @@ void CVideo::RunAudioThread(size_t ParentThreadIndex, size_t ThreadIndex) { std::unique_lock LockAudio(pThreadData->m_AudioFillMutex); - lock_wait(g_WriteLock); - 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); - lock_unlock(g_WriteLock); + { + CLockScope ls(g_WriteLock); + 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); + } pThreadData->m_AudioFrameToFill = 0; pThreadData->m_AudioFillCond.notify_all(); @@ -547,10 +549,11 @@ void CVideo::RunVideoThread(size_t ParentThreadIndex, size_t ThreadIndex) } { std::unique_lock LockVideo(pThreadData->m_VideoFillMutex); - lock_wait(g_WriteLock); - m_VideoStream.m_vpFrames[ThreadIndex]->pts = (int64_t)m_VideoStream.pEnc->frame_number; - WriteFrame(&m_VideoStream, ThreadIndex); - lock_unlock(g_WriteLock); + { + CLockScope ls(g_WriteLock); + m_VideoStream.m_vpFrames[ThreadIndex]->pts = (int64_t)m_VideoStream.pEnc->frame_number; + WriteFrame(&m_VideoStream, ThreadIndex); + } pThreadData->m_VideoFrameToFill = 0; pThreadData->m_VideoFillCond.notify_all(); diff --git a/src/engine/server/register.cpp b/src/engine/server/register.cpp index 5086a20a3..6346ec18b 100644 --- a/src/engine/server/register.cpp +++ b/src/engine/server/register.cpp @@ -1,5 +1,6 @@ #include "register.h" +#include #include #include #include @@ -263,11 +264,14 @@ void CRegister::CProtocol::SendRegister() FormatUuid(m_pParent->m_ChallengeSecret, aChallengeUuid, sizeof(aChallengeUuid)); char aChallengeSecret[64]; str_format(aChallengeSecret, sizeof(aChallengeSecret), "%s:%s", aChallengeUuid, ProtocolToString(m_Protocol)); + int InfoSerial; + bool SendInfo; - lock_wait(m_pShared->m_pGlobal->m_Lock); - int InfoSerial = m_pShared->m_pGlobal->m_InfoSerial; - bool SendInfo = InfoSerial > m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial; - lock_unlock(m_pShared->m_pGlobal->m_Lock); + { + CLockScope ls(m_pShared->m_pGlobal->m_Lock); + InfoSerial = m_pShared->m_pGlobal->m_InfoSerial; + SendInfo = InfoSerial > m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial; + } std::unique_ptr pRegister; if(SendInfo) @@ -297,14 +301,16 @@ void CRegister::CProtocol::SendRegister() pRegister->LogProgress(HTTPLOG::FAILURE); pRegister->IpResolve(ProtocolToIpresolve(m_Protocol)); - lock_wait(m_pShared->m_Lock); - if(m_pShared->m_LatestResponseStatus != STATUS_OK) + int RequestIndex; { - log_info(ProtocolToSystem(m_Protocol), "registering..."); + CLockScope ls(m_pShared->m_Lock); + if(m_pShared->m_LatestResponseStatus != STATUS_OK) + { + log_info(ProtocolToSystem(m_Protocol), "registering..."); + } + RequestIndex = m_pShared->m_NumTotalRequests; + m_pShared->m_NumTotalRequests += 1; } - int RequestIndex = m_pShared->m_NumTotalRequests; - m_pShared->m_NumTotalRequests += 1; - lock_unlock(m_pShared->m_Lock); m_pParent->m_pEngine->AddJob(std::make_shared(m_Protocol, m_pParent->m_ServerPort, RequestIndex, InfoSerial, m_pShared, std::move(pRegister))); m_NewChallengeToken = false; @@ -321,7 +327,7 @@ CRegister::CProtocol::CProtocol(CRegister *pParent, int Protocol) : void CRegister::CProtocol::CheckChallengeStatus() { - lock_wait(m_pShared->m_Lock); + CLockScope ls(m_pShared->m_Lock); // No requests in flight? if(m_pShared->m_LatestResponseIndex == m_pShared->m_NumTotalRequests - 1) { @@ -340,7 +346,6 @@ void CRegister::CProtocol::CheckChallengeStatus() break; } } - lock_unlock(m_pShared->m_Lock); } void CRegister::CProtocol::Update() @@ -396,41 +401,40 @@ void CRegister::CProtocol::CJob::Run() json_value_free(pJson); return; } - lock_wait(m_pShared->m_Lock); - if(Status != STATUS_OK || Status != m_pShared->m_LatestResponseStatus) { - log_debug(ProtocolToSystem(m_Protocol), "status: %s", (const char *)StatusString); + CLockScope ls(m_pShared->m_Lock); + if(Status != STATUS_OK || Status != m_pShared->m_LatestResponseStatus) + { + log_debug(ProtocolToSystem(m_Protocol), "status: %s", (const char *)StatusString); + } + if(Status == m_pShared->m_LatestResponseStatus && Status == STATUS_NEEDCHALLENGE) + { + log_error(ProtocolToSystem(m_Protocol), "ERROR: the master server reports that clients can not connect to this server."); + log_error(ProtocolToSystem(m_Protocol), "ERROR: configure your firewall/nat to let through udp on port %d.", m_ServerPort); + } + json_value_free(pJson); + if(m_Index > m_pShared->m_LatestResponseIndex) + { + m_pShared->m_LatestResponseIndex = m_Index; + m_pShared->m_LatestResponseStatus = Status; + } } - if(Status == m_pShared->m_LatestResponseStatus && Status == STATUS_NEEDCHALLENGE) - { - log_error(ProtocolToSystem(m_Protocol), "ERROR: the master server reports that clients can not connect to this server."); - log_error(ProtocolToSystem(m_Protocol), "ERROR: configure your firewall/nat to let through udp on port %d.", m_ServerPort); - } - json_value_free(pJson); - if(m_Index > m_pShared->m_LatestResponseIndex) - { - m_pShared->m_LatestResponseIndex = m_Index; - m_pShared->m_LatestResponseStatus = Status; - } - lock_unlock(m_pShared->m_Lock); if(Status == STATUS_OK) { - lock_wait(m_pShared->m_pGlobal->m_Lock); + CLockScope ls(m_pShared->m_pGlobal->m_Lock); if(m_InfoSerial > m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial) { m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial = m_InfoSerial; } - lock_unlock(m_pShared->m_pGlobal->m_Lock); } else if(Status == STATUS_NEEDINFO) { - lock_wait(m_pShared->m_pGlobal->m_Lock); + CLockScope ls(m_pShared->m_pGlobal->m_Lock); if(m_InfoSerial == m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial) { // Tell other requests that they need to send the info again. m_pShared->m_pGlobal->m_LatestSuccessfulInfoSerial -= 1; } - lock_unlock(m_pShared->m_pGlobal->m_Lock); } } @@ -602,9 +606,10 @@ void CRegister::OnNewInfo(const char *pInfo) m_GotServerInfo = true; str_copy(m_aServerInfo, pInfo, sizeof(m_aServerInfo)); - lock_wait(m_pGlobal->m_Lock); - m_pGlobal->m_InfoSerial += 1; - lock_unlock(m_pGlobal->m_Lock); + { + CLockScope ls(m_pGlobal->m_Lock); + m_pGlobal->m_InfoSerial += 1; + } // Immediately send new info if it changes, but at most once per second. int64_t Now = time_get(); diff --git a/src/engine/shared/jobs.cpp b/src/engine/shared/jobs.cpp index 64cc8e277..7141d3f07 100644 --- a/src/engine/shared/jobs.cpp +++ b/src/engine/shared/jobs.cpp @@ -2,6 +2,8 @@ /* If you are missing that file, acquire a complete release at teeworlds.com. */ #include "jobs.h" +#include + IJob::IJob() : m_Status(STATE_PENDING) { @@ -54,15 +56,16 @@ void CJobPool::WorkerThread(void *pUser) // fetch job from queue sphore_wait(&pPool->m_Semaphore); - lock_wait(pPool->m_Lock); - if(pPool->m_pFirstJob) { - pJob = pPool->m_pFirstJob; - pPool->m_pFirstJob = pPool->m_pFirstJob->m_pNext; - if(!pPool->m_pFirstJob) - pPool->m_pLastJob = 0; + CLockScope ls(pPool->m_Lock); + if(pPool->m_pFirstJob) + { + pJob = pPool->m_pFirstJob; + pPool->m_pFirstJob = pPool->m_pFirstJob->m_pNext; + if(!pPool->m_pFirstJob) + pPool->m_pLastJob = 0; + } } - lock_unlock(pPool->m_Lock); // do the job if we have one if(pJob) @@ -96,16 +99,16 @@ void CJobPool::Destroy() void CJobPool::Add(std::shared_ptr pJob) { - lock_wait(m_Lock); + { + CLockScope ls(m_Lock); + // add job to queue + if(m_pLastJob) + m_pLastJob->m_pNext = pJob; + m_pLastJob = std::move(pJob); + if(!m_pFirstJob) + m_pFirstJob = m_pLastJob; + } - // add job to queue - if(m_pLastJob) - m_pLastJob->m_pNext = pJob; - m_pLastJob = std::move(pJob); - if(!m_pFirstJob) - m_pFirstJob = m_pLastJob; - - lock_unlock(m_Lock); sphore_signal(&m_Semaphore); }