From 497bc4d19977abc7b9e2c0f5024a23057e680954 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Thu, 12 Aug 2021 14:50:40 +1000 Subject: 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 --- source/blender/windowmanager/intern/wm_files.c | 75 ++++++++++++++++++---- source/blender/windowmanager/intern/wm_init_exit.c | 56 ++++++---------- source/blender/windowmanager/wm_files.h | 6 +- 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 @@ -595,20 +595,34 @@ static void wm_file_read_pre(bContext *C, bool use_data, bool UNUSED(use_userdef UI_view2d_zoom_cache_reset(); } +/** + * 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, ¶ms_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); + ¶ms_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); -- cgit v1.2.3