[metric] fix data race (#889)

fix data race in metric
alibaba · Jan 27, 2025 · 46abbb3 · 46abbb3
1 parent 9d4dfb5
commit 46abbb3
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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;