[max] Fix a bug when registering parameters in ossia.client

What happened is that the call to create_nodes would trigger a callback which would create the matchers in m_matchers.
But the code in find_or_create_matcher would also create another matcher just afterwards, pointing to the same ossia::node_base.

Then, in m_matchers = find_or_create_matchers(), the callback-created nodes would be deleted, causing the node itself to be deleted.
the matcher that was then assigned would then point to a deleted ossia::node_base, causing crashes.

src/ossia-max/src/attribute.cpp

@@ -75,31 +75,7 @@ void wrapper(void* x)
 void attribute::on_node_renamed_callback(ossia::net::node_base& node, const std::string&)
 {
   // first remove the matcher with old name
-  auto cpy = m_matchers;
-  for(const auto& m : cpy)
-  {
-    if(m->get_node() == &node)
-    {
-      m_matchers.erase(
-          std::remove(std::begin(m_matchers), std::end(m_matchers), m),
-          m_matchers.end());
-    }
-    else
-    {
-      auto parent = m->get_node()->get_parent();
-      while(parent)
-      {
-        if(parent == &node)
-        {
-          m_matchers.erase(
-              std::remove(std::begin(m_matchers), std::end(m_matchers), m),
-              m_matchers.end());
-          break;
-        }
-        parent = parent->get_parent();
-      }
-    }
-  }
+  remove_matchers(node);
 
   schedule(this, (method)wrapper, 0, nullptr, 0, nullptr);
 }
@@ -111,13 +87,9 @@ void attribute::on_parameter_created_callback(const ossia::net::parameter_base&
 
 void attribute::do_registration()
 {
-  if(m_name && std::string(m_name->s_name) != "")
+  if(m_name && std::string_view(m_name->s_name) != "")
   {
-    m_matchers = find_or_create_matchers();
-    set_matchers_index();
-
-    m_selection_path.reset();
-    fill_selection();
+    clear_and_init_registration();
 
     m_registered = true;
   }

src/ossia-max/src/device_base.cpp

@@ -143,7 +143,7 @@ void device_base::connect_slots()
     m_matchers.emplace_back(
         std::make_shared<matcher>(&m_device->get_root_node(), nullptr));
     int size = m_matchers.size();
-    m_matchers[size - 1]->m_index = size;
+    m_matchers.back()->m_index = size;
     // This is to handle [get address( message only
     // so is it really needed ?
     assert(!m_matchers.empty());

src/ossia-max/src/model.cpp

@@ -136,10 +136,7 @@ void model::do_registration()
   m_registering = true;
   m_registered = true;
 
-  m_matchers = find_or_create_matchers();
-
-  m_selection_path.reset();
-  fill_selection();
+  clear_and_init_registration();
 
   set_priority();
   set_description();

src/ossia-max/src/object_base.cpp

@@ -134,6 +134,21 @@ void object_base::highlight()
   clock_delay(m_highlight_clock, 2000);
 }
 
+void object_base::clear_and_init_registration()
+{
+    // Matchers must be cleared *before* find_or_create so that
+    // nodes that are killed on "m_matchers = "
+    // don't get found during the find_or_create_matchers()
+    m_node_selection.clear();
+    m_matchers.clear();
+
+    m_matchers = find_or_create_matchers();
+    set_matchers_index();
+
+    m_selection_path.reset();
+    fill_selection();
+}
+
 void object_base::reset_color(object_base* x)
 {
   t_object* box{};
@@ -170,8 +185,16 @@ bool regex_match_with_cache(std::string_view regex, std::string_view str)
     return re2::RE2::FullMatch(str, *r);
 }
 
+template<typename T>
+static void move_vector_into_another(std::vector<T>&& src, std::vector<T>& dst)
+{
+    dst.insert(dst.end(), std::make_move_iterator(src.begin()), std::make_move_iterator(src.end()));
+    src.clear();
+}
+
 std::vector<std::shared_ptr<matcher>> object_base::find_or_create_matchers()
 {
+  assert(m_matchers.empty());
   std::vector<std::shared_ptr<matcher>> matchers;
 
   update_path(); // TODO this might not always be necessary here
@@ -188,6 +211,7 @@ std::vector<std::shared_ptr<matcher>> object_base::find_or_create_matchers()
     bool is_prefix_pattern = ossia::traversal::is_pattern(prefix);
     bool is_osc_name_pattern = ossia::traversal::is_pattern(osc_name);
 
+
     for(auto dev : get_all_devices())
     {
       const std::string& name = dev->get_name();
@@ -264,12 +288,24 @@ std::vector<std::shared_ptr<matcher>> object_base::find_or_create_matchers()
             if(nodes.empty())
             {
               auto new_nodes = ossia::net::create_nodes(dev->get_root_node(), osc_name);
-              matchers.reserve(new_nodes.size());
-              for(auto n : new_nodes)
+              // Node were create in a callback directly inside m_matchers... rapatriate them
+              if(!m_matchers.empty()) {
+                  move_vector_into_another(std::move(m_matchers), matchers);
+                  for(auto n : new_nodes)
+                  {
+                    ossia::try_setup_parameter(
+                        static_cast<parameter*>(this)->m_type->s_name, *n);
+                  }
+              }
+              else
               {
-                ossia::try_setup_parameter(
-                    static_cast<parameter*>(this)->m_type->s_name, *n);
-                matchers.push_back(std::make_shared<matcher>(n, this));
+                  matchers.reserve(new_nodes.size());
+                  for(auto n : new_nodes)
+                  {
+                    ossia::try_setup_parameter(
+                        static_cast<parameter*>(this)->m_type->s_name, *n);
+                    matchers.push_back(std::make_shared<matcher>(n, this));
+                  }
               }
             }
             else
@@ -281,9 +317,20 @@ std::vector<std::shared_ptr<matcher>> object_base::find_or_create_matchers()
                 {
                   n = &ossia::net::create_node(*(n->get_parent()), n->get_name());
                 }
-                ossia::try_setup_parameter(
-                    static_cast<parameter*>(this)->m_type->s_name, *n);
-                matchers.push_back(std::make_shared<matcher>(n, this));
+
+                // Node were create in a callback directly inside m_matchers... rapatriate them
+                if(!m_matchers.empty())
+                {
+                    assert(m_matchers.size() == 1);
+                    matchers.push_back(std::move(m_matchers.front()));
+                    m_matchers.clear();
+                }
+                else
+                {
+                    ossia::try_setup_parameter(
+                        static_cast<parameter*>(this)->m_type->s_name, *n);
+                    matchers.push_back(std::make_shared<matcher>(n, this));
+                }
               }
             }
             break;
@@ -292,19 +339,36 @@ std::vector<std::shared_ptr<matcher>> object_base::find_or_create_matchers()
             if(nodes.empty())
             {
               auto new_nodes = ossia::net::create_nodes(dev->get_root_node(), osc_name);
+              if(!m_matchers.empty())
+              {
+                  move_vector_into_another(std::move(m_matchers), matchers);
+              }
+              else
+              {
               matchers.reserve(new_nodes.size());
               for(auto n : new_nodes)
               {
                 matchers.push_back(std::make_shared<matcher>(n, this));
               }
+              }
             }
             else
             {
               matchers.reserve(nodes.size());
               for(auto n : nodes)
               {
                 n = &ossia::net::create_node(*(n->get_parent()), n->get_name());
-                matchers.push_back(std::make_shared<matcher>(n, this));
+
+                if(!m_matchers.empty())
+                {
+                    assert(m_matchers.size() == 1);
+                    matchers.push_back(std::move(m_matchers.front()));
+                    m_matchers.clear();
+                }
+                else
+                {
+                    matchers.push_back(std::make_shared<matcher>(n, this));
+                }
               }
             }
             break;
@@ -332,6 +396,51 @@ std::vector<std::shared_ptr<matcher>> object_base::find_or_create_matchers()
   return matchers;
 }
 
+void object_base::remove_matchers(const ossia::net::node_base& node)
+{
+    for(auto it = m_matchers.begin(); it != m_matchers.end(); )
+    {
+      bool removed = false;
+      auto& m = *it;
+      if(m->get_node() == &node)
+      {
+        it = remove_matcher(it);
+        removed = true;
+      }
+      else
+      {
+        auto parent = m->get_node()->get_parent();
+        while(parent)
+        {
+          if(parent == &node)
+          {
+            it = remove_matcher(it);
+            removed = true;
+            break;
+          }
+          parent = parent->get_parent();
+        }
+      }
+
+      if(!removed)
+          ++it;
+    }
+}
+
+void object_base::remove_matcher(const std::shared_ptr<matcher>& m)
+{
+    ossia::remove_erase(m_node_selection, m.get());
+    ossia::remove_erase(m_matchers, m);
+}
+
+auto object_base::remove_matcher(matcher_vector::iterator it)
+-> matcher_vector::iterator
+{
+    auto& m = *it;
+    ossia::remove_erase(m_node_selection, m.get());
+    return m_matchers.erase(it);
+}
+
 void object_base::loadbang(object_base* x)
 {
   if(x->m_registered)

src/ossia-max/src/object_base.hpp

@@ -88,7 +88,8 @@ struct object_base
   float m_rate{10.f};
 
   std::shared_ptr<ossia::net::generic_device> m_device{};
-  std::vector<std::shared_ptr<matcher>> m_matchers{};
+  using matcher_vector = std::vector<std::shared_ptr<matcher>>;
+  matcher_vector m_matchers{};
   std::vector<matcher*> m_node_selection{};
   std::optional<ossia::traversal::path> m_selection_path{};
   static void class_setup(t_class* c);
@@ -168,6 +169,8 @@ struct object_base
   static void loadbang(object_base* x);
   void save_children_state();
   void highlight();
+  void clear_and_init_registration();
+
   static void reset_color(object_base* x);
 
   void push_parameter_value(ossia::net::parameter_base* param, const ossia::value& val);
@@ -177,6 +180,11 @@ struct object_base
 
 protected:
   std::vector<std::shared_ptr<matcher>> find_or_create_matchers();
+  void remove_matchers(const ossia::net::node_base& m);
+  void remove_matcher(const std::shared_ptr<matcher>& m);
+
+  [[nodiscard]]
+  matcher_vector::iterator remove_matcher(matcher_vector::iterator m);
 
   std::map<std::string, ossia::value> m_value_map{};
 

src/ossia-max/src/parameter.cpp

@@ -131,11 +131,7 @@ void parameter::assist(parameter* x, void* b, long m, long a, char* s)
 
 void parameter::do_registration()
 {
-  m_matchers = find_or_create_matchers();
-  set_matchers_index();
-
-  m_selection_path.reset();
-  fill_selection();
+  clear_and_init_registration();
 
   set_priority();
   set_hidden();

src/ossia-max/src/remote.cpp

@@ -292,34 +292,7 @@ void remote::on_device_created(device_base* obj)
 void remote::on_node_renamed_callback(ossia::net::node_base& node, const std::string&)
 {
   // first remove the matcher with old name
-  auto cpy = m_matchers;
-  for(const auto& m : cpy)
-  {
-    if(!m)
-      continue;
-
-    if(m->get_node() == &node)
-    {
-      m_matchers.erase(
-          std::remove(std::begin(m_matchers), std::end(m_matchers), m),
-          m_matchers.end());
-    }
-    else
-    {
-      auto parent = m->get_node()->get_parent();
-      while(parent)
-      {
-        if(parent == &node)
-        {
-          m_matchers.erase(
-              std::remove(std::begin(m_matchers), std::end(m_matchers), m),
-              m_matchers.end());
-          break;
-        }
-        parent = parent->get_parent();
-      }
-    }
-  }
+  remove_matchers(node);
 
   do_registration();
 }
@@ -331,13 +304,9 @@ void remote::on_parameter_created_callback(const ossia::net::parameter_base& add
 
 void remote::do_registration()
 {
-  if(m_name && std::string(m_name->s_name) != "")
+  if(m_name && std::string_view(m_name->s_name) != "")
   {
-    m_matchers = find_or_create_matchers();
-    set_matchers_index();
-
-    m_selection_path.reset();
-    fill_selection();
+      clear_and_init_registration();
 
     set_unit();
     m_registered = true;

src/ossia-max/src/utils.cpp

@@ -23,15 +23,19 @@ std::vector<ossia::net::generic_device*> get_all_devices()
   for(auto device_obj : instance.devices.reference())
   {
     auto dev = device_obj->m_device;
-    if(dev)
+    if(dev && !ossia::contains(devs, dev.get()))
+    {
       devs.push_back(dev.get());
+    }
   }
 
   for(auto client_obj : instance.clients.reference())
   {
     auto dev = client_obj->m_device;
-    if(dev)
+    if(dev && !ossia::contains(devs, dev.get()))
+    {
       devs.push_back(dev.get());
+    }
   }
 
   return devs;