Fix: USD hooks leak

If hooks are not unregistered Blender will report a leak on exit.

Store the hooks as unique_ptrs to remove manual memory management and
encapsulate the previous global list inside a function. These changes
ensure that everything is cleaned up on termination.

Also makes a small change to the hook documentation for a missing
`import` statement.

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

doc/python_api/examples/bpy.types.USDHook.py

@@ -55,6 +55,7 @@ import pxr.Gf as Gf
 import pxr.Sdf as Sdf
 import pxr.Usd as Usd
 import pxr.UsdShade as UsdShade
+import textwrap
 
 
 class USDHookExample(bpy.types.USDHook):

source/blender/io/usd/intern/usd_hook.cc

@@ -9,11 +9,10 @@
 #include <boost/python/call_method.hpp>
 #include <boost/python/class.hpp>
 #include <boost/python/import.hpp>
-#include <boost/python/object.hpp>
 #include <boost/python/return_value_policy.hpp>
 #include <boost/python/to_python_converter.hpp>
 
-#include "BLI_listbase.h"
+#include "BLI_utildefines.h"
 
 #include "BKE_report.h"
 
@@ -22,49 +21,52 @@
 #include "RNA_types.hh"
 #include "bpy_rna.h"
 
-#include "WM_api.hh"
-#include "WM_types.hh"
-
 #include <list>
+#include <memory>
 
 using namespace boost;
 
 namespace blender::io::usd {
 
-using USDHookList = std::list<USDHook *>;
+using USDHookList = std::list<std::unique_ptr<USDHook>>;
 
 /* USD hook type declarations */
-static USDHookList g_usd_hooks;
+static USDHookList &hook_list()
-
-void USD_register_hook(USDHook *hook)
 {
-  if (std::find(g_usd_hooks.begin(), g_usd_hooks.end(), hook) != g_usd_hooks.end()) {
+  static USDHookList hooks{};
+  return hooks;
+}
+
+void USD_register_hook(std::unique_ptr<USDHook> hook)
+{
+  if (USD_find_hook_name(hook->idname)) {
     /* The hook is already in the list. */
     return;
   }
 
   /* Add hook type to the list. */
-  g_usd_hooks.push_back(hook);
+  hook_list().push_back(std::move(hook));
 }
 
 void USD_unregister_hook(USDHook *hook)
 {
-  g_usd_hooks.remove(hook);
+  hook_list().remove_if(
+      [hook](const std::unique_ptr<USDHook> &item) { return item.get() == hook; });
 }
 
-USDHook *USD_find_hook_name(const char name[])
+USDHook *USD_find_hook_name(const char idname[])
 {
   /* sanity checks */
-  if (g_usd_hooks.empty() || (name == nullptr) || (name[0] == 0)) {
+  if (hook_list().empty() || (idname == nullptr) || (idname[0] == 0)) {
     return nullptr;
   }
 
   USDHookList::iterator hook_iter = std::find_if(
-      g_usd_hooks.begin(), g_usd_hooks.end(), [name](USDHook *hook) {
+      hook_list().begin(), hook_list().end(), [idname](const std::unique_ptr<USDHook> &item) {
-        return STREQ(hook->idname, name);
+        return STREQ(item->idname, idname);
       });
 
-  return (hook_iter == g_usd_hooks.end()) ? nullptr : *hook_iter;
+  return (hook_iter == hook_list().end()) ? nullptr : hook_iter->get();
 }
 
 /* Convert PointerRNA to a PyObject*. */
@@ -136,7 +138,7 @@ void register_hook_converters()
   static bool registered = false;
 
   /* No need to register if there are no hooks. */
-  if (g_usd_hooks.empty()) {
+  if (hook_list().empty()) {
     return;
   }
 
@@ -196,22 +198,22 @@ class USDHookInvoker {
   /* Attempt to call the function, if defined by the registered hooks. */
   void call() const
   {
-    if (g_usd_hooks.empty()) {
+    if (hook_list().empty()) {
       return;
     }
 
     PyGILState_STATE gilstate = PyGILState_Ensure();
 
     /* Iterate over the hooks and invoke the hook function, if it's defined. */
-    USDHookList::const_iterator hook_iter = g_usd_hooks.begin();
+    USDHookList::const_iterator hook_iter = hook_list().begin();
-    while (hook_iter != g_usd_hooks.end()) {
+    while (hook_iter != hook_list().end()) {
 
       /* XXX: Not sure if this is necessary:
        * Advance the iterator before invoking the callback, to guard
        * against the unlikely error where the hook is de-registered in
        * the callback. This would prevent a crash due to the iterator
        * getting invalidated. */
-      USDHook *hook = *hook_iter;
+      USDHook *hook = hook_iter->get();
       ++hook_iter;
 
       if (!hook->rna_ext.data) {
@@ -329,7 +331,7 @@ class OnImportInvoker : public USDHookInvoker {
 
 void call_export_hooks(pxr::UsdStageRefPtr stage, Depsgraph *depsgraph, ReportList *reports)
 {
-  if (g_usd_hooks.empty()) {
+  if (hook_list().empty()) {
     return;
   }
 
@@ -342,7 +344,7 @@ void call_material_export_hooks(pxr::UsdStageRefPtr stage,
                                 pxr::UsdShadeMaterial &usd_material,
                                 ReportList *reports)
 {
-  if (g_usd_hooks.empty()) {
+  if (hook_list().empty()) {
     return;
   }
 
@@ -352,7 +354,7 @@ void call_material_export_hooks(pxr::UsdStageRefPtr stage,
 
 void call_import_hooks(pxr::UsdStageRefPtr stage, ReportList *reports)
 {
-  if (g_usd_hooks.empty()) {
+  if (hook_list().empty()) {
     return;
   }
 

source/blender/io/usd/usd.hh

@@ -204,13 +204,12 @@ struct USDHook {
   ExtensionRNA rna_ext;
 };
 
-void USD_register_hook(USDHook *hook);
+void USD_register_hook(std::unique_ptr<USDHook> hook);
 /**
- * Remove the given entry from the list of registered hooks.
+ * Remove the given entry from the list of registered hooks and
- * Note that this does not free the allocated memory for the
+ * free the allocated memory for the hook instance.
- * hook instance, so a separate call to `MEM_freeN(hook)` is required.
  */
 void USD_unregister_hook(USDHook *hook);
-USDHook *USD_find_hook_name(const char name[]);
+USDHook *USD_find_hook_name(const char idname[]);
 
 };  // namespace blender::io::usd

source/blender/makesrna/intern/rna_usd.cc

@@ -46,8 +46,6 @@ static bool rna_USDHook_unregister(Main * /*bmain*/, StructRNA *type)
   /* unlink Blender-side data */
   USD_unregister_hook(hook);
 
-  MEM_freeN(hook);
-
   return true;
 }
 
@@ -60,8 +58,7 @@ static StructRNA *rna_USDHook_register(Main *bmain,
                                        StructFreeFunc free)
 {
   const char *error_prefix = "Registering USD hook class:";
-  USDHook dummy_hook = {{0}};
+  USDHook dummy_hook{};
-  USDHook *hook;
 
   /* setup dummy type info to store static properties in */
   PointerRNA dummy_hook_ptr = RNA_pointer_create(nullptr, &RNA_USDHook, &dummy_hook);
@@ -82,8 +79,7 @@ static StructRNA *rna_USDHook_register(Main *bmain,
   }
 
   /* check if we have registered this hook before, and remove it */
-  hook = USD_find_hook_name(dummy_hook.idname);
+  if (USDHook *hook = USD_find_hook_name(dummy_hook.idname)) {
-  if (hook) {
     BKE_reportf(reports,
                 RPT_INFO,
                 "%s '%s', bl_idname '%s' has been registered before, unregistering previous",
@@ -105,23 +101,24 @@ static StructRNA *rna_USDHook_register(Main *bmain,
   }
 
   /* create a new KeyingSetInfo type */
-  hook = static_cast<USDHook *>(MEM_mallocN(sizeof(USDHook), "python USD hook"));
+  auto hook = std::make_unique<USDHook>();
-  memcpy(hook, &dummy_hook, sizeof(USDHook));
+  *hook = dummy_hook;
 
   /* set RNA-extensions info */
   hook->rna_ext.srna = RNA_def_struct_ptr(&BLENDER_RNA, hook->idname, &RNA_USDHook);
   hook->rna_ext.data = data;
   hook->rna_ext.call = call;
   hook->rna_ext.free = free;
-  RNA_struct_blender_type_set(hook->rna_ext.srna, hook);
+  RNA_struct_blender_type_set(hook->rna_ext.srna, hook.get());
 
   /* add and register with other info as needed */
-  USD_register_hook(hook);
+  StructRNA *srna = hook->rna_ext.srna;
+  USD_register_hook(std::move(hook));
 
   WM_main_add_notifier(NC_WINDOW, nullptr);
 
   /* return the struct-rna added */
-  return hook->rna_ext.srna;
+  return srna;
 }
 
 #else