From 7972785d7b90771f50534fe3e1101d8adb615fa3 Mon Sep 17 00:00:00 2001 From: Germano Cavalcante Date: Wed, 24 Feb 2021 11:57:29 -0300 Subject: PyAPI: Fix memory leak of parameters used for python 'draw_callbacks' When closing the blender, while the callbacks are removed, the reference count of the object used as `customdata` is not decremented. This commit adds two functions that correctly release the python `draw_callbacks` before releasing all `draw_callbacks`. Differential Revision: https://developer.blender.org/D10478 --- source/blender/blenkernel/intern/screen.c | 7 +++ source/blender/editors/include/ED_space_api.h | 3 + source/blender/editors/space_api/spacetypes.c | 16 ++++++ source/blender/python/BPY_extern.h | 6 ++ source/blender/python/intern/bpy_rna_callback.c | 67 +++++++++++++++++----- source/blender/windowmanager/WM_api.h | 3 + source/blender/windowmanager/intern/wm.c | 3 + source/blender/windowmanager/intern/wm_operators.c | 16 ++++++ 8 files changed, 107 insertions(+), 14 deletions(-) diff --git a/source/blender/blenkernel/intern/screen.c b/source/blender/blenkernel/intern/screen.c index 80a83aecea8..df239b0c4ee 100644 --- a/source/blender/blenkernel/intern/screen.c +++ b/source/blender/blenkernel/intern/screen.c @@ -65,6 +65,10 @@ #include "BLO_read_write.h" +#ifdef WITH_PYTHON +# include "BPY_extern.h" +#endif + static void screen_free_data(ID *id) { bScreen *screen = (bScreen *)id; @@ -328,6 +332,9 @@ static ListBase spacetypes = {NULL, NULL}; static void spacetype_free(SpaceType *st) { LISTBASE_FOREACH (ARegionType *, art, &st->regiontypes) { +#ifdef WITH_PYTHON + BPY_callback_screen_free(art); +#endif BLI_freelistN(&art->drawcalls); LISTBASE_FOREACH (PanelType *, pt, &art->paneltypes) { diff --git a/source/blender/editors/include/ED_space_api.h b/source/blender/editors/include/ED_space_api.h index 47c2d4f790c..c50bbc2f1e9 100644 --- a/source/blender/editors/include/ED_space_api.h +++ b/source/blender/editors/include/ED_space_api.h @@ -73,6 +73,9 @@ void *ED_region_draw_cb_activate(struct ARegionType *art, int type); void ED_region_draw_cb_draw(const struct bContext *, struct ARegion *, int); void ED_region_draw_cb_exit(struct ARegionType *, void *); +void ED_region_draw_cb_remove_by_type(struct ARegionType *art, + void *draw_fn, + void (*free)(void *)); /* generic callbacks */ /* ed_util.c */ void ED_region_draw_mouse_line_cb(const struct bContext *C, diff --git a/source/blender/editors/space_api/spacetypes.c b/source/blender/editors/space_api/spacetypes.c index 817760615df..c112c678a09 100644 --- a/source/blender/editors/space_api/spacetypes.c +++ b/source/blender/editors/space_api/spacetypes.c @@ -272,6 +272,22 @@ void ED_region_draw_cb_draw(const bContext *C, ARegion *region, int type) } } +void ED_region_draw_cb_remove_by_type(ARegionType *art, void *draw_fn, void (*free)(void *)) +{ + RegionDrawCB *rdc = art->drawcalls.first; + while (rdc) { + RegionDrawCB *rdc_next = rdc->next; + if (rdc->draw == draw_fn) { + if (free) { + free(rdc->customdata); + } + BLI_remlink(&art->drawcalls, rdc); + MEM_freeN(rdc); + } + rdc = rdc_next; + } +} + /* ********************* space template *********************** */ /* forward declare */ void ED_spacetype_xxx(void); diff --git a/source/blender/python/BPY_extern.h b/source/blender/python/BPY_extern.h index 366d801a888..90f54c50a6d 100644 --- a/source/blender/python/BPY_extern.h +++ b/source/blender/python/BPY_extern.h @@ -21,6 +21,7 @@ #pragma once struct AnimationEvalContext; +struct ARegionType; struct ChannelDriver; /* DNA_anim_types.h */ struct ID; /* DNA_ID.h */ struct ListBase; /* DNA_listBase.h */ @@ -33,6 +34,7 @@ struct bConstraintTarget; /* DNA_constraint_types.h*/ struct bContext; struct bContextDataResult; struct bPythonConstraint; /* DNA_constraint_types.h */ +struct wmWindowManager; #include "BLI_utildefines.h" @@ -100,6 +102,10 @@ void BPY_id_release(struct ID *id); bool BPY_string_is_keyword(const char *str); +/* bpy_rna_callback.c */ +void BPY_callback_screen_free(struct ARegionType *art); +void BPY_callback_wm_free(struct wmWindowManager *wm); + /* I18n for addons */ #ifdef WITH_INTERNATIONAL const char *BPY_app_translations_py_pgettext(const char *msgctxt, const char *msgid); diff --git a/source/blender/python/intern/bpy_rna_callback.c b/source/blender/python/intern/bpy_rna_callback.c index 2f8be0c44e0..fdd3cb363ea 100644 --- a/source/blender/python/intern/bpy_rna_callback.c +++ b/source/blender/python/intern/bpy_rna_callback.c @@ -23,28 +23,24 @@ #include -#include "RNA_types.h" - -#include "BLI_utildefines.h" - -#include "bpy_capi_utils.h" -#include "bpy_rna.h" -#include "bpy_rna_callback.h" +#include "../generic/python_utildefines.h" -#include "DNA_screen_types.h" #include "DNA_space_types.h" #include "RNA_access.h" #include "RNA_enum_types.h" -#include "BKE_context.h" #include "BKE_screen.h" #include "WM_api.h" #include "ED_space_api.h" -#include "../generic/python_utildefines.h" +#include "BPY_extern.h" /* For public API. */ + +#include "bpy_capi_utils.h" +#include "bpy_rna.h" +#include "bpy_rna_callback.h" /* Own include. */ /* Use this to stop other capsules from being mis-used. */ static const char *rna_capsual_id = "RNA_HANDLE"; @@ -261,6 +257,12 @@ static eSpace_Type rna_Space_refine_reverse(StructRNA *srna) return SPACE_EMPTY; } +static void cb_rna_capsule_destructor(PyObject *capsule) +{ + PyObject *args = PyCapsule_GetContext(capsule); + Py_DECREF(args); +} + PyObject *pyrna_callback_classmethod_add(PyObject *UNUSED(self), PyObject *args) { void *handle; @@ -378,10 +380,14 @@ PyObject *pyrna_callback_classmethod_add(PyObject *UNUSED(self), PyObject *args) return NULL; } + /* Keep the 'args' reference as long as the callback exists. + * This reference is decremented in #BPY_callback_screen_free and #BPY_callback_wm_free. */ + Py_INCREF(args); + PyObject *ret = PyCapsule_New((void *)handle, rna_capsual_id, NULL); - /* Store 'args' in context as well as the handler custom-data, - * because the handle may be freed by Blender (new file, new window... etc) */ + /* Store 'args' in context as well for simple access. */ + PyCapsule_SetDestructor(ret, cb_rna_capsule_destructor); PyCapsule_SetContext(ret, args); Py_INCREF(args); @@ -412,7 +418,6 @@ PyObject *pyrna_callback_classmethod_remove(PyObject *UNUSED(self), PyObject *ar "callback_remove(handler): NULL handler given, invalid or already removed"); return NULL; } - PyObject *handle_args = PyCapsule_GetContext(py_handle); if (srna == &RNA_WindowManager) { if (!PyArg_ParseTuple( @@ -466,11 +471,45 @@ PyObject *pyrna_callback_classmethod_remove(PyObject *UNUSED(self), PyObject *ar return NULL; } + /* The handle has been removed, so decrement its customdata. */ + PyObject *handle_args = PyCapsule_GetContext(py_handle); + Py_DECREF(handle_args); + /* don't allow reuse */ if (capsule_clear) { - Py_DECREF(handle_args); + PyCapsule_Destructor destructor_fn = PyCapsule_GetDestructor(py_handle); + if (destructor_fn) { + destructor_fn(py_handle); + PyCapsule_SetDestructor(py_handle, NULL); + } PyCapsule_SetName(py_handle, rna_capsual_id_invalid); } Py_RETURN_NONE; } + +/* -------------------------------------------------------------------- */ +/** \name Public API + * \{ */ + +static void cb_customdata_free(void *customdata) +{ + PyObject *tuple = customdata; + Py_DECREF(tuple); +} + +void BPY_callback_screen_free(struct ARegionType *art) +{ + PyGILState_STATE gilstate = PyGILState_Ensure(); + ED_region_draw_cb_remove_by_type(art, cb_region_draw, cb_customdata_free); + PyGILState_Release(gilstate); +} + +void BPY_callback_wm_free(struct wmWindowManager *wm) +{ + PyGILState_STATE gilstate = PyGILState_Ensure(); + WM_paint_cursor_remove_by_type(wm, cb_wm_cursor_draw, cb_customdata_free); + PyGILState_Release(gilstate); +} + +/** \} */ diff --git a/source/blender/windowmanager/WM_api.h b/source/blender/windowmanager/WM_api.h index 9eb4dd832cb..545371f4540 100644 --- a/source/blender/windowmanager/WM_api.h +++ b/source/blender/windowmanager/WM_api.h @@ -227,6 +227,9 @@ struct wmPaintCursor *WM_paint_cursor_activate( void *customdata); bool WM_paint_cursor_end(struct wmPaintCursor *handle); +void WM_paint_cursor_remove_by_type(struct wmWindowManager *wm, + void *draw_fn, + void (*free)(void *)); void WM_paint_cursor_tag_redraw(struct wmWindow *win, struct ARegion *region); void WM_cursor_warp(struct wmWindow *win, int x, int y); diff --git a/source/blender/windowmanager/intern/wm.c b/source/blender/windowmanager/intern/wm.c index 549b59e9e1d..b43357b1462 100644 --- a/source/blender/windowmanager/intern/wm.c +++ b/source/blender/windowmanager/intern/wm.c @@ -595,6 +595,9 @@ void wm_close_and_free(bContext *C, wmWindowManager *wm) WM_msgbus_destroy(wm->message_bus); } +#ifdef WITH_PYTHON + BPY_callback_wm_free(wm); +#endif BLI_freelistN(&wm->paintcursors); WM_drag_free_list(&wm->drags); diff --git a/source/blender/windowmanager/intern/wm_operators.c b/source/blender/windowmanager/intern/wm_operators.c index 297575e8dff..bab8359472f 100644 --- a/source/blender/windowmanager/intern/wm_operators.c +++ b/source/blender/windowmanager/intern/wm_operators.c @@ -2056,6 +2056,22 @@ bool WM_paint_cursor_end(wmPaintCursor *handle) return false; } +void WM_paint_cursor_remove_by_type(wmWindowManager *wm, void *draw_fn, void (*free)(void *)) +{ + wmPaintCursor *pc = wm->paintcursors.first; + while (pc) { + wmPaintCursor *pc_next = pc->next; + if (pc->draw == draw_fn) { + if (free) { + free(pc->customdata); + } + BLI_remlink(&wm->paintcursors, pc); + MEM_freeN(pc); + } + pc = pc_next; + } +} + /** \} */ /* -------------------------------------------------------------------- */ -- cgit v1.2.3