Fix T89046: Startup file with Python drivers crashes on load

Resolve order of initialization error reading startup file,
support postponing running wm_file_read_post until Blender
has been initialized.

Deferring updates allows duplicate initialization
to be removed from WM_init.

Reviewed By: mont29

Ref D12184

3 files changed, 88 insertions, 49 deletions

diff --git a/source/blender/windowmanager/intern/wm_files.c b/source/blender/windowmanager/intern/wm_files.c
index 65659dd7ab7..21c16e94097 100644
--- a/source/blender/windowmanager/intern/wm_files.c
+++ b/source/blender/windowmanager/intern/wm_files.c
@@ -596,19 +596,33 @@ static void wm_file_read_pre(bContext *C, bool use_data, bool UNUSED(use_userdef
 }
 
 /**
+ * Parameters for #wm_file_read_post, also used for deferred initialization.
+ */
+struct wmFileReadPost_Params {
+  uint use_data : 1;
+  uint use_userdef : 1;
+
+  uint is_startup_file : 1;
+  uint is_factory_startup : 1;
+  uint reset_app_template : 1;
+};
+
+/**
  * Logic shared between #WM_file_read & #wm_homefile_read,
  * updates to make after reading a file.
  */
-static void wm_file_read_post(bContext *C,
-                              const bool is_startup_file,
-                              const bool is_factory_startup,
-                              const bool use_data,
-                              const bool use_userdef,
-                              const bool reset_app_template)
+static void wm_file_read_post(bContext *C, const struct wmFileReadPost_Params *params)
 {
-  bool addons_loaded = false;
   wmWindowManager *wm = CTX_wm_manager(C);
 
+  const bool use_data = params->use_data;
+  const bool use_userdef = params->use_userdef;
+  const bool is_startup_file = params->is_startup_file;
+  const bool is_factory_startup = params->is_factory_startup;
+  const bool reset_app_template = params->reset_app_template;
+
+  bool addons_loaded = false;
+
   if (use_data) {
     if (!G.background) {
       /* remove windows which failed to be added via WM_check */
@@ -910,7 +924,14 @@ bool WM_file_read(bContext *C, const char *filepath, ReportList *reports)
         wm_history_file_update();
       }
 
-      wm_file_read_post(C, false, false, use_data, use_userdef, false);
+      wm_file_read_post(C,
+                        &(const struct wmFileReadPost_Params){
+                            .use_data = use_data,
+                            .use_userdef = use_userdef,
+                            .is_startup_file = false,
+                            .is_factory_startup = false,
+                            .reset_app_template = false,
+                        });
 
       bf_reports.duration.whole = PIL_check_seconds_timer() - bf_reports.duration.whole;
       file_read_reports_finalize(&bf_reports);
@@ -994,11 +1015,17 @@ const char *WM_init_state_app_template_get(void)
 /**
  * Called on startup, (context entirely filled with NULLs)
  * or called for 'New File' both `startup.blend` and `userpref.blend` are checked.
+ *
+ * \param r_params_file_read_post: Support postponed initialization,
+ * needed for initial startup when only some sub-systems have been initialized.
+ * When non-null, #wm_file_read_post doesn't run, instead it's arguments are stored
+ * in this return argument.
+ * The caller is responsible for calling #wm_homefile_read_post with this return argument.
  */
 void wm_homefile_read_ex(bContext *C,
                          const struct wmHomeFileRead_Params *params_homefile,
                          ReportList *reports,
-                         bool *r_is_factory_startup)
+                         struct wmFileReadPost_Params **r_params_file_read_post)
 {
 #if 0 /* UNUSED, keep as this may be needed later & the comment below isn't self evident. */
   /* Context does not always have valid main pointer here. */
@@ -1302,10 +1329,21 @@ void wm_homefile_read_ex(bContext *C,
     G.save_over = 0;
   }
 
-  wm_file_read_post(C, true, is_factory_startup, use_data, use_userdef, reset_app_template);
-
-  if (r_is_factory_startup) {
-    *r_is_factory_startup = is_factory_startup;
+  {
+    const struct wmFileReadPost_Params params_file_read_post = {
+        .use_data = use_data,
+        .use_userdef = use_userdef,
+        .is_startup_file = true,
+        .is_factory_startup = is_factory_startup,
+        .reset_app_template = reset_app_template,
+    };
+    if (r_params_file_read_post == NULL) {
+      wm_file_read_post(C, &params_file_read_post);
+    }
+    else {
+      *r_params_file_read_post = MEM_mallocN(sizeof(struct wmFileReadPost_Params), __func__);
+      **r_params_file_read_post = params_file_read_post;
+    }
   }
 }
 
@@ -1316,6 +1354,17 @@ void wm_homefile_read(bContext *C,
   wm_homefile_read_ex(C, params_homefile, reports, NULL);
 }
 
+/**
+ * Special case, support deferred execution of #wm_file_read_post,
+ * Needed when loading for the first time to workaround order of initialization bug, see T89046.
+ */
+void wm_homefile_read_post(struct bContext *C,
+                           const struct wmFileReadPost_Params *params_file_read_post)
+{
+  wm_file_read_post(C, params_file_read_post);
+  MEM_freeN((void *)params_file_read_post);
+}
+
 /* -------------------------------------------------------------------- */
 /** \name Blend-File History API
  * \{ */
diff --git a/source/blender/windowmanager/intern/wm_init_exit.c b/source/blender/windowmanager/intern/wm_init_exit.c
index 953aa683441..4b839384853 100644
--- a/source/blender/windowmanager/intern/wm_init_exit.c
+++ b/source/blender/windowmanager/intern/wm_init_exit.c
@@ -279,14 +279,31 @@ void WM_init(bContext *C, int argc, const char **argv)
 
   WM_msgbus_types_init();
 
-  bool is_factory_startup = true;
-
   /* Studio-lights needs to be init before we read the home-file,
    * otherwise the versioning cannot find the default studio-light. */
   BKE_studiolight_init();
 
   BLI_assert((G.fileflags & G_FILE_NO_UI) == 0);
 
+  /**
+   * NOTE(@campbellbarton): Startup file and order of initialization.
+   *
+   * Loading #BLENDER_STARTUP_FILE, #BLENDER_USERPREF_FILE, starting Python and other sub-systems,
+   * have inter-dependencies, for example.
+   *
+   * - Some sub-systems depend on the preferences (initializing icons depend on the theme).
+   * - Add-ons depends on the preferences to know what has been enabled.
+   * - Add-ons depends on the window-manger to register their key-maps.
+   * - Evaluating the startup file depends on Python for animation-drivers (see T89046).
+   * - Starting Python depends on the startup file so key-maps can be added in the window-manger.
+   *
+   * Loading preferences early, then application subsystems and finally the startup data would
+   * simplify things if it weren't for key-maps being part of the window-manager
+   * which is blend file data.
+   * Creating a dummy window-manager early, or moving the key-maps into the preferences
+   * would resolve this and may be worth looking into long-term, see: D12184 for details.
+   */
+  struct wmFileReadPost_Params *params_file_read_post = NULL;
   wm_homefile_read_ex(C,
                       &(const struct wmHomeFileRead_Params){
                           .use_data = true,
@@ -297,7 +314,7 @@ void WM_init(bContext *C, int argc, const char **argv)
                           .app_template_override = WM_init_state_app_template_get(),
                       },
                       NULL,
-                      &is_factory_startup);
+                      &params_file_read_post);
 
   /* Call again to set from userpreferences... */
   BLT_lang_set(NULL);
@@ -327,14 +344,6 @@ void WM_init(bContext *C, int argc, const char **argv)
 
   ED_spacemacros_init();
 
-  /* NOTE(campbell): there is a bug where python needs initializing before loading the
-   * startup.blend because it may contain PyDrivers. It also needs to be after
-   * initializing space types and other internal data.
-   *
-   * However can't redo this at the moment. Solution is to load python
-   * before wm_homefile_read() or make py-drivers check if python is running.
-   * Will try fix when the crash can be repeated. */
-
 #ifdef WITH_PYTHON
   BPY_python_start(C, argc, argv);
   BPY_python_reset(C);
@@ -374,30 +383,7 @@ void WM_init(bContext *C, int argc, const char **argv)
   }
 #endif
 
-  {
-    Main *bmain = CTX_data_main(C);
-    /* NOTE: logic here is from wm_file_read_post,
-     * call functions that depend on Python being initialized. */
-
-    /* normally 'wm_homefile_read' will do this,
-     * however python is not initialized when called from this function.
-     *
-     * unlikely any handlers are set but its possible,
-     * note that recovering the last session does its own callbacks. */
-    CTX_wm_window_set(C, CTX_wm_manager(C)->windows.first);
-
-    BKE_callback_exec_null(bmain, BKE_CB_EVT_VERSION_UPDATE);
-    BKE_callback_exec_null(bmain, BKE_CB_EVT_LOAD_POST);
-    if (is_factory_startup) {
-      BKE_callback_exec_null(bmain, BKE_CB_EVT_LOAD_FACTORY_STARTUP_POST);
-    }
-
-    wm_file_read_report(C, bmain);
-
-    if (!G.background) {
-      CTX_wm_window_set(C, NULL);
-    }
-  }
+  wm_homefile_read_post(C, params_file_read_post);
 }
 
 void WM_init_splash(bContext *C)
diff --git a/source/blender/windowmanager/wm_files.h b/source/blender/windowmanager/wm_files.h
index 7009885495b..2fa5a68829e 100644
--- a/source/blender/windowmanager/wm_files.h
+++ b/source/blender/windowmanager/wm_files.h
@@ -24,6 +24,7 @@
 #pragma once
 
 struct Main;
+struct wmFileReadPost_Params;
 struct wmGenericCallback;
 struct wmOperatorType;
 
@@ -64,11 +65,14 @@ struct wmHomeFileRead_Params {
 void wm_homefile_read_ex(struct bContext *C,
                          const struct wmHomeFileRead_Params *params_homefile,
                          struct ReportList *reports,
-                         bool *r_is_factory_startup);
+                         struct wmFileReadPost_Params **r_params_file_read_post);
 void wm_homefile_read(struct bContext *C,
                       const struct wmHomeFileRead_Params *params_homefile,
                       struct ReportList *reports);
 
+void wm_homefile_read_post(struct bContext *C,
+                           const struct wmFileReadPost_Params *params_file_read_post);
+
 void wm_file_read_report(bContext *C, struct Main *bmain);
 
 void wm_close_file_dialog(bContext *C, struct wmGenericCallback *post_action);