[Bugfix] UIEntity has no tp_dealloc — Py_INCREF(self) in init() is never balanced #275

New issue

Closed

opened 2026-03-07 23:25:22 +00:00 by john · 0 comments

john commented

2026-03-07 23:25:22 +00:00

Owner

Summary

UIEntity::init() calls Py_INCREF(self) at line 249 to store a back-reference, but PyUIEntityType has no tp_dealloc defined — it defaults to the base type's dealloc which does not call Py_DECREF on data->self. This means every Entity object has an extra unbalanced reference that is never released.

This is closely related to #266 (the reference cycle issue), but is a distinct problem: even if the self field is removed, the missing tp_dealloc means there's no custom cleanup for entities at all.

Root Cause

UIEntity.h:132-172 — PyUIEntityType definition:

inline PyTypeObject PyUIEntityType = {
    // ...
    .tp_methods = UIEntity_all_methods,
    .tp_getset = UIEntity::getsetters,
    .tp_init = (initproc)UIEntity::init,
    .tp_new = PyType_GenericNew,
    // NO .tp_dealloc defined!
};

UIEntity.cpp:248-249 — init creates the reference:

self->data->self = (PyObject*)self;
Py_INCREF(self);  // This increment is never balanced

Without a tp_dealloc, there's no place to:

Call Py_DECREF on data->self
Clear weak references (PyObject_ClearWeakRefs)
Release the shared_ptr<UIEntity> properly

Impact

Every entity leaks one reference count permanently
Entities are never garbage collected via refcount (only via cyclic GC, if at all)
The weakreflist member is never cleaned up
The shared_ptr<UIEntity> data member is never explicitly released

Fix

Add a proper tp_dealloc:

void UIEntity::dealloc(PyUIEntityObject* self) {
    if (self->weakreflist) {
        PyObject_ClearWeakRefs((PyObject*)self);
    }
    // Break the reference cycle
    if (self->data && self->data->self == (PyObject*)self) {
        self->data->self = nullptr;
        // Note: Don't Py_DECREF here — we ARE the object being deallocated
    }
    // Release shared_ptr
    self->data.reset();
    Py_TYPE(self)->tp_free((PyObject*)self);
}

And set it in the type:

.tp_dealloc = (destructor)UIEntity::dealloc,

Severity

High — combined with #266, this ensures every entity leaks permanently. The missing dealloc also means weak references and shared_ptrs are not properly cleaned up.

#266 — UIEntity self reference cycle (the Py_INCREF that creates the leak)
#251 — tp_dealloc callback destruction fix (other types already have proper dealloc)

## Summary `UIEntity::init()` calls `Py_INCREF(self)` at line 249 to store a back-reference, but `PyUIEntityType` has **no `tp_dealloc` defined** — it defaults to the base type's dealloc which does not call `Py_DECREF` on `data->self`. This means every Entity object has an extra unbalanced reference that is never released. This is closely related to #266 (the reference cycle issue), but is a distinct problem: even if the `self` field is removed, the missing `tp_dealloc` means there's no custom cleanup for entities at all. ## Root Cause `UIEntity.h:132-172` — PyUIEntityType definition: ```cpp inline PyTypeObject PyUIEntityType = { // ... .tp_methods = UIEntity_all_methods, .tp_getset = UIEntity::getsetters, .tp_init = (initproc)UIEntity::init, .tp_new = PyType_GenericNew, // NO .tp_dealloc defined! }; ``` `UIEntity.cpp:248-249` — init creates the reference: ```cpp self->data->self = (PyObject*)self; Py_INCREF(self); // This increment is never balanced ``` Without a `tp_dealloc`, there's no place to: 1. Call `Py_DECREF` on `data->self` 2. Clear weak references (`PyObject_ClearWeakRefs`) 3. Release the `shared_ptr<UIEntity>` properly ## Impact - Every entity leaks one reference count permanently - Entities are never garbage collected via refcount (only via cyclic GC, if at all) - The `weakreflist` member is never cleaned up - The `shared_ptr<UIEntity>` data member is never explicitly released ## Fix Add a proper `tp_dealloc`: ```cpp void UIEntity::dealloc(PyUIEntityObject* self) { if (self->weakreflist) { PyObject_ClearWeakRefs((PyObject*)self); } // Break the reference cycle if (self->data && self->data->self == (PyObject*)self) { self->data->self = nullptr; // Note: Don't Py_DECREF here — we ARE the object being deallocated } // Release shared_ptr self->data.reset(); Py_TYPE(self)->tp_free((PyObject*)self); } ``` And set it in the type: ```cpp .tp_dealloc = (destructor)UIEntity::dealloc, ``` ## Severity **High** — combined with #266, this ensures every entity leaks permanently. The missing dealloc also means weak references and shared_ptrs are not properly cleaned up. ## Related - #266 — UIEntity self reference cycle (the `Py_INCREF` that creates the leak) - #251 — tp_dealloc callback destruction fix (other types already have proper dealloc)