[Bugfix] UIEntity self reference cycle causes permanent memory leak

john commented

2026-03-07 23:20:21 +00:00

Owner

Summary

Every UIEntity created from Python leaks its Python wrapper object permanently. UIEntity::init() creates a reference cycle between the Python wrapper and the C++ object that is never broken, preventing garbage collection.

Root Cause

UIEntity.cpp:248-249:

// Store reference to Python object (legacy - to be removed)
self->data->self = (PyObject*)self;
Py_INCREF(self);

This creates an uncollectable cycle:

PyUIEntityObject holds std::shared_ptr<UIEntity> data → prevents C++ object destruction
UIEntity::self holds PyObject* with Py_INCREF → prevents Python object destruction

Neither can be freed because each prevents the other from reaching refcount 0. The comment says "legacy - to be removed" — it should be removed.

Impact

Every entity created leaks ~200+ bytes permanently. In a game that creates/destroys many entities (e.g., spawning enemies, projectiles, zone transitions), this is a continuous memory leak.

Reproduction

import mcrfpy, gc

# Create and "discard" entities
for i in range(10000):
    e = mcrfpy.Entity((0, 0))
    # e goes out of scope but is never freed

gc.collect()
# Memory usage keeps growing — entities are never collected

Fix

The PythonObjectCache (weak reference cache) already provides the same functionality that self was meant to provide (preserving derived class identity). The fix is:

Remove self->data->self = (PyObject*)self; Py_INCREF(self); from UIEntity::init()
Remove the self field from UIEntity class entirely
Update any code that reads entity->self to use PythonObjectCache::lookup() instead
Audit UIEntityCollectionIter::next() (line 45) which checks target->self != nullptr

Severity

High — every entity permanently leaks. Not a crash, but causes unbounded memory growth in long-running games.

## Summary Every `UIEntity` created from Python leaks its Python wrapper object permanently. `UIEntity::init()` creates a reference cycle between the Python wrapper and the C++ object that is never broken, preventing garbage collection. ## Root Cause `UIEntity.cpp:248-249`: ```cpp // Store reference to Python object (legacy - to be removed) self->data->self = (PyObject*)self; Py_INCREF(self); ``` This creates an uncollectable cycle: 1. `PyUIEntityObject` holds `std::shared_ptr<UIEntity> data` → prevents C++ object destruction 2. `UIEntity::self` holds `PyObject*` with `Py_INCREF` → prevents Python object destruction Neither can be freed because each prevents the other from reaching refcount 0. The comment says "legacy - to be removed" — it should be removed. ## Impact Every entity created leaks ~200+ bytes permanently. In a game that creates/destroys many entities (e.g., spawning enemies, projectiles, zone transitions), this is a continuous memory leak. ## Reproduction ```python import mcrfpy, gc # Create and "discard" entities for i in range(10000): e = mcrfpy.Entity((0, 0)) # e goes out of scope but is never freed gc.collect() # Memory usage keeps growing — entities are never collected ``` ## Fix The `PythonObjectCache` (weak reference cache) already provides the same functionality that `self` was meant to provide (preserving derived class identity). The fix is: 1. Remove `self->data->self = (PyObject*)self; Py_INCREF(self);` from `UIEntity::init()` 2. Remove the `self` field from `UIEntity` class entirely 3. Update any code that reads `entity->self` to use `PythonObjectCache::lookup()` instead 4. Audit `UIEntityCollectionIter::next()` (line 45) which checks `target->self != nullptr` ## Severity **High** — every entity permanently leaks. Not a crash, but causes unbounded memory growth in long-running games.

john referenced this issue

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

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

john referenced this issue

2026-03-07 23:26:21 +00:00

[Meta] Engine memory safety audit — 7DRL 2026 post-mortem #279

john added the

Bugfix

label

2026-03-08 00:00:53 +00:00

john referenced this issue

2026-03-08 03:20:15 +00:00

[Tiny Feature] Re-enable ASan leak detection after fixing #266/#275 #286

john commented

2026-03-09 04:24:40 +00:00

Author

Owner

Reopened: Subclass identity regression (commit `836a058`)

The Phase 3 fix (a12e035) that removed UIEntity::self to fix the permanent memory leak also broke Python subclass identity preservation. When the Python wrapper for a subclass like GameEntity(mcrfpy.Entity) was the only reference and went out of scope, the wrapper was GC'd. Later access via grid.entities returned a new base Entity wrapper, losing all custom methods (send, tooltip, bump, etc.).

This caused regressions in Liber Noster (7DRL 2026) where entities lost their derived behavior after GC cycles.

Fix: Added UIEntity::pyobject — a strong reference to the Python wrapper that keeps the subclass alive while the entity is in a grid. Cleared at all grid exit points via releasePyIdentity():

die() — entity removed from grid
set_grid(None) — entity detached from grid
EntityCollection operations: del, remove(), pop(), setitem, slice deletion/replacement

The key difference from the old self field: the strong reference is released when the entity leaves the grid, breaking the cycle and allowing proper GC. The old code never released it.

Regression test: tests/regression/issue_266_subclass_identity_test.py (29 tests exercising Liber Noster patterns — subclass hierarchy, isinstance checks, combat mixins, GC survival, die/pop/remove).

## Reopened: Subclass identity regression (commit 836a058) The Phase 3 fix (a12e035) that removed `UIEntity::self` to fix the permanent memory leak also broke Python subclass identity preservation. When the Python wrapper for a subclass like `GameEntity(mcrfpy.Entity)` was the only reference and went out of scope, the wrapper was GC'd. Later access via `grid.entities` returned a new base `Entity` wrapper, losing all custom methods (send, tooltip, bump, etc.). This caused regressions in Liber Noster (7DRL 2026) where entities lost their derived behavior after GC cycles. **Fix**: Added `UIEntity::pyobject` — a strong reference to the Python wrapper that keeps the subclass alive while the entity is in a grid. Cleared at all grid exit points via `releasePyIdentity()`: - `die()` — entity removed from grid - `set_grid(None)` — entity detached from grid - EntityCollection operations: `del`, `remove()`, `pop()`, `setitem`, slice deletion/replacement The key difference from the old `self` field: the strong reference is **released when the entity leaves the grid**, breaking the cycle and allowing proper GC. The old code never released it. Regression test: `tests/regression/issue_266_subclass_identity_test.py` (29 tests exercising Liber Noster patterns — subclass hierarchy, isinstance checks, combat mixins, GC survival, die/pop/remove).

john referenced this issue from a commit

2026-03-14 06:25:16 +00:00

Remove entity self-reference cycle

john closed this issue

2026-03-14 06:25:16 +00:00

john referenced this issue from a commit

2026-03-14 06:25:16 +00:00

Preserve Python subclass identity for entities in grids (reopens #266)

john reopened this issue

2026-03-14 06:25:16 +00:00

john referenced this issue

2026-03-15 00:30:45 +00:00

Entity step() callback — Python-side turn handler #299

john commented

2026-03-15 20:18:08 +00:00

Author

Owner

Status update

The self reference cycle has been fixed for UIEntity — replaced with the pyobject pattern (strong ref only while in grid, released at all exit points).

The old self->data->self pattern remains only in src/3d/Entity3D.cpp:725. Once that's fixed, this issue can be closed.

Part of the Grid & Entity Overhaul Roadmap (docs/GRID_ENTITY_OVERHAUL_ROADMAP.md), Phase 1 (Foundation).

## Status update The `self` reference cycle has been fixed for `UIEntity` — replaced with the `pyobject` pattern (strong ref only while in grid, released at all exit points). The old `self->data->self` pattern remains only in `src/3d/Entity3D.cpp:725`. Once that's fixed, this issue can be closed. Part of the Grid & Entity Overhaul Roadmap (`docs/GRID_ENTITY_OVERHAUL_ROADMAP.md`), **Phase 1** (Foundation).

john referenced this issue

2026-03-15 20:18:33 +00:00

Entity step() callback — Python-side turn handler #299

john referenced this issue from a commit

2026-03-16 11:21:02 +00:00

Phase 1: Safety & performance foundation for Grid/Entity overhaul

john closed this issue

2026-03-16 11:21:02 +00:00

john referenced this issue from a commit

2026-04-10 05:09:19 +00:00

Re-enable ASan leak detection, add Massif heap profiling target

john referenced this issue