Skip to content

Commit

Permalink
Use reader writer lock guard for CRT logger instance pointer access
Browse files Browse the repository at this point in the history
  • Loading branch information
SergeyRyabinin committed Jul 27, 2023
1 parent 73746ee commit dd29cdd
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/aws-cpp-sdk-core/source/Aws.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,13 @@ namespace Aws

Aws::Config::CleanupConfigAndCredentialsCacheManager();

Aws::Client::CoreErrorsMapper::CleanupCoreErrorsMapper();
Aws::CleanupCrt();
if (options.loggingOptions.logLevel != Aws::Utils::Logging::LogLevel::Off)
{
Aws::Utils::Logging::ShutdownCRTLogging();
Aws::Utils::Logging::ShutdownAWSLogging();
}
Aws::Client::CoreErrorsMapper::CleanupCoreErrorsMapper();
Aws::CleanupCrt();
#ifdef USE_AWS_MEMORY_MANAGEMENT
if(options.memoryManagementOptions.memoryManager)
{
Expand Down
36 changes: 18 additions & 18 deletions src/aws-cpp-sdk-core/source/utils/logging/CRTLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#include <aws/core/utils/logging/CRTLogging.h>
#include <aws/core/utils/logging/CRTLogSystem.h>
#include <aws/core/utils/logging/LogLevel.h>
#include <aws/core/utils/threading/ReaderWriterLock.h>
#include <aws/common/logging.h>

#include <memory>
#include <mutex>
#include <assert.h>
Expand All @@ -22,6 +24,7 @@ namespace Utils
namespace Logging {

static std::shared_ptr<CRTLogSystemInterface> CRTLogSystem(nullptr);
static Aws::Utils::Threading::ReaderWriterLock CRTLogSystemLock;

static bool s_CRTLogsRedirectionIsSet(false);
static aws_logger s_sdkCrtLogger;
Expand All @@ -34,16 +37,15 @@ static int s_aws_logger_redirect_log(
const char *format, ...)
{
AWS_UNREFERENCED_PARAM(logger);
// use local variable because global variable can be modified in the middle by another thread.
std::shared_ptr<CRTLogSystemInterface> localCRTLogSystem = CRTLogSystem;
if (localCRTLogSystem)
Aws::Utils::Threading::ReaderLockGuard guard(CRTLogSystemLock);
if (CRTLogSystem)
{
assert(logger->p_impl == &s_sdkCrtLogger);
Aws::Utils::Logging::LogLevel logLevel = static_cast<LogLevel>(log_level);
const char* subjectName = aws_log_subject_name(subject);
va_list args;
va_start(args, format);
localCRTLogSystem->Log(logLevel, subjectName, format, args);
CRTLogSystem->Log(logLevel, subjectName, format, args);
va_end(args);
return AWS_OP_SUCCESS;
}
Expand All @@ -64,11 +66,11 @@ static int s_aws_logger_redirect_log(
static enum aws_log_level s_aws_logger_redirect_get_log_level(struct aws_logger *logger, aws_log_subject_t subject)
{
AWS_UNREFERENCED_PARAM(logger);
std::shared_ptr<CRTLogSystemInterface> localCRTLogSystem = CRTLogSystem;
if (localCRTLogSystem)
Aws::Utils::Threading::ReaderLockGuard guard(CRTLogSystemLock);
if (CRTLogSystem)
{
assert(logger->p_impl == &s_sdkCrtLogger);
return (aws_log_level) (localCRTLogSystem->GetLogLevel());
return (aws_log_level) (CRTLogSystem->GetLogLevel());
}
else if (s_CRTLogsRedirectionIsSet)
{
Expand All @@ -82,11 +84,11 @@ static enum aws_log_level s_aws_logger_redirect_get_log_level(struct aws_logger
static void s_aws_logger_redirect_clean_up(struct aws_logger *logger)
{
AWS_UNREFERENCED_PARAM(logger);
std::shared_ptr<CRTLogSystemInterface> localCRTLogSystem = CRTLogSystem;
if (localCRTLogSystem)
Aws::Utils::Threading::ReaderLockGuard guard(CRTLogSystemLock);
if (CRTLogSystem)
{
assert(logger->p_impl == &s_sdkCrtLogger);
return localCRTLogSystem->CleanUp();
return CRTLogSystem->CleanUp();
}
else if (s_CRTLogsRedirectionIsSet)
{
Expand All @@ -99,11 +101,11 @@ static void s_aws_logger_redirect_clean_up(struct aws_logger *logger)
static int s_aws_logger_redirect_set_log_level(struct aws_logger *logger, enum aws_log_level log_level)
{
AWS_UNREFERENCED_PARAM(logger);
std::shared_ptr<CRTLogSystemInterface> localCRTLogSystem = CRTLogSystem;
if (localCRTLogSystem)
Aws::Utils::Threading::ReaderLockGuard guard(CRTLogSystemLock);
if (CRTLogSystem)
{
assert(logger->p_impl == &s_sdkCrtLogger);
localCRTLogSystem->SetLogLevel(static_cast<LogLevel>(log_level));
CRTLogSystem->SetLogLevel(static_cast<LogLevel>(log_level));
return AWS_OP_SUCCESS;
}
else if (s_CRTLogsRedirectionIsSet)
Expand Down Expand Up @@ -151,16 +153,14 @@ void SetUpCrtLogsRedirection()
}

void InitializeCRTLogging(const std::shared_ptr<CRTLogSystemInterface>& inputCrtLogSystem) {
Aws::Utils::Threading::WriterLockGuard g(CRTLogSystemLock);
SetUpCrtLogsRedirection();
// stop using CRTLogSystem to avoid race condition and lost logs in ~CRTLogSystemInterface
std::shared_ptr<CRTLogSystemInterface> tmpCRTLogSystem = std::move(CRTLogSystem);
CRTLogSystem = inputCrtLogSystem;
}

void ShutdownCRTLogging() {
// stop using CRTLogSystem to avoid race condition and lost logs in ~CRTLogSystemInterface
std::shared_ptr<CRTLogSystemInterface> tmpCRTLogSystem = std::move(CRTLogSystem);
tmpCRTLogSystem.reset(); // just being explicit
Aws::Utils::Threading::WriterLockGuard g(CRTLogSystemLock);
CRTLogSystem.reset();
}

} // namespace Logging
Expand Down

0 comments on commit dd29cdd

Please sign in to comment.