[Bf-blender-cvs] [e945ca34664] asset-browser-poselib: Fix crash with "Current File" library and "Reload Scripts"
Julian Eisel
noreply at git.blender.org
Fri Apr 16 02:09:10 CEST 2021
Commit: e945ca34664249203e374ab9137720bed64de07e
Author: Julian Eisel
Date: Fri Apr 16 02:00:29 2021 +0200
Branches: asset-browser-poselib
https://developer.blender.org/rBe945ca34664249203e374ab9137720bed64de07e
Fix crash with "Current File" library and "Reload Scripts"
When a (Python defined) UI-list type is unregistered, we have to make sure no
list still references that type. We do the same for other such UI types.
This didn't happen to be an issue before, since the list type was not accessed
until the list was drawn again (which re-assigns the type).
===================================================================
M source/blender/blenkernel/BKE_screen.h
M source/blender/blenkernel/intern/screen.c
M source/blender/editors/interface/interface_templates.c
M source/blender/makesdna/DNA_screen_types.h
M source/blender/makesrna/intern/rna_ui.c
M source/blender/windowmanager/WM_api.h
M source/blender/windowmanager/intern/wm_uilist_type.c
===================================================================
diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h
index e8f1799d4e4..2e6493a2fcb 100644
--- a/source/blender/blenkernel/BKE_screen.h
+++ b/source/blender/blenkernel/BKE_screen.h
@@ -335,8 +335,6 @@ typedef void (*uiListFilterItemsFunc)(struct uiList *ui_list,
/* Listen to notifiers. Only for lists defined in C. */
typedef void (*uiListListener)(struct uiList *ui_list, wmRegionListenerParams *params);
-typedef void (*uiListFreeRuntimeDataFunc)(struct uiList *ui_list);
-
typedef struct uiListType {
struct uiListType *next, *prev;
@@ -349,9 +347,6 @@ typedef struct uiListType {
/* For lists defined in C only. */
uiListListener listener;
- /* Not exposed in the API. Only for UI code to free runtime data. */
- uiListFreeRuntimeDataFunc free_runtime_data_fn;
-
/* RNA integration */
ExtensionRNA rna_ext;
} uiListType;
diff --git a/source/blender/blenkernel/intern/screen.c b/source/blender/blenkernel/intern/screen.c
index e9d4cd84756..b2e9a429c2a 100644
--- a/source/blender/blenkernel/intern/screen.c
+++ b/source/blender/blenkernel/intern/screen.c
@@ -675,12 +675,13 @@ void BKE_area_region_free(SpaceType *st, ARegion *region)
BKE_area_region_panels_free(®ion->panels);
LISTBASE_FOREACH (uiList *, uilst, ®ion->ui_lists) {
- if (uilst->type && uilst->type->free_runtime_data_fn) {
- uilst->type->free_runtime_data_fn(uilst);
+ if (uilst->dyn_data && uilst->dyn_data->free_runtime_data_fn) {
+ uilst->dyn_data->free_runtime_data_fn(uilst);
}
if (uilst->properties) {
IDP_FreeProperty(uilst->properties);
}
+ MEM_SAFE_FREE(uilst->dyn_data);
}
if (region->gizmo_map != NULL) {
diff --git a/source/blender/editors/interface/interface_templates.c b/source/blender/editors/interface/interface_templates.c
index cd60ec5d006..a52ca159c15 100644
--- a/source/blender/editors/interface/interface_templates.c
+++ b/source/blender/editors/interface/interface_templates.c
@@ -5846,7 +5846,6 @@ static void uilist_free_dyn_data(uiList *ui_list)
MEM_SAFE_FREE(dyn_data->items_filter_flags);
MEM_SAFE_FREE(dyn_data->items_filter_neworder);
MEM_SAFE_FREE(dyn_data->customdata);
- MEM_freeN(dyn_data);
}
/**
@@ -6238,12 +6237,12 @@ static uiList *ui_list_ensure(bContext *C,
ui_list->dyn_data = MEM_callocN(sizeof(uiListDyn), "uiList.dyn_data");
}
uiListDyn *dyn_data = ui_list->dyn_data;
+ /* Note that this isn't a `uiListType` callback, it's stored in the runtime list data. Otherwise
+ * the runtime data could leak when the type is unregistered (e.g. on "Reload Scripts"). */
+ dyn_data->free_runtime_data_fn = uilist_free_dyn_data;
/* Because we can't actually pass type across save&load... */
ui_list->type = ui_list_type;
- /* Touching the type here is not nice, but this is just a stupid callback to free UI data from
- * BKE. It's always the same for any type visible in fact. */
- ui_list->type->free_runtime_data_fn = uilist_free_dyn_data;
ui_list->layout_type = layout_type;
/* Reset filtering data. */
diff --git a/source/blender/makesdna/DNA_screen_types.h b/source/blender/makesdna/DNA_screen_types.h
index 5c2203c7b6e..7d01cfd39c8 100644
--- a/source/blender/makesdna/DNA_screen_types.h
+++ b/source/blender/makesdna/DNA_screen_types.h
@@ -43,6 +43,7 @@ struct SpaceLink;
struct SpaceType;
struct uiBlock;
struct uiLayout;
+struct uiList;
struct wmDrawBuffer;
struct wmTimer;
struct wmTooltipState;
@@ -245,11 +246,16 @@ typedef struct PanelCategoryStack {
char idname[64];
} PanelCategoryStack;
+typedef void (*uiListFreeRuntimeDataFunc)(struct uiList *ui_list);
+
/* uiList dynamic data... */
/* These two Lines with # tell makesdna this struct can be excluded. */
#
#
typedef struct uiListDyn {
+ /** Callback to free UI data when freeing UI-Lists in BKE. */
+ uiListFreeRuntimeDataFunc free_runtime_data_fn;
+
/** Number of rows needed to draw all elements. */
int height;
/** Actual visual height of the list (in rows). */
diff --git a/source/blender/makesrna/intern/rna_ui.c b/source/blender/makesrna/intern/rna_ui.c
index 7f92f2d66c2..97852123954 100644
--- a/source/blender/makesrna/intern/rna_ui.c
+++ b/source/blender/makesrna/intern/rna_ui.c
@@ -659,7 +659,7 @@ static void uilist_filter_items(uiList *ui_list,
RNA_parameter_list_free(&list);
}
-static void rna_UIList_unregister(Main *UNUSED(bmain), StructRNA *type)
+static void rna_UIList_unregister(Main *bmain, StructRNA *type)
{
uiListType *ult = RNA_struct_blender_type_get(type);
@@ -670,7 +670,7 @@ static void rna_UIList_unregister(Main *UNUSED(bmain), StructRNA *type)
RNA_struct_free_extension(type, &ult->rna_ext);
RNA_struct_free(&BLENDER_RNA, type);
- WM_uilisttype_freelink(ult);
+ WM_uilisttype_remove_ptr(bmain, ult);
/* update while blender is running */
WM_main_add_notifier(NC_WINDOW, NULL);
diff --git a/source/blender/windowmanager/WM_api.h b/source/blender/windowmanager/WM_api.h
index 683d31dbbec..cb703773d8f 100644
--- a/source/blender/windowmanager/WM_api.h
+++ b/source/blender/windowmanager/WM_api.h
@@ -604,7 +604,7 @@ void WM_operator_type_modal_from_exec_for_object_edit_coords(struct wmOperatorTy
void WM_uilisttype_init(void);
struct uiListType *WM_uilisttype_find(const char *idname, bool quiet);
bool WM_uilisttype_add(struct uiListType *ult);
-void WM_uilisttype_freelink(struct uiListType *ult);
+void WM_uilisttype_remove_ptr(struct Main *bmain, struct uiListType *ult);
void WM_uilisttype_free(void);
void WM_uilisttype_to_full_list_id(const struct uiListType *ult,
diff --git a/source/blender/windowmanager/intern/wm_uilist_type.c b/source/blender/windowmanager/intern/wm_uilist_type.c
index 6d298ee63f1..957ba45c038 100644
--- a/source/blender/windowmanager/intern/wm_uilist_type.c
+++ b/source/blender/windowmanager/intern/wm_uilist_type.c
@@ -25,6 +25,7 @@
#include "BLI_sys_types.h"
+#include "DNA_space_types.h"
#include "DNA_windowmanager_types.h"
#include "MEM_guardedalloc.h"
@@ -32,9 +33,11 @@
#include "UI_interface.h"
#include "BLI_ghash.h"
+#include "BLI_listbase.h"
#include "BLI_string.h"
#include "BLI_utildefines.h"
+#include "BKE_main.h"
#include "BKE_screen.h"
#include "WM_api.h"
@@ -64,8 +67,62 @@ bool WM_uilisttype_add(uiListType *ult)
return 1;
}
-void WM_uilisttype_freelink(uiListType *ult)
+static void wm_uilisttype_unlink_from_region(const uiListType *ult, ARegion *region)
{
+ LISTBASE_FOREACH (uiList *, list, ®ion->ui_lists) {
+ if (list->type == ult) {
+ /* Don't delete the list, it's not just runtime data but stored in files. Freeing would make
+ * that data get lost. */
+ list->type = NULL;
+ }
+ }
+}
+
+static void wm_uilisttype_unlink_from_area(const uiListType *ult, ScrArea *area)
+{
+ LISTBASE_FOREACH (SpaceLink *, space_link, &area->spacedata) {
+ ListBase *regionbase = (space_link == area->spacedata.first) ? &area->regionbase :
+ &space_link->regionbase;
+ LISTBASE_FOREACH (ARegion *, region, regionbase) {
+ wm_uilisttype_unlink_from_region(ult, region);
+ }
+ }
+}
+
+/**
+ * For all lists representing \a ult, clear their `uiListType` pointer. Use when a list-type is
+ * deleted, so that the UI doesn't keep references to it.
+ *
+ * This is a common pattern for unregistering (usually .py defined) types at runtime, e.g. see
+ * #WM_gizmomaptype_group_unlink().
+ * Note that unlike in some other cases using this pattern, we don't actually free the lists with
+ * type \a ult, we just clear the reference to the type. That's because UI-Lists are written to
+ * files and we don't want them to get lost together with their (user visible) settings.
+ */
+static void wm_uilisttype_unlink(Main *bmain, const uiListType *ult)
+{
+ for (wmWindowManager *wm = bmain->wm.first; wm != NULL; wm = wm->id.next) {
+ LISTBASE_FOREACH (wmWindow *, win, &wm->windows) {
+ LISTBASE_FOREACH (ScrArea *, global_area, &win->global_areas.areabase) {
+ wm_uilisttype_unlink_from_area(ult, global_area);
+ }
+ }
+ }
+
+ for (bScreen *screen = bmain->screens.first; screen != NULL; screen = screen->id.next) {
+ LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) {
+ wm_uilisttype_unlink_from_area(ult, area);
+ }
+
+ LISTBASE_FOREACH (ARegion *, region, &screen->regionbase) {
+ wm_uilisttype_unlink_from_region(ult, region);
+ }
+ }
+}
+
+void WM_uilisttype_remove_ptr(Main *bmain, uiListType *ult)
+{
+ wm_uilisttype_unlink(bmain, ult);
bool ok = BLI_ghash_remove(uilisttypes_hash, ult->idname, NULL, MEM_freeN);
More information about the Bf-blender-cvs
mailing list