From 39be52418df4f082d0a8294bc931b022ec59ccb0 Mon Sep 17 00:00:00 2001
From: "Zezheng.Li" <lizezheng.lzz@alibaba-inc.com>
Date: Sun, 26 Jan 2025 17:45:46 +0800
Subject: [PATCH] fix data race in metric

---
 include/ylt/metric/counter.hpp       | 10 +++++-----
 include/ylt/metric/summary_impl.hpp  |  2 +-
 include/ylt/metric/system_metric.hpp |  3 ++-
 src/metric/benchmark/bench.hpp       |  6 ++----
 src/metric/tests/test_metric.cpp     |  1 -
 5 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/include/ylt/metric/counter.hpp b/include/ylt/metric/counter.hpp
index e676f9401..d77a2a912 100644
--- a/include/ylt/metric/counter.hpp
+++ b/include/ylt/metric/counter.hpp
@@ -58,8 +58,8 @@ class basic_static_counter : public static_metric {
   }
 
   value_type update(value_type value) {
-    if (!has_change_) [[unlikely]] {
-      has_change_ = true;
+    if (!has_change_.load(std::memory_order::relaxed)) [[unlikely]] {
+      has_change_.store(true, std::memory_order::relaxed);
     }
     return default_label_value_.update(value);
   }
@@ -70,7 +70,7 @@ class basic_static_counter : public static_metric {
 
   void serialize(std::string &str) override {
     auto value = default_label_value_.value();
-    if (value == 0 && !has_change_) {
+    if (value == 0 && !has_change_.load(std::memory_order::relaxed)) {
       return;
     }
 
@@ -81,7 +81,7 @@ class basic_static_counter : public static_metric {
 #ifdef CINATRA_ENABLE_METRIC_JSON
   void serialize_to_json(std::string &str) override {
     auto value = default_label_value_.value();
-    if (value == 0 && !has_change_) {
+    if (value == 0 && !has_change_.load(std::memory_order::relaxed)) {
       return;
     }
 
@@ -126,7 +126,7 @@ class basic_static_counter : public static_metric {
     str.pop_back();
   }
 
-  bool has_change_ = false;
+  std::atomic<bool> has_change_ = false;
   uint32_t dupli_count_;
   thread_local_value<value_type> default_label_value_;
 };
diff --git a/include/ylt/metric/summary_impl.hpp b/include/ylt/metric/summary_impl.hpp
index 54292368f..3cc8e29b4 100644
--- a/include/ylt/metric/summary_impl.hpp
+++ b/include/ylt/metric/summary_impl.hpp
@@ -131,7 +131,7 @@ class summary_impl {
     template <bool inc_order>
     void stat_impl(uint64_t& count,
                    std::vector<std::pair<int16_t, uint_type>>& result, int i) {
-      auto piece = arr[i].load(std::memory_order_relaxed);
+      auto piece = arr[i].load(std::memory_order_acquire);
       if (piece) {
         if constexpr (inc_order) {
           for (int j = 0; j < piece->size(); ++j) {
diff --git a/include/ylt/metric/system_metric.hpp b/include/ylt/metric/system_metric.hpp
index dd786a9d0..79b4268a1 100644
--- a/include/ylt/metric/system_metric.hpp
+++ b/include/ylt/metric/system_metric.hpp
@@ -381,13 +381,14 @@ inline void start_stat(std::weak_ptr<coro_io::period_timer> weak) {
     return;
   }
 
+  ylt_stat();
+
   timer->expires_after(std::chrono::seconds(1));
   timer->async_wait([timer](std::error_code ec) {
     if (ec) {
       return;
     }
 
-    ylt_stat();
     start_stat(timer);
   });
 }
diff --git a/src/metric/benchmark/bench.hpp b/src/metric/benchmark/bench.hpp
index 4306bc673..03ed878f1 100644
--- a/src/metric/benchmark/bench.hpp
+++ b/src/metric/benchmark/bench.hpp
@@ -134,8 +134,7 @@ inline void bench_dynamic_counter_mixed_with_delete(
                                           {"a", "b"});
   bench_mixed_impl(
       counter,
-      [&, i = 0]() mutable {
-        ++i;
+      [&]() mutable {
         std::array<std::string, 2> label = {
             "123e4567-e89b-12d3-a456-426614174000",
             std::to_string(get_random(max_cnt))};
@@ -153,8 +152,7 @@ inline void bench_dynamic_counter_mixed(size_t thd_num,
                                           {"a", "b"});
   bench_mixed_impl(
       counter,
-      [&, i = 0]() mutable {
-        ++i;
+      [&]() mutable {
         counter.inc({"123e4567-e89b-12d3-a456-426614174000",
                      std::to_string(get_random(max_cnt))},
                     1);
diff --git a/src/metric/tests/test_metric.cpp b/src/metric/tests/test_metric.cpp
index d0a7560c1..f9d0e74fd 100644
--- a/src/metric/tests/test_metric.cpp
+++ b/src/metric/tests/test_metric.cpp
@@ -1767,7 +1767,6 @@ TEST_CASE("test serialize with multiple threads") {
 #if defined(__GNUC__)
 TEST_CASE("test system metric") {
   start_system_metric();
-  metric::detail::ylt_stat();
 
   auto s = system_metric_manager::instance().serialize_static();
   std::cout << s;