dolysis
/
rocksdb
mirror of https://github.com/facebook/rocksdb.git
3
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix data race in AutoRollLogger (#12436) 

Summary:
`logger_` can be destructed in `ResetLogger()` so we should access them under `mutex_`. Similarly `status_` can be updated only under `mutex_` or in constructor.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/12436

Test Plan: I tried running tsan crash test with log_file_time_to_roll = 2, but not able to repro yet. Will monitor internal crash tests.

Reviewed By: hx235

Differential Revision: D54916371

Pulled By: cbi42

fbshipit-source-id: 4a3e3b40fbc2ae242598afdbd4bed5fb8ccf8d65
This commit is contained in:
Changyu Bi 2024-03-14 14:28:33 -07:00 committed by Facebook GitHub Bot
parent dd24bda137
commit e91263edb9
2 changed files with 10 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
logging/auto_roll_logger.cc
View File

@ -192,14 +192,13 @@ void AutoRollLogger::LogInternal(const char* format, ...) {
} }
void AutoRollLogger::Logv(const char* format, va_list ap) { void AutoRollLogger::Logv(const char* format, va_list ap) {
assert(GetStatus().ok());
if (!logger_) {
return;
}
std::shared_ptr<Logger> logger; std::shared_ptr<Logger> logger;
{ {
MutexLock l(&mutex_); MutexLock l(&mutex_);
assert(GetStatus().ok());
if (!logger_) {
return;
}
if ((kLogFileTimeToRoll > 0 && LogExpired()) || if ((kLogFileTimeToRoll > 0 && LogExpired()) ||
(kMaxLogFileSize > 0 && logger_->GetLogFileSize() >= kMaxLogFileSize)) { (kMaxLogFileSize > 0 && logger_->GetLogFileSize() >= kMaxLogFileSize)) {
RollLogFile(); RollLogFile();
@ -240,10 +239,6 @@ void AutoRollLogger::WriteHeaderInfo() {
} }
void AutoRollLogger::LogHeader(const char* format, va_list args) { void AutoRollLogger::LogHeader(const char* format, va_list args) {
if (!logger_) {
return;
}
// header message are to be retained in memory. Since we cannot make any // header message are to be retained in memory. Since we cannot make any
// assumptions about the data contained in va_list, we will retain them as // assumptions about the data contained in va_list, we will retain them as
// strings // strings
@ -253,6 +248,9 @@ void AutoRollLogger::LogHeader(const char* format, va_list args) {
va_end(tmp); va_end(tmp);
MutexLock l(&mutex_); MutexLock l(&mutex_);
if (!logger_) {
return;
}
headers_.push_back(data); headers_.push_back(data);
// Log the original message to the current log // Log the original message to the current log

7
logging/auto_roll_logger.h
View File

@ -42,13 +42,12 @@ class AutoRollLogger : public Logger {
Status GetStatus() { return status_; } Status GetStatus() { return status_; }
size_t GetLogFileSize() const override { size_t GetLogFileSize() const override {
if (!logger_) {
return 0;
}
std::shared_ptr<Logger> logger; std::shared_ptr<Logger> logger;
{ {
MutexLock l(&mutex_); MutexLock l(&mutex_);
if (!logger_) {
return 0;
}
// pin down the current logger_ instance before releasing the mutex. // pin down the current logger_ instance before releasing the mutex.
logger = logger_; logger = logger_;
} }