From 848832f8b0d5f7a178ee18eaf0dfee12c4b7edbd Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Thu, 25 Apr 2024 18:26:50 +0200 Subject: [PATCH] Flush GraphNode RCU when removing Routes Since 44610c787 RCU keeps references until another write happens. even before then, some shared_ptr references may have been kept. When using a process graph, a route's activision-set can hold references to other graph-nodes (routes). This lead to Routes not being deleted until a second graph-reorder flushed the RCU. --- libs/ardour/ardour/graphnode.h | 1 + libs/ardour/graphnode.cc | 6 ++++++ libs/ardour/session.cc | 11 +++++++++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/libs/ardour/ardour/graphnode.h b/libs/ardour/ardour/graphnode.h index 853d35c7b5..1675597540 100644 --- a/libs/ardour/ardour/graphnode.h +++ b/libs/ardour/ardour/graphnode.h @@ -60,6 +60,7 @@ public: node_set_t const& activation_set (GraphChain const* const g) const; int init_refcount (GraphChain const* const g) const; + void flush_graph_activision_rcu (); protected: friend struct GraphChain; diff --git a/libs/ardour/graphnode.cc b/libs/ardour/graphnode.cc index 95ddd6a1b0..7f08259313 100644 --- a/libs/ardour/graphnode.cc +++ b/libs/ardour/graphnode.cc @@ -46,6 +46,12 @@ GraphActivision::init_refcount (GraphChain const* const g) const return m->at (g); } +void +GraphActivision::flush_graph_activision_rcu () +{ + _activation_set.flush (); +} + /* ****************************************************************************/ GraphNode::GraphNode (std::shared_ptr graph) diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc index b2bab19796..6f72fd602e 100644 --- a/libs/ardour/session.cc +++ b/libs/ardour/session.cc @@ -3849,12 +3849,10 @@ Session::remove_routes (std::shared_ptr routes_to_remove) * going away, then flush old references out of the graph. */ - routes.flush (); // maybe unsafe, see below. resort_routes (); /* get rid of it from the dead wood collection in the route list manager */ /* XXX i think this is unsafe as it currently stands, but i am not sure. (pd, october 2nd, 2006) */ - routes.flush (); /* remove these routes from the selection if appropriate, and signal @@ -3882,6 +3880,15 @@ Session::remove_routes (std::shared_ptr routes_to_remove) return; } + /* really drop reference to the Surround Master to + * unload the vapor plugin. While the RCU keeps a refecent the + * SurroundMaster, a new SurroundMaster cannot be added. + */ + std::shared_ptr r = routes.reader (); + for (auto const& rt : *r) { + rt->flush_graph_activision_rcu (); + } + PropertyChange pc; pc.add (Properties::order); PresentationInfo::Change (pc);