john/McRogueFace
1
0
Fork 0
Code Issues 32 Pull requests Projects Releases Wiki Activity

Fix: Derivable drawable types participate in garbage collector cycle detection

Browse source
This commit is contained in:
John McCardle 2026-01-27 13:21:10 -05:00
commit 86bfebefcb
8 changed files with 420 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

41
src/UIArc.h
View file

@ -118,14 +118,21 @@ namespace mcrfpydef {
.tp_itemsize = 0,
.tp_dealloc = (destructor)[](PyObject* self) {
PyUIArcObject* obj = (PyUIArcObject*)self;
PyObject_GC_UnTrack(self);
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
.tp_repr = (reprfunc)UIArc::repr,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR(
"Arc(center=None, radius=0, start_angle=0, end_angle=90, color=None, thickness=1, **kwargs)\n\n"
"An arc UI element for drawing curved line segments.\n\n"
@ -162,6 +169,38 @@ namespace mcrfpydef {
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override\n"
),
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUIArcObject* obj = (PyUIArcObject*)self;
if (obj->data) {
if (obj->data->click_callable) {
PyObject* cb = obj->data->click_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_enter_callable) {
PyObject* cb = obj->data->on_enter_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_exit_callable) {
PyObject* cb = obj->data->on_exit_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_move_callable) {
PyObject* cb = obj->data->on_move_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
}
return 0;
},
.tp_clear = [](PyObject* self) -> int {
PyUIArcObject* obj = (PyUIArcObject*)self;
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
return 0;
},
.tp_methods = UIArc_methods,
.tp_getset = UIArc::getsetters,
.tp_base = &mcrfpydef::PyDrawableType,

52
src/UICaption.h
View file

@ -59,13 +59,21 @@ namespace mcrfpydef {
.tp_dealloc = (destructor)[](PyObject* self)
{
PyUICaptionObject* obj = (PyUICaptionObject*)self;
// Untrack from GC before destroying
PyObject_GC_UnTrack(self);
// Clear weak references
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
// TODO - reevaluate with PyFont usage; UICaption does not own the font
// release reference to font object
if (obj->font) Py_DECREF(obj->font);
// Clear Python references to break cycles
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
// Release reference to font object
Py_CLEAR(obj->font);
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
@ -73,7 +81,7 @@ namespace mcrfpydef {
//.tp_hash = NULL,
//.tp_iter
//.tp_iternext
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR("Caption(pos=None, font=None, text='', **kwargs)\n\n"
"A text display UI element with customizable font and styling.\n\n"
"Args:\n"
@ -114,6 +122,42 @@ namespace mcrfpydef {
" margin (float): General margin for alignment\n"
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override"),
// tp_traverse visits Python object references for GC cycle detection
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUICaptionObject* obj = (PyUICaptionObject*)self;
Py_VISIT(obj->font);
if (obj->data) {
if (obj->data->click_callable) {
PyObject* callback = obj->data->click_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_enter_callable) {
PyObject* callback = obj->data->on_enter_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_exit_callable) {
PyObject* callback = obj->data->on_exit_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_move_callable) {
PyObject* callback = obj->data->on_move_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
}
return 0;
},
// tp_clear breaks reference cycles by clearing Python references
.tp_clear = [](PyObject* self) -> int {
PyUICaptionObject* obj = (PyUICaptionObject*)self;
Py_CLEAR(obj->font);
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
return 0;
},
.tp_methods = UICaption_methods,
//.tp_members = PyUIFrame_members,
.tp_getset = UICaption::getsetters,

41
src/UICircle.h
View file

@ -107,14 +107,21 @@ namespace mcrfpydef {
.tp_itemsize = 0,
.tp_dealloc = (destructor)[](PyObject* self) {
PyUICircleObject* obj = (PyUICircleObject*)self;
PyObject_GC_UnTrack(self);
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
.tp_repr = (reprfunc)UICircle::repr,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR(
"Circle(radius=0, center=None, fill_color=None, outline_color=None, outline=0, **kwargs)\n\n"
"A circle UI element for drawing filled or outlined circles.\n\n"
@ -149,6 +156,38 @@ namespace mcrfpydef {
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override\n"
),
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUICircleObject* obj = (PyUICircleObject*)self;
if (obj->data) {
if (obj->data->click_callable) {
PyObject* cb = obj->data->click_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_enter_callable) {
PyObject* cb = obj->data->on_enter_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_exit_callable) {
PyObject* cb = obj->data->on_exit_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_move_callable) {
PyObject* cb = obj->data->on_move_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
}
return 0;
},
.tp_clear = [](PyObject* self) -> int {
PyUICircleObject* obj = (PyUICircleObject*)self;
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
return 0;
},
.tp_methods = UICircle_methods,
.tp_getset = UICircle::getsetters,
.tp_base = &mcrfpydef::PyDrawableType,

57
src/UIFrame.h
View file

@ -84,10 +84,19 @@ namespace mcrfpydef {
.tp_dealloc = (destructor)[](PyObject* self)
{
PyUIFrameObject* obj = (PyUIFrameObject*)self;
// Untrack from GC before destroying
PyObject_GC_UnTrack(self);
// Clear weak references
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
// Clear Python references to break cycles
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
@ -95,7 +104,7 @@ namespace mcrfpydef {
//.tp_hash = NULL,
//.tp_iter
//.tp_iternext
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR("Frame(pos=None, size=None, **kwargs)\n\n"
"A rectangular frame UI element that can contain other drawable elements.\n\n"
"Args:\n"
@ -139,6 +148,49 @@ namespace mcrfpydef {
" margin (float): General margin for alignment\n"
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override"),
// tp_traverse visits Python object references for GC cycle detection
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUIFrameObject* obj = (PyUIFrameObject*)self;
if (obj->data) {
// Visit callback references
if (obj->data->click_callable) {
PyObject* callback = obj->data->click_callable->borrow();
if (callback && callback != Py_None) {
Py_VISIT(callback);
}
}
if (obj->data->on_enter_callable) {
PyObject* callback = obj->data->on_enter_callable->borrow();
if (callback && callback != Py_None) {
Py_VISIT(callback);
}
}
if (obj->data->on_exit_callable) {
PyObject* callback = obj->data->on_exit_callable->borrow();
if (callback && callback != Py_None) {
Py_VISIT(callback);
}
}
if (obj->data->on_move_callable) {
PyObject* callback = obj->data->on_move_callable->borrow();
if (callback && callback != Py_None) {
Py_VISIT(callback);
}
}
}
return 0;
},
// tp_clear breaks reference cycles by clearing Python references
.tp_clear = [](PyObject* self) -> int {
PyUIFrameObject* obj = (PyUIFrameObject*)self;
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
return 0;
},
.tp_methods = UIFrame_methods,
//.tp_members = PyUIFrame_members,
.tp_getset = UIFrame::getsetters,
@ -150,6 +202,9 @@ namespace mcrfpydef {
if (self) {
self->data = std::make_shared<UIFrame>();
self->weakreflist = nullptr;
// Note: For GC types, tracking happens automatically via tp_alloc
// when Py_TPFLAGS_HAVE_GC is set. Do NOT call PyObject_GC_Track here
// as it would double-track and cause corruption.
}
return (PyObject*)self;
}

66
src/UIGrid.h
View file

@ -235,16 +235,29 @@ namespace mcrfpydef {
.tp_dealloc = (destructor)[](PyObject* self)
{
PyUIGridObject* obj = (PyUIGridObject*)self;
// Untrack from GC before destroying
PyObject_GC_UnTrack(self);
// Clear weak references
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
// Clear Python references to break cycles
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
// Grid-specific cell callbacks
obj->data->on_cell_enter_callable.reset();
obj->data->on_cell_exit_callable.reset();
obj->data->on_cell_click_callable.reset();
}
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
.tp_repr = (reprfunc)UIGrid::repr,
.tp_as_mapping = &UIGrid::mpmethods, // Enable grid[x, y] subscript access
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR("Grid(pos=None, size=None, grid_size=None, texture=None, **kwargs)\n\n"
"A grid-based UI element for tile-based rendering and entity management.\n\n"
"Args:\n"
@ -296,6 +309,57 @@ namespace mcrfpydef {
" margin (float): General margin for alignment\n"
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override"),
// tp_traverse visits Python object references for GC cycle detection
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUIGridObject* obj = (PyUIGridObject*)self;
if (obj->data) {
// Base class callbacks
if (obj->data->click_callable) {
PyObject* callback = obj->data->click_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_enter_callable) {
PyObject* callback = obj->data->on_enter_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_exit_callable) {
PyObject* callback = obj->data->on_exit_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_move_callable) {
PyObject* callback = obj->data->on_move_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
// Grid-specific cell callbacks
if (obj->data->on_cell_enter_callable) {
PyObject* callback = obj->data->on_cell_enter_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_cell_exit_callable) {
PyObject* callback = obj->data->on_cell_exit_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_cell_click_callable) {
PyObject* callback = obj->data->on_cell_click_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
}
return 0;
},
// tp_clear breaks reference cycles by clearing Python references
.tp_clear = [](PyObject* self) -> int {
PyUIGridObject* obj = (PyUIGridObject*)self;
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
obj->data->on_cell_enter_callable.reset();
obj->data->on_cell_exit_callable.reset();
obj->data->on_cell_click_callable.reset();
}
return 0;
},
.tp_methods = UIGrid_all_methods,
//.tp_members = UIGrid::members,
.tp_getset = UIGrid::getsetters,

41
src/UILine.h
View file

@ -104,14 +104,21 @@ namespace mcrfpydef {
.tp_itemsize = 0,
.tp_dealloc = (destructor)[](PyObject* self) {
PyUILineObject* obj = (PyUILineObject*)self;
PyObject_GC_UnTrack(self);
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
.tp_repr = (reprfunc)UILine::repr,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR(
"Line(start=None, end=None, thickness=1.0, color=None, **kwargs)\n\n"
"A line UI element for drawing straight lines between two points.\n\n"
@ -144,6 +151,38 @@ namespace mcrfpydef {
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override\n"
),
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUILineObject* obj = (PyUILineObject*)self;
if (obj->data) {
if (obj->data->click_callable) {
PyObject* cb = obj->data->click_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_enter_callable) {
PyObject* cb = obj->data->on_enter_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_exit_callable) {
PyObject* cb = obj->data->on_exit_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
if (obj->data->on_move_callable) {
PyObject* cb = obj->data->on_move_callable->borrow();
if (cb && cb != Py_None) Py_VISIT(cb);
}
}
return 0;
},
.tp_clear = [](PyObject* self) -> int {
PyUILineObject* obj = (PyUILineObject*)self;
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
return 0;
},
.tp_methods = UILine_methods,
.tp_getset = UILine::getsetters,
.tp_base = &mcrfpydef::PyDrawableType,

47
src/UISprite.h
View file

@ -91,12 +91,19 @@ namespace mcrfpydef {
.tp_dealloc = (destructor)[](PyObject* self)
{
PyUISpriteObject* obj = (PyUISpriteObject*)self;
// Untrack from GC before destroying
PyObject_GC_UnTrack(self);
// Clear weak references
if (obj->weakreflist != NULL) {
PyObject_ClearWeakRefs(self);
}
// release reference to font object
//if (obj->texture) Py_DECREF(obj->texture);
// Clear Python references to break cycles
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
obj->data.reset();
Py_TYPE(self)->tp_free(self);
},
@ -104,7 +111,7 @@ namespace mcrfpydef {
//.tp_hash = NULL,
//.tp_iter
//.tp_iternext
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC,
.tp_doc = PyDoc_STR("Sprite(pos=None, texture=None, sprite_index=0, **kwargs)\n\n"
"A sprite UI element that displays a texture or portion of a texture atlas.\n\n"
"Args:\n"
@ -143,6 +150,40 @@ namespace mcrfpydef {
" margin (float): General margin for alignment\n"
" horiz_margin (float): Horizontal margin override\n"
" vert_margin (float): Vertical margin override"),
// tp_traverse visits Python object references for GC cycle detection
.tp_traverse = [](PyObject* self, visitproc visit, void* arg) -> int {
PyUISpriteObject* obj = (PyUISpriteObject*)self;
if (obj->data) {
if (obj->data->click_callable) {
PyObject* callback = obj->data->click_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_enter_callable) {
PyObject* callback = obj->data->on_enter_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_exit_callable) {
PyObject* callback = obj->data->on_exit_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
if (obj->data->on_move_callable) {
PyObject* callback = obj->data->on_move_callable->borrow();
if (callback && callback != Py_None) Py_VISIT(callback);
}
}
return 0;
},
// tp_clear breaks reference cycles by clearing Python references
.tp_clear = [](PyObject* self) -> int {
PyUISpriteObject* obj = (PyUISpriteObject*)self;
if (obj->data) {
obj->data->click_unregister();
obj->data->on_enter_unregister();
obj->data->on_exit_unregister();
obj->data->on_move_unregister();
}
return 0;
},
.tp_methods = UISprite_methods,
//.tp_members = PyUIFrame_members,
.tp_getset = UISprite::getsetters,

87
tests/regression/subclass_callback_segfault_test.py Normal file
View file

@ -0,0 +1,87 @@
#!/usr/bin/env python3
"""Minimal reproduction of segfault when calling subclass method callback.
The issue: When a Frame subclass assigns self.on_click = self._on_click,
reading it back works but there's a segfault during cleanup.
"""
import mcrfpy
import sys
import gc
class MyFrame(mcrfpy.Frame):
def __init__(self, **kwargs):
super().__init__(**kwargs)
self.on_click = self._on_click
def _on_click(self, pos, button, action):
print(f"Clicked at {pos}, button={button}, action={action}")
def test_minimal():
"""Minimal test case."""
print("Creating MyFrame...")
obj = MyFrame(pos=(100, 100), size=(100, 100))
print(f"Reading on_click: {obj.on_click}")
print(f"Type: {type(obj.on_click)}")
print("Attempting to call on_click...")
try:
obj.on_click((50, 50), "left", "start")
print("Call succeeded!")
except Exception as e:
print(f"Exception: {type(e).__name__}: {e}")
print("Clearing callback...")
obj.on_click = None
print("Deleting object...")
del obj
print("Running GC...")
gc.collect()
print("About to exit...")
sys.exit(0)
def test_without_callback_clear():
"""Test without clearing callback first."""
print("Creating MyFrame...")
obj = MyFrame(pos=(100, 100), size=(100, 100))
print("Calling...")
obj.on_click((50, 50), "left", "start")
print("Deleting without clearing callback...")
del obj
gc.collect()
print("About to exit...")
sys.exit(0)
def test_added_to_scene():
"""Test when added to scene."""
print("Creating scene and MyFrame...")
scene = mcrfpy.Scene("test")
obj = MyFrame(pos=(100, 100), size=(100, 100))
scene.children.append(obj)
print("Calling via scene.children[0]...")
scene.children[0].on_click((50, 50), "left", "start")
print("About to exit...")
sys.exit(0)
if __name__ == "__main__":
# Try different scenarios
import sys
if len(sys.argv) > 1:
if sys.argv[1] == "2":
test_without_callback_clear()
elif sys.argv[1] == "3":
test_added_to_scene()
else:
test_minimal()