[Bugfix] A GC'd Python subclass wrapper resurrects as its base type (residual gap from #369) #373

New issue

Closed

opened 2026-07-14 10:47:56 +00:00 by john · 0 comments

john commented

2026-07-14 10:47:56 +00:00

Owner

Spun out of #369, which is otherwise closed.

What #369 fixed

.parent, mcrfpy.find()/find_all(), and UIEntityCollection concat/slice each hand-rolled their own tp_alloc switch instead of consulting PythonObjectCache, so child.parent is parent was always False. They now all route through UIDrawable::pyobject_for() (and convertEntityToPython() for entities), so object identity holds and a Python subclass reached through those paths comes back intact.

What it did NOT fix

PythonObjectCache stores weakrefs. Object identity therefore only survives as long as someone in Python still holds a reference to the wrapper.

If a Python subclass wrapper is garbage-collected while only C++ still holds the object, the next lookup misses the cache and allocates a fresh base-type wrapper. The subclass, and any attributes the user set on it, are gone:

class MyFrame(mcrfpy.Frame):
    def __init__(self, **kw):
        super().__init__(**kw)
        self.hp = 100

parent = MyFrame(pos=(0,0), size=(100,100))
scene.children.append(parent)
kid = mcrfpy.Frame(pos=(1,1), size=(10,10))
parent.children.append(kid)

del parent          # C++ still owns it via the scene's collection
gc.collect()

type(kid.parent)    # mcrfpy.Frame, NOT MyFrame
kid.parent.hp       # AttributeError

Why it wasn't fixed in #369

The entity path solves this with a pin: UIEntity::pyobject (#266) is a strong ref from the C++ object back to its wrapper, so the cache weakref can never die while the entity is in a grid. That works because entity lifetime has a clear boundary — grid membership — and releasePyIdentity() breaks the cycle at every exit point.

Drawables have no equivalent boundary. They live in scene UI collections, frames, grid overlays; "leaving" is diffuse. An unconditional self-pin would leak every drawable ever created, since the wrapper→C++ and C++→wrapper refs form a cycle that nothing would break.

Possible directions

Owner-held strong ref, mirroring the #348 fix for UIGridView::get_grid: the owning UICollection pins wrappers for drawables whose is_python_subclass flag is set, and releases on removal. Bounded (only subclassed drawables pay), but needs a release at every removal path — the same audit releasePyIdentity() needed.
Pin only subclassed drawables at construction, with removeFromParent() / collection-removal as the release point.
Accept and document that subclass identity requires the user to keep a reference. Cheapest, but it is a sharp edge that will keep biting.

is_python_subclass already exists on UIDrawable (UIDrawable.h:270), so the discrimination in (1)/(2) is available today.

Test

tests/regression/issue_369_parent_identity_test.py covers the live-wrapper case. A test for this issue should del the only Python reference, gc.collect(), and assert the subclass and its attributes survive a .parent round trip.

Spun out of #369, which is otherwise closed. ## What #369 fixed `.parent`, `mcrfpy.find()`/`find_all()`, and `UIEntityCollection` concat/slice each hand-rolled their own `tp_alloc` switch instead of consulting `PythonObjectCache`, so `child.parent is parent` was always `False`. They now all route through `UIDrawable::pyobject_for()` (and `convertEntityToPython()` for entities), so object identity holds and a Python subclass reached through those paths comes back intact. ## What it did NOT fix `PythonObjectCache` stores **weakrefs**. Object identity therefore only survives as long as *someone in Python* still holds a reference to the wrapper. If a Python subclass wrapper is garbage-collected while only C++ still holds the object, the next lookup misses the cache and allocates a **fresh base-type wrapper**. The subclass, and any attributes the user set on it, are gone: ```python class MyFrame(mcrfpy.Frame): def __init__(self, **kw): super().__init__(**kw) self.hp = 100 parent = MyFrame(pos=(0,0), size=(100,100)) scene.children.append(parent) kid = mcrfpy.Frame(pos=(1,1), size=(10,10)) parent.children.append(kid) del parent # C++ still owns it via the scene's collection gc.collect() type(kid.parent) # mcrfpy.Frame, NOT MyFrame kid.parent.hp # AttributeError ``` ## Why it wasn't fixed in #369 The entity path solves this with a **pin**: `UIEntity::pyobject` (#266) is a strong ref from the C++ object back to its wrapper, so the cache weakref can never die while the entity is in a grid. That works because entity lifetime has a clear boundary — grid membership — and `releasePyIdentity()` breaks the cycle at every exit point. Drawables have no equivalent boundary. They live in scene UI collections, frames, grid overlays; "leaving" is diffuse. An unconditional self-pin would leak **every drawable ever created**, since the wrapper→C++ and C++→wrapper refs form a cycle that nothing would break. ## Possible directions 1. **Owner-held strong ref**, mirroring the #348 fix for `UIGridView::get_grid`: the owning `UICollection` pins wrappers for drawables whose `is_python_subclass` flag is set, and releases on removal. Bounded (only subclassed drawables pay), but needs a release at every removal path — the same audit `releasePyIdentity()` needed. 2. **Pin only subclassed drawables** at construction, with `removeFromParent()` / collection-removal as the release point. 3. **Accept and document** that subclass identity requires the user to keep a reference. Cheapest, but it is a sharp edge that will keep biting. `is_python_subclass` already exists on `UIDrawable` (`UIDrawable.h:270`), so the discrimination in (1)/(2) is available today. ## Test `tests/regression/issue_369_parent_identity_test.py` covers the live-wrapper case. A test for this issue should `del` the only Python reference, `gc.collect()`, and assert the subclass and its attributes survive a `.parent` round trip.