Fix T94708: negative reference count error with Python API callbacks

Regression in 7972785d7b that caused
Python callback arguments to be de-referenced twice - potentially
accessing freed memory. Making a new-file with a circle-select
tool active triggered this (for example).

Now arguments aren't de-referenced when Blender it's self has already
removed the callback handle.
This commit is contained in:
2022-01-07 16:32:01 +11:00
parent 1642028310
commit f24854005d
3 changed files with 15 additions and 8 deletions

View File

@@ -86,7 +86,7 @@ void *ED_region_draw_cb_activate(struct ARegionType *art,
int type); int type);
void ED_region_draw_cb_draw(const struct bContext *C, struct ARegion *region, int type); void ED_region_draw_cb_draw(const struct bContext *C, struct ARegion *region, int type);
void ED_region_surface_draw_cb_draw(struct ARegionType *art, int type); void ED_region_surface_draw_cb_draw(struct ARegionType *art, int type);
void ED_region_draw_cb_exit(struct ARegionType *art, void *handle); bool ED_region_draw_cb_exit(struct ARegionType *art, void *handle);
void ED_region_draw_cb_remove_by_type(struct ARegionType *art, void ED_region_draw_cb_remove_by_type(struct ARegionType *art,
void *draw_fn, void *draw_fn,
void (*free)(void *)); void (*free)(void *));

View File

@@ -248,15 +248,16 @@ void *ED_region_draw_cb_activate(ARegionType *art,
return rdc; return rdc;
} }
void ED_region_draw_cb_exit(ARegionType *art, void *handle) bool ED_region_draw_cb_exit(ARegionType *art, void *handle)
{ {
LISTBASE_FOREACH (RegionDrawCB *, rdc, &art->drawcalls) { LISTBASE_FOREACH (RegionDrawCB *, rdc, &art->drawcalls) {
if (rdc == (RegionDrawCB *)handle) { if (rdc == (RegionDrawCB *)handle) {
BLI_remlink(&art->drawcalls, rdc); BLI_remlink(&art->drawcalls, rdc);
MEM_freeN(rdc); MEM_freeN(rdc);
return; return true;
} }
} }
return false;
} }
static void ed_region_draw_cb_draw(const bContext *C, ARegion *region, ARegionType *art, int type) static void ed_region_draw_cb_draw(const bContext *C, ARegion *region, ARegionType *art, int type)

View File

@@ -383,6 +383,7 @@ PyObject *pyrna_callback_classmethod_remove(PyObject *UNUSED(self), PyObject *ar
void *handle; void *handle;
StructRNA *srna; StructRNA *srna;
bool capsule_clear = false; bool capsule_clear = false;
bool handle_removed = false;
if (PyTuple_GET_SIZE(args) < 2) { if (PyTuple_GET_SIZE(args) < 2) {
PyErr_SetString(PyExc_ValueError, "callback_remove(handler): expected at least 2 args"); PyErr_SetString(PyExc_ValueError, "callback_remove(handler): expected at least 2 args");
@@ -406,7 +407,7 @@ PyObject *pyrna_callback_classmethod_remove(PyObject *UNUSED(self), PyObject *ar
args, "OO!:WindowManager.draw_cursor_remove", &cls, &PyCapsule_Type, &py_handle)) { args, "OO!:WindowManager.draw_cursor_remove", &cls, &PyCapsule_Type, &py_handle)) {
return NULL; return NULL;
} }
WM_paint_cursor_end(handle); handle_removed = WM_paint_cursor_end(handle);
capsule_clear = true; capsule_clear = true;
} }
else if (RNA_struct_is_a(srna, &RNA_Space)) { else if (RNA_struct_is_a(srna, &RNA_Space)) {
@@ -445,7 +446,7 @@ PyObject *pyrna_callback_classmethod_remove(PyObject *UNUSED(self), PyObject *ar
params.region_type_enum.value_orig); params.region_type_enum.value_orig);
return NULL; return NULL;
} }
ED_region_draw_cb_exit(art, handle); handle_removed = ED_region_draw_cb_exit(art, handle);
capsule_clear = true; capsule_clear = true;
} }
else { else {
@@ -453,9 +454,14 @@ PyObject *pyrna_callback_classmethod_remove(PyObject *UNUSED(self), PyObject *ar
return NULL; return NULL;
} }
/* The handle has been removed, so decrement its customdata. */ /* When `handle_removed == false`: Blender has already freed the data
PyObject *handle_args = PyCapsule_GetContext(py_handle); * (freeing screen data when loading a new file for example).
Py_DECREF(handle_args); * This will have already decremented the user, so don't decrement twice. */
if (handle_removed == true) {
/* The handle has been removed, so decrement its custom-data. */
PyObject *handle_args = PyCapsule_GetContext(py_handle);
Py_DECREF(handle_args);
}
/* don't allow reuse */ /* don't allow reuse */
if (capsule_clear) { if (capsule_clear) {