Refactor locking in AsynchronousMetrics

src/Common/AsynchronousMetrics.cpp

@@ -98,7 +98,7 @@ AsynchronousMetrics::AsynchronousMetrics(
 }
 
 #if defined(OS_LINUX)
-void AsynchronousMetrics::openSensors()
+void AsynchronousMetrics::openSensors() TSA_REQUIRES(data_mutex)
 {
     LOG_TRACE(log, "Scanning /sys/class/thermal");
 
@@ -136,7 +136,7 @@ void AsynchronousMetrics::openSensors()
     }
 }
 
-void AsynchronousMetrics::openBlockDevices()
+void AsynchronousMetrics::openBlockDevices() TSA_REQUIRES(data_mutex)
 {
     LOG_TRACE(log, "Scanning /sys/block");
 
@@ -163,7 +163,7 @@ void AsynchronousMetrics::openBlockDevices()
     }
 }
 
-void AsynchronousMetrics::openEDAC()
+void AsynchronousMetrics::openEDAC() TSA_REQUIRES(data_mutex)
 {
     LOG_TRACE(log, "Scanning /sys/devices/system/edac");
 
@@ -194,7 +194,7 @@ void AsynchronousMetrics::openEDAC()
     }
 }
 
-void AsynchronousMetrics::openSensorsChips()
+void AsynchronousMetrics::openSensorsChips() TSA_REQUIRES(data_mutex)
 {
     LOG_TRACE(log, "Scanning /sys/class/hwmon");
 
@@ -306,7 +306,7 @@ AsynchronousMetrics::~AsynchronousMetrics()
 
 AsynchronousMetricValues AsynchronousMetrics::getValues() const
 {
-    std::lock_guard lock(thread_mutex);
+    std::lock_guard lock(data_mutex);
     return values;
 }
 
@@ -563,6 +563,8 @@ void AsynchronousMetrics::update(TimePoint update_time)
 
     AsynchronousMetricValues new_values;
 
+    std::lock_guard lock(data_mutex);
+
     auto current_time = std::chrono::system_clock::now();
     auto time_since_previous_update = current_time - previous_update_time;
     previous_update_time = update_time;
@@ -1591,7 +1593,6 @@ void AsynchronousMetrics::update(TimePoint update_time)
     first_run = false;
 
     // Finally, update the current metrics.
-    std::lock_guard lock(thread_mutex);
     values = new_values;
 }
 

src/Common/AsynchronousMetrics.h

@@ -85,46 +85,51 @@ class AsynchronousMetrics
 
     ProtocolServerMetricsFunc protocol_server_metrics_func;
 
+    std::unique_ptr<ThreadFromGlobalPool> thread;
+
     mutable std::mutex thread_mutex;
     std::condition_variable wait_cond;
     bool quit TSA_GUARDED_BY(thread_mutex) = false;
-    AsynchronousMetricValues values TSA_GUARDED_BY(thread_mutex);
+
+    mutable std::mutex data_mutex;
 
     /// Some values are incremental and we have to calculate the difference.
     /// On first run we will only collect the values to subtract later.
-    bool first_run = true;
-    TimePoint previous_update_time;
+    bool first_run TSA_GUARDED_BY(data_mutex) = true;
+    TimePoint previous_update_time TSA_GUARDED_BY(data_mutex);
+
+    AsynchronousMetricValues values TSA_GUARDED_BY(data_mutex);
 
 #if defined(OS_LINUX) || defined(OS_FREEBSD)
-    MemoryStatisticsOS memory_stat;
+    MemoryStatisticsOS memory_stat TSA_GUARDED_BY(data_mutex);
 #endif
 
 #if defined(OS_LINUX)
-    std::optional<ReadBufferFromFilePRead> meminfo;
-    std::optional<ReadBufferFromFilePRead> loadavg;
-    std::optional<ReadBufferFromFilePRead> proc_stat;
-    std::optional<ReadBufferFromFilePRead> cpuinfo;
-    std::optional<ReadBufferFromFilePRead> file_nr;
-    std::optional<ReadBufferFromFilePRead> uptime;
-    std::optional<ReadBufferFromFilePRead> net_dev;
-
-    std::optional<ReadBufferFromFilePRead> cgroupmem_limit_in_bytes;
-    std::optional<ReadBufferFromFilePRead> cgroupmem_usage_in_bytes;
-    std::optional<ReadBufferFromFilePRead> cgroupcpu_cfs_period;
-    std::optional<ReadBufferFromFilePRead> cgroupcpu_cfs_quota;
-    std::optional<ReadBufferFromFilePRead> cgroupcpu_max;
-
-    std::vector<std::unique_ptr<ReadBufferFromFilePRead>> thermal;
+    std::optional<ReadBufferFromFilePRead> meminfo TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> loadavg TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> proc_stat TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> cpuinfo TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> file_nr TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> uptime TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> net_dev TSA_GUARDED_BY(data_mutex);
+
+    std::optional<ReadBufferFromFilePRead> cgroupmem_limit_in_bytes TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> cgroupmem_usage_in_bytes TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> cgroupcpu_cfs_period TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> cgroupcpu_cfs_quota TSA_GUARDED_BY(data_mutex);
+    std::optional<ReadBufferFromFilePRead> cgroupcpu_max TSA_GUARDED_BY(data_mutex);
+
+    std::vector<std::unique_ptr<ReadBufferFromFilePRead>> thermal TSA_GUARDED_BY(data_mutex);
 
     std::unordered_map<String /* device name */,
         std::unordered_map<String /* label name */,
-            std::unique_ptr<ReadBufferFromFilePRead>>> hwmon_devices;
+            std::unique_ptr<ReadBufferFromFilePRead>>> hwmon_devices TSA_GUARDED_BY(data_mutex);
 
     std::vector<std::pair<
         std::unique_ptr<ReadBufferFromFilePRead> /* correctable errors */,
-        std::unique_ptr<ReadBufferFromFilePRead> /* uncorrectable errors */>> edac;
+        std::unique_ptr<ReadBufferFromFilePRead> /* uncorrectable errors */>> edac TSA_GUARDED_BY(data_mutex);
 
-    std::unordered_map<String /* device name */, std::unique_ptr<ReadBufferFromFilePRead>> block_devs;
+    std::unordered_map<String /* device name */, std::unique_ptr<ReadBufferFromFilePRead>> block_devs TSA_GUARDED_BY(data_mutex);
 
     /// TODO: socket statistics.
 
@@ -154,9 +159,9 @@ class AsynchronousMetrics
         ProcStatValuesOther operator-(const ProcStatValuesOther & other) const;
     };
 
-    ProcStatValuesCPU proc_stat_values_all_cpus{};
-    ProcStatValuesOther proc_stat_values_other{};
-    std::vector<ProcStatValuesCPU> proc_stat_values_per_cpu;
+    ProcStatValuesCPU proc_stat_values_all_cpus TSA_GUARDED_BY(data_mutex) {};
+    ProcStatValuesOther proc_stat_values_other TSA_GUARDED_BY(data_mutex) {};
+    std::vector<ProcStatValuesCPU> proc_stat_values_per_cpu TSA_GUARDED_BY(data_mutex);
 
     /// https://www.kernel.org/doc/Documentation/block/stat.txt
     struct BlockDeviceStatValues
@@ -181,7 +186,7 @@ class AsynchronousMetrics
         BlockDeviceStatValues operator-(const BlockDeviceStatValues & other) const;
     };
 
-    std::unordered_map<String /* device name */, BlockDeviceStatValues> block_device_stats;
+    std::unordered_map<String /* device name */, BlockDeviceStatValues> block_device_stats TSA_GUARDED_BY(data_mutex);
 
     struct NetworkInterfaceStatValues
     {
@@ -197,18 +202,16 @@ class AsynchronousMetrics
         NetworkInterfaceStatValues operator-(const NetworkInterfaceStatValues & other) const;
     };
 
-    std::unordered_map<String /* device name */, NetworkInterfaceStatValues> network_interface_stats;
+    std::unordered_map<String /* device name */, NetworkInterfaceStatValues> network_interface_stats TSA_GUARDED_BY(data_mutex);
 
-    Stopwatch block_devices_rescan_delay;
+    Stopwatch block_devices_rescan_delay TSA_GUARDED_BY(data_mutex);
 
     void openSensors();
     void openBlockDevices();
     void openSensorsChips();
     void openEDAC();
 #endif
 
-    std::unique_ptr<ThreadFromGlobalPool> thread;
-
     void run();
     void update(TimePoint update_time);
 };