Fix #134283: defer freeing tree/node/socket types

Currently, tree, node and socket types are always freed immediately when the
Python code unregisters them. This is problematic, because there may still be
references to those type pointers in evaluated data owned by potentially various
depsgraphs. It's not possible to change data in these depsgraphs, because they
may be independent from the original data and might be worked on by a separate
thread. So when the type pointers are freed directly, there will be a lot of
dangling pointers in evaluated copies. Since those are used to free the nodes,
there will be a crash when the depsgraph updates. In practice, this does not
happen that often, because typically custom node tree addons are not disabled
while in use. They still used to crash often, but only when Blender exits and
unregisters all types.

The solution is to just keep the typeinfo pointers alive and free them all at
the very end. This obviously has the downside that the list of pointers we need
to keep track of can grow endlessly, however in practice that doesn't really
happen under any normal circumstances.

I'm still getting some other crashes when enabling/disabling Sverchok while
testing, but not entirely reliably and also without this patch (the crash there
happens in RNA code). So some additional work will probably be needed later to
make this work properly in all cases.

Pull Request: https://projects.blender.org/blender/blender/pulls/134360

source/blender/blenkernel/BKE_node_tree_update.hh

@@ -38,6 +38,7 @@ void BKE_ntree_update_tag_node_new(bNodeTree *ntree, bNode *node);
 void BKE_ntree_update_tag_node_removed(bNodeTree *ntree);
 void BKE_ntree_update_tag_node_mute(bNodeTree *ntree, bNode *node);
 void BKE_ntree_update_tag_node_internal_link(bNodeTree *ntree, bNode *node);
+void BKE_ntree_update_tag_node_type(bNodeTree *ntree, bNode *node);
 
 void BKE_ntree_update_tag_socket_property(bNodeTree *ntree, bNodeSocket *socket);
 void BKE_ntree_update_tag_socket_new(bNodeTree *ntree, bNodeSocket *socket);

source/blender/blenkernel/intern/node.cc

@@ -1535,6 +1535,7 @@ static void node_set_typeinfo(const bContext *C,
   else {
     node->typeinfo = &NodeTypeUndefined;
   }
+  BKE_ntree_update_tag_node_type(ntree, node);
 }
 
 /* WARNING: default_value must either be null or match the typeinfo at this point.
@@ -1658,6 +1659,34 @@ bNodeTreeType *node_tree_type_find(const StringRef idname)
   return *value;
 }
 
+static void defer_free_tree_type(bNodeTreeType *tree_type)
+{
+  static ResourceScope scope;
+  scope.add_destruct_call([tree_type]() { MEM_delete(tree_type); });
+}
+
+static void defer_free_node_type(bNodeType *ntype)
+{
+  static ResourceScope scope;
+  scope.add_destruct_call([ntype]() {
+    /* May be null if the type is statically allocated. */
+    if (ntype->free_self) {
+      ntype->free_self(ntype);
+    }
+  });
+}
+
+static void defer_free_socket_type(bNodeSocketType *stype)
+{
+  static ResourceScope scope;
+  scope.add_destruct_call([stype]() {
+    /* May be null if the type is statically allocated. */
+    if (stype->free_self) {
+      stype->free_self(stype);
+    }
+  });
+}
+
 void node_tree_type_add(bNodeTreeType *nt)
 {
   get_node_tree_type_map().add(nt);
@@ -1674,7 +1703,11 @@ static void ntree_free_type(void *treetype_v)
   /* Probably not. It is pretty much expected we want to update G_MAIN here I think -
    * or we'd want to update *all* active Mains, which we cannot do anyway currently. */
   update_typeinfo(G_MAIN, nullptr, treetype, nullptr, nullptr, true);
-  MEM_delete(treetype);
+
+  /* Defer freeing the tree type, because it may still be referenced by trees in depsgraph
+   * copies. We can't just remove these tree types, because the depsgraph may exist completely
+   * separate from original data. */
+  defer_free_tree_type(treetype);
 }
 
 void node_tree_type_free_link(const bNodeTreeType *nt)
@@ -1719,13 +1752,15 @@ static void node_free_type(void *nodetype_v)
    * or we'd want to update *all* active Mains, which we cannot do anyway currently. */
   update_typeinfo(G_MAIN, nullptr, nullptr, nodetype, nullptr, true);
 
+  /* Setting this to null is necessary for the case of static node types. When running tests,
+   * they may be registered and unregistered multiple times. */
   delete nodetype->static_declaration;
   nodetype->static_declaration = nullptr;
 
-  /* Can be null when the type is not dynamically allocated. */
-  if (nodetype->free_self) {
-    nodetype->free_self(nodetype);
-  }
+  /* Defer freeing the node type, because it may still be referenced by nodes in depsgraph
+   * copies. We can't just remove these node types, because the depsgraph may exist completely
+   * separate from original data. */
+  defer_free_node_type(nodetype);
 }
 
 void node_register_type(bNodeType *nt)
@@ -1813,7 +1848,10 @@ static void node_free_socket_type(void *socktype_v)
    * or we'd want to update *all* active Mains, which we cannot do anyway currently. */
   update_typeinfo(G_MAIN, nullptr, nullptr, nullptr, socktype, true);
 
-  socktype->free_self(socktype);
+  /* Defer freeing the socket type, because it may still be referenced by nodes in depsgraph
+   * copies. We can't just remove these socket types, because the depsgraph may exist completely
+   * separate from original data. */
+  defer_free_socket_type(socktype);
 }
 
 void node_register_socket_type(bNodeSocketType *st)

source/blender/blenkernel/intern/node_tree_update.cc

@@ -585,6 +585,12 @@ class NodeTreeMainUpdater {
             nodes::update_node_declaration_and_sockets(ntree, *node);
           }
         }
+        else if (node->is_undefined()) {
+          /* If a node has become undefined (it generally was unregistered from Python), it does
+           * not have a declaration anymore. */
+          delete node->runtime->declaration;
+          node->runtime->declaration = nullptr;
+        }
         if (ntype.updatefunc) {
           ntype.updatefunc(&ntree, node);
         }
@@ -1711,6 +1717,11 @@ void BKE_ntree_update_tag_node_new(bNodeTree *ntree, bNode *node)
   add_node_tag(ntree, node, NTREE_CHANGED_NODE_PROPERTY);
 }
 
+void BKE_ntree_update_tag_node_type(bNodeTree *ntree, bNode *node)
+{
+  add_node_tag(ntree, node, NTREE_CHANGED_NODE_PROPERTY);
+}
+
 void BKE_ntree_update_tag_socket_property(bNodeTree *ntree, bNodeSocket *socket)
 {
   add_socket_tag(ntree, socket, NTREE_CHANGED_SOCKET_PROPERTY);

source/blender/makesrna/intern/rna_node_socket.cc

@@ -130,7 +130,7 @@ static void rna_NodeSocket_draw_color_simple(const blender::bke::bNodeSocketType
   RNA_parameter_list_free(&list);
 }
 
-static bool rna_NodeSocket_unregister(Main * /*bmain*/, StructRNA *type)
+static bool rna_NodeSocket_unregister(Main *bmain, StructRNA *type)
 {
   blender::bke::bNodeSocketType *st = static_cast<blender::bke::bNodeSocketType *>(
       RNA_struct_blender_type_get(type));
@@ -145,10 +145,11 @@ static bool rna_NodeSocket_unregister(Main * /*bmain*/, StructRNA *type)
 
   /* update while blender is running */
   WM_main_add_notifier(NC_NODE | NA_EDITED, nullptr);
+  BKE_main_ensure_invariants(*bmain);
   return true;
 }
 
-static StructRNA *rna_NodeSocket_register(Main * /*bmain*/,
+static StructRNA *rna_NodeSocket_register(Main *bmain,
                                           ReportList *reports,
                                           void *data,
                                           const char *identifier,
@@ -213,7 +214,7 @@ static StructRNA *rna_NodeSocket_register(Main * /*bmain*/,
 
   /* update while blender is running */
   WM_main_add_notifier(NC_NODE | NA_EDITED, nullptr);
-
+  BKE_main_ensure_invariants(*bmain);
   return st->ext_socket.srna;
 }
 

source/blender/makesrna/intern/rna_nodetree.cc

@@ -1021,7 +1021,7 @@ static bool rna_NodeTree_valid_socket_type(blender::bke::bNodeTreeType *ntreetyp
   return valid;
 }
 
-static bool rna_NodeTree_unregister(Main * /*bmain*/, StructRNA *type)
+static bool rna_NodeTree_unregister(Main *bmain, StructRNA *type)
 {
   blender::bke::bNodeTreeType *nt = static_cast<blender::bke::bNodeTreeType *>(
       RNA_struct_blender_type_get(type));
@@ -1037,6 +1037,7 @@ static bool rna_NodeTree_unregister(Main * /*bmain*/, StructRNA *type)
 
   /* update while blender is running */
   WM_main_add_notifier(NC_NODE | NA_EDITED, nullptr);
+  BKE_main_ensure_invariants(*bmain);
   return true;
 }
 
@@ -1116,7 +1117,7 @@ static StructRNA *rna_NodeTree_register(Main *bmain,
 
   /* update while blender is running */
   WM_main_add_notifier(NC_NODE | NA_EDITED, nullptr);
-
+  BKE_main_ensure_invariants(*bmain);
   return nt->rna_ext.srna;
 }
 
@@ -1931,7 +1932,7 @@ static void rna_Node_is_registered_node_type_runtime(bContext * /*C*/,
   RNA_parameter_set_lookup(parms, "result", &result);
 }
 
-static bool rna_Node_unregister(Main * /*bmain*/, StructRNA *type)
+static bool rna_Node_unregister(Main *bmain, StructRNA *type)
 {
   blender::bke::bNodeType *nt = static_cast<blender::bke::bNodeType *>(
       RNA_struct_blender_type_get(type));
@@ -1948,6 +1949,7 @@ static bool rna_Node_unregister(Main * /*bmain*/, StructRNA *type)
 
   /* update while blender is running */
   WM_main_add_notifier(NC_NODE | NA_EDITED, nullptr);
+  BKE_main_ensure_invariants(*bmain);
   return true;
 }
 
@@ -2088,7 +2090,7 @@ static StructRNA *rna_Node_register(Main *bmain,
 
   /* update while blender is running */
   WM_main_add_notifier(NC_NODE | NA_EDITED, nullptr);
-
+  BKE_main_ensure_invariants(*bmain);
   return nt->rna_ext.srna;
 }