phy: fix resource grid pool destructor

Author: xavierarteaga

include/srsran/phy/support/shared_resource_grid.h

@@ -32,18 +32,11 @@ class shared_resource_grid
     /// Default destructor.
     virtual ~pool_interface() = default;
 
-    /// \brief Gets the reference to the resource grid from a given identifier.
-    ///
-    /// \param[in] identifier Internal pool resource grid identifier.
-    /// \return A reference to the selected resource grid.
-    /// \remark An assertion is triggered if the identifier is invalid.
-    virtual resource_grid& get(unsigned identifier) = 0;
-
-    /// \brief Notifies the pool of the release of the resource grid.
-    ///
-    /// \param[in] identifier Internal pool resource grid identifier.
-    /// \remark An assertion is triggered if the identifier is invalid.
-    virtual void notify_release_scope(unsigned identifier) = 0;
+    /// Gets the reference to the resource grid.
+    virtual resource_grid& get() = 0;
+
+    /// Notifies the pool of the release of the resource grid.
+    virtual void notify_release_scope() = 0;
   };
 
   /// Default constructor creates an invalid resource grid.
@@ -52,19 +45,15 @@ class shared_resource_grid
   /// \brief Constructs a shared resource grid.
   /// \param pool_       Reference to the resource grid pool.
   /// \param ref_count_  Reference counter.
-  /// \param identifier_ Resource grid identifier within the pool.
-  shared_resource_grid(pool_interface& pool_, std::atomic<unsigned>& ref_count_, unsigned identifier_) :
-    pool(&pool_), ref_count(&ref_count_), identifier(identifier_)
+  shared_resource_grid(pool_interface& pool_, std::atomic<unsigned>& ref_count_) : pool(&pool_), ref_count(&ref_count_)
   {
   }
 
   /// \brief Move constructor - moves the ownership of other resource grid.
   ///
-  /// Copies the pool reference, reference count and grid identifier. It invalidates the other instance without .
+  /// Copies the pool reference and reference count. It invalidates the other instance.
   shared_resource_grid(shared_resource_grid&& other) noexcept :
-    pool(std::exchange(other.pool, nullptr)),
-    ref_count(std::exchange(other.ref_count, nullptr)),
-    identifier(other.identifier)
+    pool(std::exchange(other.pool, nullptr)), ref_count(std::exchange(other.ref_count, nullptr))
   {
   }
 
@@ -102,7 +91,7 @@ class shared_resource_grid
     if (is_valid()) {
       bool last = dec_ref_count();
       if (last) {
-        pool->notify_release_scope(identifier);
+        pool->notify_release_scope();
       }
       pool      = nullptr;
       ref_count = nullptr;
@@ -113,28 +102,28 @@ class shared_resource_grid
   resource_grid& get()
   {
     srsran_assert(is_valid(), "The resource grid is invalid.");
-    return pool->get(identifier);
+    return pool->get();
   }
 
   /// Gets the resource grid for read-only.
   resource_grid& get() const
   {
     srsran_assert(pool != nullptr, "The resource grid is invalid.");
-    return pool->get(identifier);
+    return pool->get();
   }
 
   /// Gets the resource grid reader.
   const resource_grid_reader& get_reader() const
   {
     srsran_assert(pool != nullptr, "The resource grid is invalid.");
-    return pool->get(identifier).get_reader();
+    return pool->get().get_reader();
   }
 
   /// Gets the resource grid writer.
   resource_grid_writer& get_writer() const
   {
     srsran_assert(pool != nullptr, "The resource grid is invalid.");
-    return pool->get(identifier).get_writer();
+    return pool->get().get_writer();
   }
 
   /// Explicit copy of the resource grid.

lib/phy/support/resource_grid_pool_impl.cpp

@@ -21,107 +21,59 @@
 
 using namespace srsran;
 
-resource_grid_pool_impl::resource_grid_pool_impl(task_executor*                 async_executor_,
-                                                 std::vector<resource_grid_ptr> grids_) :
-  logger(srslog::fetch_basic_logger("PHY", true)),
-  grids(std::move(grids_)),
-  grids_scope_count(grids.size()),
-  grids_str_zero(grids.size()),
-  grids_str_reserved(grids.size()),
-  async_executor(async_executor_)
+shared_resource_grid resource_grid_pool_wrapper::try_reserve(srsran::stop_event_token token)
 {
-  // Create tracing list.
-  for (unsigned i_grid = 0, i_grid_end = grids.size(); i_grid != i_grid_end; ++i_grid) {
-    grids_str_zero[i_grid]     = "set_all_zero#" + std::to_string(i_grid);
-    grids_str_reserved[i_grid] = "rg_reserved#" + std::to_string(i_grid);
-  }
-
-  // All grids scopes must be zero.
-  std::fill(grids_scope_count.begin(), grids_scope_count.end(), ref_counter_available);
-}
-
-resource_grid_pool_impl::~resource_grid_pool_impl()
-{
-  // Wait for all resource grid to be returned to the pool.
-  std::for_each(grids_scope_count.begin(), grids_scope_count.end(), [](auto& ref_counter) {
-    for (unsigned current_ref_counter = ref_counter.load(std::memory_order_acquire);
-         current_ref_counter != ref_counter_available;
-         current_ref_counter = ref_counter.load(std::memory_order_acquire)) {
-      std::this_thread::sleep_for(std::chrono::milliseconds(1));
-    };
-  });
-}
-
-shared_resource_grid resource_grid_pool_impl::allocate_resource_grid(slot_point slot)
-{
-  // Trace point for grid reservation.
-  trace_point tp = l1_common_tracer.now();
-
-  // Select an identifier from the current request counter.
-  unsigned identifier = counter;
+  // Try to transition from available to one scope.
+  unsigned expected = ref_counter_available;
+  bool     success  = scope_count.compare_exchange_strong(expected, 1, std::memory_order_acq_rel);
 
-  // Increment request counter and wrap it with the number of grids.
-  counter = (counter + 1) % grids.size();
-
-  // Select counter for the selected identifier.
-  std::atomic<unsigned>& ref_count = grids_scope_count[identifier];
-
-  // Try to reset the reference counter.
-  unsigned expected  = ref_counter_available;
-  bool     available = ref_count.compare_exchange_strong(expected, 1, std::memory_order_acq_rel);
-
-  // Return an invalid grid if not available.
-  if (!available) {
-    logger.warning(slot.sfn(), slot.slot_index(), "Resource grid with identifier {} is not available.", identifier);
+  // Return an invalid shared grid if the state transition is unsuccessful.
+  if (!success) {
     return {};
   }
 
-  // Trace the resource grid reservation.
-  l1_common_tracer << trace_event(grids_str_reserved[identifier].c_str(), tp);
+  // Keep token if successful.
+  stop_token = std::move(token);
 
-  return {*this, ref_count, identifier};
+  // Create valid shared resource grid.
+  return shared_resource_grid{*this, scope_count};
 }
 
-resource_grid& resource_grid_pool_impl::get(unsigned identifier)
+void resource_grid_pool_wrapper::release()
 {
-  srsran_assert(identifier < grids.size(),
-                "RG identifier (i.e., {}) is out of range {}.",
-                identifier,
-                interval<unsigned>(0, grids.size()));
-  return *grids[identifier];
+  // Move token to the scope of this method before transitioning to available.
+  stop_event_token token = std::move(stop_token);
+
+  // Transition to available.
+  [[maybe_unused]] unsigned prev_scope_count = scope_count.exchange(ref_counter_available, std::memory_order_acq_rel);
+
+  // Assert that the grid is not present in any scope before becoming available.
+  srsran_assert((prev_scope_count == ref_counter_available) || (prev_scope_count == 0),
+                "The resource grid state cannot transition to available while it is present in {} scopes.",
+                prev_scope_count);
 }
 
-void resource_grid_pool_impl::notify_release_scope(unsigned identifier)
+void resource_grid_pool_wrapper::notify_release_scope()
 {
-  // verify the identifier is valid.
-  srsran_assert(identifier < grids.size(),
-                "RG identifier (i.e., {}) is out of range {}.",
-                identifier,
-                interval<unsigned>(0, grids.size()));
-
   // Skip zeroing if the grid is empty.
-  if (grids[identifier]->get_reader().is_empty()) {
-    grids_scope_count[identifier] = ref_counter_available;
+  if (grid->get_reader().is_empty()) {
+    release();
     return;
   }
 
   // If the pool is not configured with an asynchronous executor, it skips the zeroing process.
   if (async_executor == nullptr) {
-    grids_scope_count[identifier] = ref_counter_available;
+    release();
     return;
   }
 
   // Create lambda function for setting the grid to zero.
-  auto set_all_zero_func = [this, identifier]() noexcept SRSRAN_RTSAN_NONBLOCKING {
-    trace_point tp = l1_common_tracer.now();
-
+  auto set_all_zero_func = [this]() noexcept SRSRAN_RTSAN_NONBLOCKING {
     // Set grid to zero.
-    grids[identifier]->set_all_zero();
+    grid->set_all_zero();
 
     // Make the grid available.
-    grids_scope_count[identifier] = ref_counter_available;
-
-    l1_common_tracer << trace_event(grids_str_zero[identifier].c_str(), tp);
+    release();
   };
 
   // Try to execute the asynchronous housekeeping task.
@@ -130,6 +82,41 @@ void resource_grid_pool_impl::notify_release_scope(unsigned identifier)
   // Ensure the resource grid is marked as available even if it is not empty.
   // Avoid warnings about failure to prevent false alarms during gNb teardown.
   if (!success) {
-    grids_scope_count[identifier] = ref_counter_available;
+    release();
   }
 }
+
+resource_grid& resource_grid_pool_wrapper::get()
+{
+  return *grid;
+}
+
+resource_grid_pool_impl::~resource_grid_pool_impl()
+{
+  stop_control.stop();
+}
+
+shared_resource_grid resource_grid_pool_impl::allocate_resource_grid(slot_point slot)
+{
+  // Get a stop token, return an invalid grid if it is stopping.
+  stop_event_token token = stop_control.get_token();
+  if (token.stop_requested()) {
+    return {};
+  }
+
+  // Select an identifier from the current request counter.
+  unsigned identifier = counter;
+
+  // Increment request counter and wrap it with the number of grids.
+  counter = (counter + 1) % grids.size();
+
+  // Try reserving the resource grid.
+  shared_resource_grid grid = grids[identifier].try_reserve(std::move(token));
+
+  // Log a warning message if the grid is not available.
+  if (!grid) {
+    logger.warning(slot.sfn(), slot.slot_index(), "Resource grid with identifier {} is not available.", identifier);
+  }
+
+  return grid;
+}

lib/phy/support/resource_grid_pool_impl.h

@@ -15,6 +15,7 @@
 #include "srsran/phy/support/resource_grid_pool.h"
 #include "srsran/phy/support/shared_resource_grid.h"
 #include "srsran/srslog/srslog.h"
+#include "srsran/support/synchronization/stop_event.h"
 #include "srsran/support/tracing/event_tracing.h"
 #include <memory>
 #include <vector>
@@ -23,19 +24,76 @@ namespace srsran {
 
 class task_executor;
 
+/// \brief Resource grid state control.
+///
+/// This class controls the number of scopes in which a shared resource grid is present. Also, it guarantees that the
+/// resource grid is returned to the pool prior the destruction.
+class resource_grid_pool_wrapper : private shared_resource_grid::pool_interface
+{
+public:
+  /// Constructs a resource grid wrapper for the pool.
+  resource_grid_pool_wrapper(std::unique_ptr<resource_grid> grid_, task_executor* async_executor_) :
+    grid(std::move(grid_)), async_executor(async_executor_)
+  {
+    srsran_assert(grid, "Invalid resource grid pointer.");
+  }
+
+  /// Forbid copy constructor.
+  resource_grid_pool_wrapper(const resource_grid_pool_wrapper& other) = delete;
+
+  /// Move constructor.
+  resource_grid_pool_wrapper(resource_grid_pool_wrapper&& other) :
+    grid(std::move(other.grid)), async_executor(other.async_executor)
+  {
+  }
+
+  /// \brief Try to reserve the resource grid.
+  ///
+  /// \return True if the resource grid is reserved successfully. Otherwise, false.
+  shared_resource_grid try_reserve(stop_event_token token);
+
+private:
+  /// Reference counter value to indicate the availability of a resource grid.
+  static constexpr unsigned ref_counter_available = std::numeric_limits<unsigned>::max();
+
+  /// \brief Release resource grid.
+  ///
+  /// This method transitions the reference count from zero to available.
+  ///
+  /// \remark An assertion is triggered if the resource grid is present in any scope.
+  void release();
+
+  // See the shared_resource_grid::pool_interface interface for documentation.
+  void notify_release_scope() override;
+
+  // See the shared_resource_grid::pool_interface interface for documentation.
+  resource_grid& get() override;
+
+  /// Internal resource grid;
+  std::unique_ptr<resource_grid> grid;
+  /// Asynchronous task executor.
+  task_executor* async_executor;
+  /// \brief Internal resource grid scope count.
+  ///
+  /// A resource grid is available when the counter is equal to \c ref_counter_available.
+  std::atomic<unsigned> scope_count = ref_counter_available;
+  /// Signaling mechanism for safe stop.
+  stop_event_token stop_token;
+};
+
 /// \brief Implements a resource grid pool.
 ///
 /// It zeroes the resource grids asynchronously upon their scope release if it is given an asynchronous executor.
 /// Otherwise, it does not zero the resource grid.
-class resource_grid_pool_impl : public resource_grid_pool, private shared_resource_grid::pool_interface
+class resource_grid_pool_impl : public resource_grid_pool
 {
 public:
-  using resource_grid_ptr = std::unique_ptr<resource_grid>;
-
   /// \brief Constructs a resource grid pool.
-  /// \param async_executor_ Asynchronous housekeeping executor.
-  /// \param grids_          Resource grids.
-  resource_grid_pool_impl(task_executor* async_executor_, std::vector<resource_grid_ptr> grids_);
+  /// \param grids_ Resource grids.
+  resource_grid_pool_impl(std::vector<resource_grid_pool_wrapper> grids_) :
+    logger(srslog::fetch_basic_logger("PHY", true)), grids(std::move(grids_))
+  {
+  }
 
   /// The destructor waits until all resource grids are available to avoid dereferencing the reference counter.
   ~resource_grid_pool_impl() override;
@@ -44,31 +102,14 @@ class resource_grid_pool_impl : public resource_grid_pool, private shared_resour
   shared_resource_grid allocate_resource_grid(slot_point slot) override;
 
 private:
-  /// Reference counter value to indicate the availability of a resource grid.
-  static constexpr unsigned ref_counter_available = std::numeric_limits<unsigned>::max();
-
-  // See shared_resource_grid::pool_interface for documentation.
-  resource_grid& get(unsigned identifier) override;
-
-  // See shared_resource_grid::pool_interface for documentation.
-  void notify_release_scope(unsigned identifier) override;
-
   /// PHY logger.
   srslog::basic_logger& logger;
-  /// Actual pool of resource grids.
-  std::vector<resource_grid_ptr> grids;
   /// Counts the resource grid requests.
   unsigned counter = 0;
-  /// \brief Resource grid scope count.
-  ///
-  /// A resource grid is available when the counter is equal to \c ref_counter_available.
-  std::vector<std::atomic<unsigned>> grids_scope_count;
-  /// Pool of resource grid zero set string for tracing.
-  std::vector<std::string> grids_str_zero;
-  /// Pool of resource grid reservation string for tracing.
-  std::vector<std::string> grids_str_reserved;
-  /// Asynchronous task executor.
-  task_executor* async_executor;
+  /// Resource grid state controllers. There is a controller for each grid.
+  std::vector<resource_grid_pool_wrapper> grids;
+  /// Stop control.
+  stop_event_source stop_control;
 };
 
 } // namespace srsran