From e91263edb9ece1c4cdb2ad83cb93c38ac60bde85 Mon Sep 17 00:00:00 2001
From: Changyu Bi <changyubi@meta.com>
Date: Thu, 14 Mar 2024 14:28:33 -0700
Subject: [PATCH] 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
---
 logging/auto_roll_logger.cc | 16 +++++++---------
 logging/auto_roll_logger.h  |  7 +++----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/logging/auto_roll_logger.cc b/logging/auto_roll_logger.cc
index 9e9ad45aee..c186ab44a9 100644
--- a/logging/auto_roll_logger.cc
+++ b/logging/auto_roll_logger.cc
@@ -192,14 +192,13 @@ void AutoRollLogger::LogInternal(const char* format, ...) {
 }
 
 void AutoRollLogger::Logv(const char* format, va_list ap) {
-  assert(GetStatus().ok());
-  if (!logger_) {
-    return;
-  }
-
   std::shared_ptr<Logger> logger;
   {
     MutexLock l(&mutex_);
+    assert(GetStatus().ok());
+    if (!logger_) {
+      return;
+    }
     if ((kLogFileTimeToRoll > 0 && LogExpired()) ||
         (kMaxLogFileSize > 0 && logger_->GetLogFileSize() >= kMaxLogFileSize)) {
       RollLogFile();
@@ -240,10 +239,6 @@ void AutoRollLogger::WriteHeaderInfo() {
 }
 
 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
   // assumptions about the data contained in va_list, we will retain them as
   // strings
@@ -253,6 +248,9 @@ void AutoRollLogger::LogHeader(const char* format, va_list args) {
   va_end(tmp);
 
   MutexLock l(&mutex_);
+  if (!logger_) {
+    return;
+  }
   headers_.push_back(data);
 
   // Log the original message to the current log
diff --git a/logging/auto_roll_logger.h b/logging/auto_roll_logger.h
index 9cc2ee0df4..be0b14051a 100644
--- a/logging/auto_roll_logger.h
+++ b/logging/auto_roll_logger.h
@@ -42,13 +42,12 @@ class AutoRollLogger : public Logger {
   Status GetStatus() { return status_; }
 
   size_t GetLogFileSize() const override {
-    if (!logger_) {
-      return 0;
-    }
-
     std::shared_ptr<Logger> logger;
     {
       MutexLock l(&mutex_);
+      if (!logger_) {
+        return 0;
+      }
       // pin down the current logger_ instance before releasing the mutex.
       logger = logger_;
     }