Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use reader writer lock guard for CRT logger instance pointer access #2552

Merged
merged 1 commit into from
Jul 28, 2023
Merged

Use reader writer lock guard for CRT logger instance pointer access #2552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/aws-cpp-sdk-core/source/Aws.cpp
View file
Open in desktop
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
View file
Open in desktop
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
Loading