From a744aef227361b584c7af939bc13a39fa4447768 Mon Sep 17 00:00:00 2001 From: Niels Dekker Date: Tue, 31 Jan 2023 20:53:19 +0100 Subject: [PATCH] STYLE: Use `lock_guard` in Logger classes and GPUImageDataManager Replaced `m_Mutex.lock()`/`m_Mutex.unlock()` pairs, following C++ Core Guidelines, September 23, 2022, "Use RAII, never plain `lock()`/`unlock()`" https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#cp20-use-raii-never-plain-lockunlock --- .../Common/include/itkLoggerThreadWrapper.hxx | 90 +++++++++--------- .../Core/Common/src/itkStdStreamLogOutput.cxx | 12 +-- Modules/Core/Common/src/itkThreadLogger.cxx | 93 +++++++++---------- .../include/itkGPUImageDataManager.hxx | 8 +- 4 files changed, 91 insertions(+), 112 deletions(-) diff --git a/Modules/Core/Common/include/itkLoggerThreadWrapper.hxx b/Modules/Core/Common/include/itkLoggerThreadWrapper.hxx index 5b44db59ced..608f6a05f43 100644 --- a/Modules/Core/Common/include/itkLoggerThreadWrapper.hxx +++ b/Modules/Core/Common/include/itkLoggerThreadWrapper.hxx @@ -30,10 +30,9 @@ template void LoggerThreadWrapper::SetPriorityLevel(PriorityLevelEnum level) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_OperationQ.push(OperationEnum::SET_PRIORITY_LEVEL); this->m_LevelQ.push(level); - this->m_Mutex.unlock(); } /** Get the priority level for the current logger. Only messages that have @@ -43,9 +42,8 @@ template typename SimpleLoggerType::PriorityLevelEnum LoggerThreadWrapper::GetPriorityLevel() const { - this->m_Mutex.lock(); - PriorityLevelEnum level = this->m_PriorityLevel; - this->m_Mutex.unlock(); + const std::lock_guard lockGuard(m_Mutex); + PriorityLevelEnum level = this->m_PriorityLevel; return level; } @@ -53,20 +51,18 @@ template void LoggerThreadWrapper::SetLevelForFlushing(PriorityLevelEnum level) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_LevelForFlushing = level; this->m_OperationQ.push(OperationEnum::SET_LEVEL_FOR_FLUSHING); this->m_LevelQ.push(level); - this->m_Mutex.unlock(); } template typename SimpleLoggerType::PriorityLevelEnum LoggerThreadWrapper::GetLevelForFlushing() const { - this->m_Mutex.lock(); - PriorityLevelEnum level = this->m_LevelForFlushing; - this->m_Mutex.unlock(); + const std::lock_guard lockGuard(m_Mutex); + PriorityLevelEnum level = this->m_LevelForFlushing; return level; } @@ -74,18 +70,16 @@ template void LoggerThreadWrapper::SetDelay(DelayType delay) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_Delay = delay; - this->m_Mutex.unlock(); } template auto LoggerThreadWrapper::GetDelay() const -> DelayType { - this->m_Mutex.lock(); - DelayType delay = this->m_Delay; - this->m_Mutex.unlock(); + const std::lock_guard lockGuard(m_Mutex); + DelayType delay = this->m_Delay; return delay; } @@ -94,17 +88,16 @@ template void LoggerThreadWrapper::AddLogOutput(OutputType * output) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_OperationQ.push(OperationEnum::ADD_LOG_OUTPUT); this->m_OutputQ.push(output); - this->m_Mutex.unlock(); } template void LoggerThreadWrapper::Write(PriorityLevelEnum level, std::string const & content) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); if (this->m_PriorityLevel >= level) { this->m_OperationQ.push(OperationEnum::WRITE); @@ -115,7 +108,6 @@ LoggerThreadWrapper::Write(PriorityLevelEnum level, std::strin { this->PrivateFlush(); } - this->m_Mutex.unlock(); } template @@ -156,9 +148,8 @@ template void LoggerThreadWrapper::Flush() { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->PrivateFlush(); - this->m_Mutex.unlock(); } template @@ -186,36 +177,39 @@ LoggerThreadWrapper::ThreadFunction() { while (!m_TerminationRequested) { - m_Mutex.lock(); - while (!m_OperationQ.empty()) { - switch (m_OperationQ.front()) + const std::lock_guard lockGuard(m_Mutex); + while (!m_OperationQ.empty()) { - case OperationEnum::SET_PRIORITY_LEVEL: - this->m_PriorityLevel = m_LevelQ.front(); - m_LevelQ.pop(); - break; - - case OperationEnum::SET_LEVEL_FOR_FLUSHING: - this->m_LevelForFlushing = m_LevelQ.front(); - m_LevelQ.pop(); - break; - - case OperationEnum::ADD_LOG_OUTPUT: - this->m_Output->AddLogOutput(m_OutputQ.front()); - m_OutputQ.pop(); - break; - - case OperationEnum::WRITE: - SimpleLoggerType::Write(m_LevelQ.front(), m_MessageQ.front()); - m_LevelQ.pop(); - m_MessageQ.pop(); - break; + switch (m_OperationQ.front()) + { + case OperationEnum::SET_PRIORITY_LEVEL: + this->m_PriorityLevel = m_LevelQ.front(); + m_LevelQ.pop(); + break; + + case OperationEnum::SET_LEVEL_FOR_FLUSHING: + this->m_LevelForFlushing = m_LevelQ.front(); + m_LevelQ.pop(); + break; + + case OperationEnum::ADD_LOG_OUTPUT: + this->m_Output->AddLogOutput(m_OutputQ.front()); + m_OutputQ.pop(); + break; + + case OperationEnum::WRITE: + SimpleLoggerType::Write(m_LevelQ.front(), m_MessageQ.front()); + m_LevelQ.pop(); + m_MessageQ.pop(); + break; + } + m_OperationQ.pop(); } - m_OperationQ.pop(); - } - SimpleLoggerType::PrivateFlush(); - m_Mutex.unlock(); + SimpleLoggerType::PrivateFlush(); + + } // end of scope of lockGuard (unlocking m_Mutex automatically) + itksys::SystemTools::Delay(this->GetDelay()); } } diff --git a/Modules/Core/Common/src/itkStdStreamLogOutput.cxx b/Modules/Core/Common/src/itkStdStreamLogOutput.cxx index aeac12ff4ef..5969058c27e 100644 --- a/Modules/Core/Common/src/itkStdStreamLogOutput.cxx +++ b/Modules/Core/Common/src/itkStdStreamLogOutput.cxx @@ -47,48 +47,44 @@ StdStreamLogOutput::SetStream(StreamType & Stream) void StdStreamLogOutput::Flush() { - StdStreamLogOutput::m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); if (this->m_Stream) { this->m_Stream->flush(); } - StdStreamLogOutput::m_Mutex.unlock(); } /** Write to a buffer */ void StdStreamLogOutput::Write(double timestamp) { - StdStreamLogOutput::m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); if (this->m_Stream) { (*this->m_Stream) << timestamp; } - StdStreamLogOutput::m_Mutex.unlock(); } /** Write to a buffer */ void StdStreamLogOutput::Write(std::string const & content) { - StdStreamLogOutput::m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); if (this->m_Stream) { (*this->m_Stream) << content; } - StdStreamLogOutput::m_Mutex.unlock(); } /** Write to a buffer */ void StdStreamLogOutput::Write(std::string const & content, double timestamp) { - StdStreamLogOutput::m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); if (this->m_Stream) { (*this->m_Stream) << timestamp << " : " << content; } - StdStreamLogOutput::m_Mutex.unlock(); } void diff --git a/Modules/Core/Common/src/itkThreadLogger.cxx b/Modules/Core/Common/src/itkThreadLogger.cxx index 4d00b0fb7dd..dfc2d5d9135 100644 --- a/Modules/Core/Common/src/itkThreadLogger.cxx +++ b/Modules/Core/Common/src/itkThreadLogger.cxx @@ -41,70 +41,63 @@ ThreadLogger::~ThreadLogger() void ThreadLogger::SetPriorityLevel(PriorityLevelEnum level) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_OperationQ.push(SET_PRIORITY_LEVEL); this->m_LevelQ.push(level); - this->m_Mutex.unlock(); } Logger::PriorityLevelEnum ThreadLogger::GetPriorityLevel() const { - this->m_Mutex.lock(); - PriorityLevelEnum level = this->m_PriorityLevel; - this->m_Mutex.unlock(); + const std::lock_guard lockGuard(m_Mutex); + PriorityLevelEnum level = this->m_PriorityLevel; return level; } void ThreadLogger::SetLevelForFlushing(PriorityLevelEnum level) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_LevelForFlushing = level; this->m_OperationQ.push(SET_LEVEL_FOR_FLUSHING); this->m_LevelQ.push(level); - this->m_Mutex.unlock(); } Logger::PriorityLevelEnum ThreadLogger::GetLevelForFlushing() const { - this->m_Mutex.lock(); - PriorityLevelEnum level = this->m_LevelForFlushing; - this->m_Mutex.unlock(); + const std::lock_guard lockGuard(m_Mutex); + PriorityLevelEnum level = this->m_LevelForFlushing; return level; } void ThreadLogger::SetDelay(DelayType delay) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_Delay = delay; - this->m_Mutex.unlock(); } ThreadLogger::DelayType ThreadLogger::GetDelay() const { - this->m_Mutex.lock(); - DelayType delay = this->m_Delay; - this->m_Mutex.unlock(); + const std::lock_guard lockGuard(m_Mutex); + DelayType delay = this->m_Delay; return delay; } void ThreadLogger::AddLogOutput(OutputType * output) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_OperationQ.push(ADD_LOG_OUTPUT); this->m_OutputQ.push(output); - this->m_Mutex.unlock(); } void ThreadLogger::Write(PriorityLevelEnum level, std::string const & content) { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_OperationQ.push(WRITE); this->m_MessageQ.push(content); this->m_LevelQ.push(level); @@ -112,16 +105,14 @@ ThreadLogger::Write(PriorityLevelEnum level, std::string const & content) { this->InternalFlush(); } - this->m_Mutex.unlock(); } void ThreadLogger::Flush() { - this->m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); this->m_OperationQ.push(FLUSH); this->InternalFlush(); - this->m_Mutex.unlock(); } void @@ -167,38 +158,40 @@ ThreadLogger::ThreadFunction() { while (!m_TerminationRequested) { - m_Mutex.lock(); - while (!m_OperationQ.empty()) { - switch (m_OperationQ.front()) + const std::lock_guard lockGuard(m_Mutex); + while (!m_OperationQ.empty()) { - case ThreadLogger::SET_PRIORITY_LEVEL: - m_PriorityLevel = m_LevelQ.front(); - m_LevelQ.pop(); - break; - - case ThreadLogger::SET_LEVEL_FOR_FLUSHING: - m_LevelForFlushing = m_LevelQ.front(); - m_LevelQ.pop(); - break; - - case ThreadLogger::ADD_LOG_OUTPUT: - m_Output->AddLogOutput(m_OutputQ.front()); - m_OutputQ.pop(); - break; - - case ThreadLogger::WRITE: - Logger::Write(m_LevelQ.front(), m_MessageQ.front()); - m_LevelQ.pop(); - m_MessageQ.pop(); - break; - case ThreadLogger::FLUSH: - Logger::Flush(); - break; + switch (m_OperationQ.front()) + { + case ThreadLogger::SET_PRIORITY_LEVEL: + m_PriorityLevel = m_LevelQ.front(); + m_LevelQ.pop(); + break; + + case ThreadLogger::SET_LEVEL_FOR_FLUSHING: + m_LevelForFlushing = m_LevelQ.front(); + m_LevelQ.pop(); + break; + + case ThreadLogger::ADD_LOG_OUTPUT: + m_Output->AddLogOutput(m_OutputQ.front()); + m_OutputQ.pop(); + break; + + case ThreadLogger::WRITE: + Logger::Write(m_LevelQ.front(), m_MessageQ.front()); + m_LevelQ.pop(); + m_MessageQ.pop(); + break; + case ThreadLogger::FLUSH: + Logger::Flush(); + break; + } + m_OperationQ.pop(); } - m_OperationQ.pop(); - } - m_Mutex.unlock(); + } // end of scope of lockGuard (unlocking m_Mutex automatically) + itksys::SystemTools::Delay(this->GetDelay()); } } diff --git a/Modules/Core/GPUCommon/include/itkGPUImageDataManager.hxx b/Modules/Core/GPUCommon/include/itkGPUImageDataManager.hxx index 9ae121d9145..a681e889144 100644 --- a/Modules/Core/GPUCommon/include/itkGPUImageDataManager.hxx +++ b/Modules/Core/GPUCommon/include/itkGPUImageDataManager.hxx @@ -63,7 +63,7 @@ GPUImageDataManager::MakeCPUBufferUpToDate() { if (m_Image.IsNotNull()) { - m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); ModifiedTimeType gpu_time = this->GetMTime(); TimeStamp cpu_time_stamp = m_Image->GetTimeStamp(); @@ -96,8 +96,6 @@ GPUImageDataManager::MakeCPUBufferUpToDate() m_IsCPUBufferDirty = false; m_IsGPUBufferDirty = false; } - - m_Mutex.unlock(); } } @@ -107,7 +105,7 @@ GPUImageDataManager::MakeGPUBufferUpToDate() { if (m_Image.IsNotNull()) { - m_Mutex.lock(); + const std::lock_guard lockGuard(m_Mutex); ModifiedTimeType gpu_time = this->GetMTime(); TimeStamp cpu_time_stamp = m_Image->GetTimeStamp(); @@ -139,8 +137,6 @@ GPUImageDataManager::MakeGPUBufferUpToDate() m_IsCPUBufferDirty = false; m_IsGPUBufferDirty = false; } - - m_Mutex.unlock(); } }