tsan: fix data race in MemoryTracker::setDescription and `MemoryTra…

…cker::logPeakMemoryUsage` (#7368) close #7367
pingcap · Apr 25, 2023 · 1591442 · 1591442
1 parent bd3d697
commit 1591442
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 8 deletions.
diff --git a/dbms/src/Common/MemoryTracker.cpp b/dbms/src/Common/MemoryTracker.cpp
@@ -67,7 +67,8 @@ static Poco::Logger * getLogger()
 
 void MemoryTracker::logPeakMemoryUsage() const
 {
-    LOG_DEBUG(getLogger(), "Peak memory usage{}: {}.", (description ? " " + std::string(description) : ""), formatReadableSizeWithBinarySuffix(peak));
+    const char * tmp_decr = description.load();
+    LOG_DEBUG(getLogger(), "Peak memory usage{}: {}.", (tmp_decr ? " " + std::string(tmp_decr) : ""), formatReadableSizeWithBinarySuffix(peak));
 }
 
 void MemoryTracker::alloc(Int64 size, bool check_memory_limit)
@@ -89,8 +90,9 @@ void MemoryTracker::alloc(Int64 size, bool check_memory_limit)
         {
             DB::FmtBuffer fmt_buf;
             fmt_buf.append("Memory tracker accuracy ");
-            if (description)
-                fmt_buf.fmtAppend(" {}", description);
+            const char * tmp_decr = description.load();
+            if (tmp_decr)
+                fmt_buf.fmtAppend(" {}", tmp_decr);
 
             fmt_buf.fmtAppend(": fault injected. real_rss ({}) is much larger than limit ({}). Debug info, threads of process: {}, memory usage tracked by ProcessList: peak {}, current {}, memory usage not tracked by ProcessList: peak {}, current {} . Virtual memory size: {}",
                               formatReadableSizeWithBinarySuffix(real_rss),
@@ -112,8 +114,9 @@ void MemoryTracker::alloc(Int64 size, bool check_memory_limit)
 
             DB::FmtBuffer fmt_buf;
             fmt_buf.append("Memory tracker");
-            if (description)
-                fmt_buf.fmtAppend(" {}", description);
+            const char * tmp_decr = description.load();
+            if (tmp_decr)
+                fmt_buf.fmtAppend(" {}", tmp_decr);
             fmt_buf.fmtAppend(": fault injected. Would use {} (attempt to allocate chunk of {} bytes), maximum: {}",
                               formatReadableSizeWithBinarySuffix(will_be),
                               size,
@@ -133,8 +136,9 @@ void MemoryTracker::alloc(Int64 size, bool check_memory_limit)
 
             DB::FmtBuffer fmt_buf;
             fmt_buf.append("Memory limit");
-            if (description)
-                fmt_buf.fmtAppend(" {}", description);
+            const char * tmp_decr = description.load();
+            if (tmp_decr)
+                fmt_buf.fmtAppend(" {}", tmp_decr);
 
             if (!is_rss_too_large)
             { // out of memory quota

diff --git a/dbms/src/Common/MemoryTracker.h b/dbms/src/Common/MemoryTracker.h
@@ -59,7 +59,7 @@ class MemoryTracker : public std::enable_shared_from_this<MemoryTracker>
     CurrentMetrics::Metric metric = CurrentMetrics::MemoryTracking;
 
     /// This description will be used as prefix into log messages (if isn't nullptr)
-    const char * description = nullptr;
+    std::atomic<const char *> description = nullptr;
 
     /// Make constructors private to ensure all objects of this class is created by `MemoryTracker::create`.
     MemoryTracker() = default;