From ec991efc06676521267aa05259df59898c758338 Mon Sep 17 00:00:00 2001
From: Martin Braun <martin.braun@ettus.com>
Date: Thu, 1 Aug 2019 17:45:29 -0700
Subject: rfnoc: graph: Lock release/commit state

Property propagation and action handling depend on the release state,
but they are lengthy operations. It is therefore imperative to not
change the release/commit state during those methods.

This commit changes the following:
- Change the release state counter from an atomic to a non-atomic
  variable
- Instead, use a mutex to lock the release state counter, and use the
  same mutex for locking access to the property propagation and action
  handling

The rfnoc_graph now tries to release the graph before shutting down
blocks to make sure they don't get destroyed while those algorithms are
still running.
---
 host/lib/include/uhdlib/rfnoc/graph.hpp | 13 ++++++++-----
 host/lib/rfnoc/graph.cpp                | 18 +++++++++++++++---
 host/lib/rfnoc/rfnoc_graph.cpp          |  5 ++++-
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/host/lib/include/uhdlib/rfnoc/graph.hpp b/host/lib/include/uhdlib/rfnoc/graph.hpp
index 87977a1dc..5b735b624 100644
--- a/host/lib/include/uhdlib/rfnoc/graph.hpp
+++ b/host/lib/include/uhdlib/rfnoc/graph.hpp
@@ -7,14 +7,14 @@
 #ifndef INCLUDED_LIBUHD_GRAPH_HPP
 #define INCLUDED_LIBUHD_GRAPH_HPP
 
-#include <uhd/rfnoc/graph_edge.hpp>
 #include <uhd/rfnoc/actions.hpp>
+#include <uhd/rfnoc/graph_edge.hpp>
 #include <uhd/rfnoc/node.hpp>
 #include <boost/graph/adjacency_list.hpp>
-#include <tuple>
-#include <memory>
 #include <deque>
-#include <atomic>
+#include <memory>
+#include <mutex>
+#include <tuple>
 
 namespace uhd { namespace rfnoc { namespace detail {
 
@@ -273,9 +273,12 @@ private:
     // message is sent
     std::recursive_mutex _action_mutex;
 
+    //! Changes to the release/commit state of the graph are locked with this mutex
+    std::recursive_mutex _release_mutex;
+
     //! This counter gets decremented everytime commit() is called. When zero,
     // the graph is committed.
-    std::atomic<size_t> _release_count{1};
+    size_t _release_count{1};
 };
 
 
diff --git a/host/lib/rfnoc/graph.cpp b/host/lib/rfnoc/graph.cpp
index 1a25f84f9..6c849b61f 100644
--- a/host/lib/rfnoc/graph.cpp
+++ b/host/lib/rfnoc/graph.cpp
@@ -175,17 +175,20 @@ void graph_t::connect(node_ref_t src_node, node_ref_t dst_node, graph_edge_t edg
 
 void graph_t::commit()
 {
+    std::lock_guard<std::recursive_mutex> l(_release_mutex);
     if (_release_count) {
         _release_count--;
+    }
+    if (_release_count == 0) {
         _check_topology();
+        resolve_all_properties();
     }
-    UHD_LOG_TRACE(LOG_ID, "graph::commit() => " << _release_count.load());
-    resolve_all_properties();
 }
 
 void graph_t::release()
 {
-    UHD_LOG_TRACE(LOG_ID, "graph::release() => " << _release_count.load());
+    std::lock_guard<std::recursive_mutex> l(_release_mutex);
+    UHD_LOG_TRACE(LOG_ID, "graph::release() => " << _release_count);
     _release_count++;
 }
 
@@ -212,6 +215,11 @@ void graph_t::resolve_all_properties()
     if (boost::num_vertices(_graph) == 0) {
         return;
     }
+    // We can't release during property propagation, so we lock this entire
+    // method to make sure that a) different threads can't interfere with each
+    // other, and b) that we don't release the graph while this method is still
+    // running.
+    std::lock_guard<std::recursive_mutex> l(_release_mutex);
     if (_release_count) {
         return;
     }
@@ -350,6 +358,10 @@ void graph_t::resolve_all_properties()
 void graph_t::enqueue_action(
     node_ref_t src_node, res_source_info src_edge, action_info::sptr action)
 {
+    // We can't release during action handling, so we lock this entire
+    // method to make sure that we don't release the graph while this method is
+    // still running.
+    std::lock_guard<std::recursive_mutex> release_lock(_release_mutex);
     if (_release_count) {
         UHD_LOG_WARNING(LOG_ID,
             "Action propagation is not enabled, graph is not committed! Will not "
diff --git a/host/lib/rfnoc/rfnoc_graph.cpp b/host/lib/rfnoc/rfnoc_graph.cpp
index b3bff6c12..795ffd289 100644
--- a/host/lib/rfnoc/rfnoc_graph.cpp
+++ b/host/lib/rfnoc/rfnoc_graph.cpp
@@ -66,8 +66,11 @@ public:
 
     ~rfnoc_graph_impl()
     {
-        _graph.reset();
+        UHD_LOG_TRACE(LOG_ID, "Releasing detail::graph...");
+        _graph->release();
+        UHD_LOG_TRACE(LOG_ID, "Shutting down all blocks ...");
         _block_registry->shutdown();
+        _graph.reset();
     }
 
     /**************************************************************************
-- 
cgit v1.2.3