microsoft/picologging

Set `tp_base` for child extension types

Closed this issue · 2 comments

None of the C++ types that are children of other types have tp_base set.

https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_base

I'm not sure of the implications of this. There's a possibility that the base types' dealloc method isn't being called.

The child types are

  • Logger (Filterer)
  • Handler (Filterer)
  • StreamHandler (Handler)

To verify, create a very simple test and set break points in the dealloc methods for the child types and trace the dealloc into the child type (or put a print statement in Filterer_dealloc to see if it ever gets called.

Actually, the module init in _picologging.cxx sets tp_base for each of those, and that seems to be the recommended best practice. I think the actual issue is that CPython doesn't automatically dealloc on tp_base and leaves that implementation detail up to us- see for example how ordereddict dealloc is handled. So I'm creating a PR that calls dealloc of each of the base classes.

Was fixed in #120