[Bf-blender-cvs] [6bf13d07341] master: Fix crash when loading different file with asset browser open

[Julian Eisel]

Commit: 6bf13d07341b15e2f2a302e306da6303cb6c0b39
Author: Julian Eisel
Date:   Thu Nov 17 15:50:08 2022 +0100
Branches: master
https://developer.blender.org/rB6bf13d07341b15e2f2a302e306da6303cb6c0b39

Fix crash when loading different file with asset browser open

Steps to reproduce were:
- Open an asset browser
- Open an asset library with assets in it
- Load a different file (e.g. File -> New -> General)

Didn't see a nice way to fix this with the current pre file load handler
callback we use for freeing asset libraries. Using this is cleaner, but
for now, the relationship between UI and asset system is too close
still, so better do explicit freeing at the right point in time.

===================================================================

M	source/blender/asset_system/AS_asset_library.h
M	source/blender/asset_system/intern/asset_library.cc
M	source/blender/asset_system/intern/asset_library_service.cc
M	source/blender/windowmanager/intern/wm_files.c

===================================================================

diff --git a/source/blender/asset_system/AS_asset_library.h b/source/blender/asset_system/AS_asset_library.h
index 83ee8cebcdf..0a67df2ecbf 100644
--- a/source/blender/asset_system/AS_asset_library.h
+++ b/source/blender/asset_system/AS_asset_library.h
@@ -16,6 +16,14 @@ extern "C" {
 /** Forward declaration, defined in intern/asset_library.hh */
 typedef struct AssetLibrary AssetLibrary;
 
+/**
+ * Force clearing of all asset library data. After calling this, new asset libraries can be loaded
+ * just as usual using #AS_asset_library_load(), no init or other setup is needed.
+ *
+ * Does not need to be called on exit, this is handled internally.
+ */
+void AS_asset_libraries_exit(void);
+
 /**
  * Return the #AssetLibrary rooted at the given directory path.
  *
diff --git a/source/blender/asset_system/intern/asset_library.cc b/source/blender/asset_system/intern/asset_library.cc
index 90d83d369d6..5beab18cba6 100644
--- a/source/blender/asset_system/intern/asset_library.cc
+++ b/source/blender/asset_system/intern/asset_library.cc
@@ -27,6 +27,12 @@ using namespace blender::asset_system;
 
 bool asset_system::AssetLibrary::save_catalogs_when_file_is_saved = true;
 
+/* Can probably removed once #WITH_DESTROY_VIA_LOAD_HANDLER gets enabled by default. */
+void AS_asset_libraries_exit()
+{
+  AssetLibraryService::destroy();
+}
+
 asset_system::AssetLibrary *AS_asset_library_load(const Main *bmain,
                                                   const AssetLibraryReference &library_reference)
 {
diff --git a/source/blender/asset_system/intern/asset_library_service.cc b/source/blender/asset_system/intern/asset_library_service.cc
index e46557e1b29..51d8e5fcee6 100644
--- a/source/blender/asset_system/intern/asset_library_service.cc
+++ b/source/blender/asset_system/intern/asset_library_service.cc
@@ -19,6 +19,17 @@
 
 #include "CLG_log.h"
 
+/* When enabled, use a pre file load handler (#BKE_CB_EVT_LOAD_PRE) callback to destroy the asset
+ * library service. Without this an explicit call from the file loading code is needed to do this,
+ * which is not as nice.
+ *
+ * TODO Currently disabled because UI data depends on asset library data, so we have to make sure
+ * it's freed in the right order (UI first). Pre-load handlers don't give us this order.
+ * Should be addressed with a proper ownership model for the asset system:
+ * https://wiki.blender.org/wiki/Source/Architecture/Asset_System/Back_End#Ownership_Model
+ */
+//#define WITH_DESTROY_VIA_LOAD_HANDLER
+
 static CLG_LogRef LOG = {"asset_system.asset_library_service"};
 
 namespace blender::asset_system {
@@ -134,6 +145,7 @@ void AssetLibraryService::allocate_service_instance()
   }
 }
 
+#ifdef WITH_DESTROY_VIA_LOAD_HANDLER
 static void on_blendfile_load(struct Main * /*bMain*/,
                               struct PointerRNA ** /*pointers*/,
                               const int /*num_pointers*/,
@@ -141,9 +153,11 @@ static void on_blendfile_load(struct Main * /*bMain*/,
 {
   AssetLibraryService::destroy();
 }
+#endif
 
 void AssetLibraryService::app_handler_register()
 {
+#ifdef WITH_DESTROY_VIA_LOAD_HANDLER
   /* The callback system doesn't own `on_load_callback_store_`. */
   on_load_callback_store_.alloc = false;
 
@@ -151,13 +165,16 @@ void AssetLibraryService::app_handler_register()
   on_load_callback_store_.arg = this;
 
   BKE_callback_add(&on_load_callback_store_, BKE_CB_EVT_LOAD_PRE);
+#endif
 }
 
 void AssetLibraryService::app_handler_unregister()
 {
+#ifdef WITH_DESTROY_VIA_LOAD_HANDLER
   BKE_callback_remove(&on_load_callback_store_, BKE_CB_EVT_LOAD_PRE);
   on_load_callback_store_.func = nullptr;
   on_load_callback_store_.arg = nullptr;
+#endif
 }
 
 bool AssetLibraryService::has_any_unsaved_catalogs() const
diff --git a/source/blender/windowmanager/intern/wm_files.c b/source/blender/windowmanager/intern/wm_files.c
index cfeaedaa2b1..dd3f6db6297 100644
--- a/source/blender/windowmanager/intern/wm_files.c
+++ b/source/blender/windowmanager/intern/wm_files.c
@@ -220,6 +220,11 @@ static void wm_window_match_init(bContext *C, ListBase *wmlist)
   CTX_wm_menu_set(C, NULL);
 
   ED_editors_exit(G_MAIN, true);
+
+  /* Asset loading is done by the UI/editors and they keep pointers into it. So make sure to clear
+   * it after UI/editors. */
+  ED_assetlist_storage_exit();
+  AS_asset_libraries_exit();
 }
 
 static void wm_window_substitute_old(wmWindowManager *oldwm,
@@ -606,12 +611,6 @@ void wm_file_read_report(bContext *C, Main *bmain)
 static void wm_file_read_pre(bContext *C, bool use_data, bool UNUSED(use_userdef))
 {
   if (use_data) {
-    /* XXX Do before executing the callbacks below, otherwise the asset list refers to storage in
-     * the asset library that's destructed through a callback below.
-     * Asset list is weak design and mixes asset representation lifetime management with UI
-     * lifetime. The asset system needs a better defined ownership model. */
-    ED_assetlist_storage_exit();
-
     BKE_callback_exec_null(CTX_data_main(C), BKE_CB_EVT_LOAD_PRE);
     BLI_timer_on_file_load();
   }