Remove entity self-reference cycle
UIEntity::init() stored self->data->self = (PyObject*)self with Py_INCREF(self), creating a reference cycle that prevented entities from ever being freed. The matching Py_DECREF never existed. Fix: Remove the `self` field from UIEntity entirely. Replace all read sites (iter next, getitem, get_perspective, entities_in_radius) with PythonObjectCache lookups using serial_number, which uses weak references and doesn't prevent garbage collection. Also adds tp_dealloc to PyUIEntityType to properly clean up the shared_ptr and weak references when the Python wrapper is freed. Closes #266, closes #275 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
71eb01c950
commit
a12e035a71
5 changed files with 123 additions and 30 deletions
|
|
@ -19,7 +19,7 @@
|
|||
|
||||
|
||||
UIEntity::UIEntity()
|
||||
: self(nullptr), grid(nullptr), position(0.0f, 0.0f), sprite_offset(0.0f, 0.0f)
|
||||
: grid(nullptr), position(0.0f, 0.0f), sprite_offset(0.0f, 0.0f)
|
||||
{
|
||||
// Initialize sprite with safe defaults (sprite has its own safe constructor now)
|
||||
// gridstate vector starts empty - will be lazily initialized when needed
|
||||
|
|
@ -241,10 +241,6 @@ int UIEntity::init(PyUIEntityObject* self, PyObject* args, PyObject* kwds) {
|
|||
}
|
||||
}
|
||||
|
||||
// Store reference to Python object (legacy - to be removed)
|
||||
self->data->self = (PyObject*)self;
|
||||
Py_INCREF(self);
|
||||
|
||||
// Set texture and sprite index
|
||||
if (texture_ptr) {
|
||||
self->data->sprite = UISprite(texture_ptr, sprite_index, sf::Vector2f(0,0), 1.0);
|
||||
|
|
|
|||
|
|
@ -60,7 +60,6 @@ PyObject* UIGridPointStateVector_to_PyList(const std::vector<UIGridPointState>&
|
|||
class UIEntity
|
||||
{
|
||||
public:
|
||||
PyObject* self = nullptr; // Reference to the Python object (if created from Python)
|
||||
uint64_t serial_number = 0; // For Python object cache
|
||||
std::shared_ptr<UIGrid> grid;
|
||||
std::vector<UIGridPointState> gridstate;
|
||||
|
|
@ -135,6 +134,12 @@ namespace mcrfpydef {
|
|||
.tp_name = "mcrfpy.Entity",
|
||||
.tp_basicsize = sizeof(PyUIEntityObject),
|
||||
.tp_itemsize = 0,
|
||||
.tp_dealloc = [](PyObject* obj) {
|
||||
auto* self = (PyUIEntityObject*)obj;
|
||||
if (self->weakreflist) PyObject_ClearWeakRefs(obj);
|
||||
self->data.reset();
|
||||
Py_TYPE(obj)->tp_free(obj);
|
||||
},
|
||||
.tp_repr = (reprfunc)UIEntity::repr,
|
||||
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
|
||||
.tp_doc = PyDoc_STR("Entity(grid_pos=None, texture=None, sprite_index=0, **kwargs)\n\n"
|
||||
|
|
|
|||
|
|
@ -41,10 +41,12 @@ PyObject* UIEntityCollectionIter::next(PyUIEntityCollectionIterObject* self)
|
|||
auto target = *self->current;
|
||||
++self->current;
|
||||
|
||||
// Return the stored Python object if it exists (preserves derived types)
|
||||
if (target->self != nullptr) {
|
||||
Py_INCREF(target->self);
|
||||
return target->self;
|
||||
// Check cache first to preserve derived class identity
|
||||
if (target->serial_number != 0) {
|
||||
PyObject* cached = PythonObjectCache::getInstance().lookup(target->serial_number);
|
||||
if (cached) {
|
||||
return cached; // Already INCREF'd by lookup
|
||||
}
|
||||
}
|
||||
|
||||
// Otherwise create and return a new Python Entity object
|
||||
|
|
@ -107,13 +109,7 @@ PyObject* UIEntityCollection::getitem(PyUIEntityCollectionObject* self, Py_ssize
|
|||
}
|
||||
}
|
||||
|
||||
// Legacy: If the entity has a stored Python object reference, return that
|
||||
if (target->self != nullptr) {
|
||||
Py_INCREF(target->self);
|
||||
return target->self;
|
||||
}
|
||||
|
||||
// Otherwise, create a new base Entity object
|
||||
// Create a new base Entity object
|
||||
PyTypeObject* entity_type = &mcrfpydef::PyUIEntityType;
|
||||
|
||||
auto o = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0);
|
||||
|
|
|
|||
|
|
@ -1442,13 +1442,7 @@ PyObject* UIGrid::get_perspective(PyUIGridObject* self, void* closure)
|
|||
}
|
||||
}
|
||||
|
||||
// Legacy: If the entity has a stored Python object reference
|
||||
if (locked->self != nullptr) {
|
||||
Py_INCREF(locked->self);
|
||||
return locked->self;
|
||||
}
|
||||
|
||||
// Otherwise, create a new base Entity object
|
||||
// Create a new base Entity object
|
||||
auto type = &mcrfpydef::PyUIEntityType;
|
||||
auto o = (PyUIEntityObject*)type->tp_alloc(type, 0);
|
||||
if (o) {
|
||||
|
|
@ -1936,11 +1930,12 @@ PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, Py
|
|||
for (size_t i = 0; i < entities.size(); i++) {
|
||||
auto& entity = entities[i];
|
||||
|
||||
// Return stored Python object if it exists
|
||||
if (entity->self != nullptr) {
|
||||
Py_INCREF(entity->self);
|
||||
PyList_SET_ITEM(result, i, entity->self);
|
||||
} else {
|
||||
// Check cache first to preserve derived class identity
|
||||
PyObject* py_entity = nullptr;
|
||||
if (entity->serial_number != 0) {
|
||||
py_entity = PythonObjectCache::getInstance().lookup(entity->serial_number);
|
||||
}
|
||||
if (!py_entity) {
|
||||
// Create new Python Entity wrapper
|
||||
auto pyEntity = (PyUIEntityObject*)entity_type->tp_alloc(entity_type, 0);
|
||||
if (!pyEntity) {
|
||||
|
|
@ -1949,8 +1944,9 @@ PyObject* UIGrid::py_entities_in_radius(PyUIGridObject* self, PyObject* args, Py
|
|||
}
|
||||
pyEntity->data = entity;
|
||||
pyEntity->weakreflist = NULL;
|
||||
PyList_SET_ITEM(result, i, (PyObject*)pyEntity);
|
||||
py_entity = (PyObject*)pyEntity;
|
||||
}
|
||||
PyList_SET_ITEM(result, i, py_entity);
|
||||
}
|
||||
|
||||
return result;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue