From ff5fe5df14a818910ce4e4b4eebf58798b695723 Mon Sep 17 00:00:00 2001
From: Andre Puschmann <andre@softwareradiosystems.com>
Date: Fri, 2 Jul 2021 22:00:32 +0200
Subject: [PATCH] rf_uhd_imp: fix race in RF metrics

---
 lib/include/srsran/radio/radio.h |  1 +
 lib/src/phy/rf/rf_imp.c          |  8 ++++----
 lib/src/phy/rf/rf_uhd_imp.cc     |  2 ++
 lib/src/radio/radio.cc           | 14 ++++++++++----
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/include/srsran/radio/radio.h b/lib/include/srsran/radio/radio.h
index 0b9f7e07c..0c206f1d6 100644
--- a/lib/include/srsran/radio/radio.h
+++ b/lib/include/srsran/radio/radio.h
@@ -90,6 +90,7 @@ private:
   std::vector<srsran_rf_info_t>                           rf_info     = {};
   std::vector<int32_t>                                    rx_offset_n = {};
   rf_metrics_t                                            rf_metrics  = {};
+  std::mutex                                              metrics_mutex;
   srslog::basic_logger&                                   logger      = srslog::fetch_basic_logger("RF", false);
   phy_interface_radio*                                    phy         = nullptr;
   cf_t*                                                   zeros       = nullptr;
diff --git a/lib/src/phy/rf/rf_imp.c b/lib/src/phy/rf/rf_imp.c
index 6da4fa88d..9a6d25966 100644
--- a/lib/src/phy/rf/rf_imp.c
+++ b/lib/src/phy/rf/rf_imp.c
@@ -174,13 +174,13 @@ int srsran_rf_open_multi(srsran_rf_t* h, char* args, uint32_t nof_channels)
 int srsran_rf_close(srsran_rf_t* rf)
 {
   // Stop gain thread
+  pthread_mutex_lock(&rf->mutex);
   if (rf->thread_gain_run) {
-    pthread_mutex_lock(&rf->mutex);
     rf->thread_gain_run = false;
-    pthread_mutex_unlock(&rf->mutex);
-    pthread_cond_signal(&rf->cond);
-    pthread_join(rf->thread_gain, NULL);
   }
+  pthread_mutex_unlock(&rf->mutex);
+  pthread_cond_signal(&rf->cond);
+  pthread_join(rf->thread_gain, NULL);
 
   return ((rf_dev_t*)rf->dev)->srsran_rf_close(rf->handler);
 }
diff --git a/lib/src/phy/rf/rf_uhd_imp.cc b/lib/src/phy/rf/rf_uhd_imp.cc
index 4443f3603..be148f2f8 100644
--- a/lib/src/phy/rf/rf_uhd_imp.cc
+++ b/lib/src/phy/rf/rf_uhd_imp.cc
@@ -194,6 +194,7 @@ static std::array<cf_t, 64 * 1024> dummy_mem = {}; // For receiving
 
 static void log_overflow(rf_uhd_handler_t* h)
 {
+  std::unique_lock<std::mutex> lock(h->tx_mutex);
   if (h->tx_state == RF_UHD_IMP_TX_STATE_BURST) {
     h->tx_state = RF_UHD_IMP_TX_STATE_END_OF_BURST;
   }
@@ -208,6 +209,7 @@ static void log_overflow(rf_uhd_handler_t* h)
 
 static void log_late(rf_uhd_handler_t* h, bool is_rx)
 {
+  std::unique_lock<std::mutex> lock(h->tx_mutex);
   if (h->tx_state == RF_UHD_IMP_TX_STATE_BURST) {
     h->tx_state = RF_UHD_IMP_TX_STATE_END_OF_BURST;
   }
diff --git a/lib/src/radio/radio.cc b/lib/src/radio/radio.cc
index fb0e46a98..74a75e7c4 100644
--- a/lib/src/radio/radio.cc
+++ b/lib/src/radio/radio.cc
@@ -979,6 +979,7 @@ srsran_rf_info_t* radio::get_info()
 
 bool radio::get_metrics(rf_metrics_t* metrics)
 {
+  std::lock_guard<std::mutex> lock(metrics_mutex);
   *metrics   = rf_metrics;
   rf_metrics = {};
   return true;
@@ -990,8 +991,11 @@ void radio::handle_rf_msg(srsran_rf_error_t error)
     return;
   }
   if (error.type == srsran_rf_error_t::SRSRAN_RF_ERROR_OVERFLOW) {
-    rf_metrics.rf_o++;
-    rf_metrics.rf_error = true;
+    {
+      std::lock_guard<std::mutex> lock(metrics_mutex);
+      rf_metrics.rf_o++;
+      rf_metrics.rf_error = true;
+    }
     logger.info("Overflow");
 
     // inform PHY about overflow
@@ -999,13 +1003,15 @@ void radio::handle_rf_msg(srsran_rf_error_t error)
       phy->radio_overflow();
     }
   } else if (error.type == srsran_rf_error_t::SRSRAN_RF_ERROR_UNDERFLOW) {
+    logger.info("Underflow");
+    std::lock_guard<std::mutex> lock(metrics_mutex);
     rf_metrics.rf_u++;
     rf_metrics.rf_error = true;
-    logger.info("Underflow");
   } else if (error.type == srsran_rf_error_t::SRSRAN_RF_ERROR_LATE) {
+    logger.info("Late (detected in %s)", error.opt ? "rx call" : "asynchronous thread");
+    std::lock_guard<std::mutex> lock(metrics_mutex);
     rf_metrics.rf_l++;
     rf_metrics.rf_error = true;
-    logger.info("Late (detected in %s)", error.opt ? "rx call" : "asynchronous thread");
   } else if (error.type == srsran_rf_error_t::SRSRAN_RF_ERROR_RX) {
     logger.error("Fatal radio error occured.");
     phy->radio_failure();