Skip to content

Commit 5fbcc67

Browse files
committed
Document Cython tp_dealloc step mapping and guard omitted steps
Map each step of Cython's generated tp_dealloc (generate_dealloc_function) to the hand-built TVMFFIPyTpDeallocSlot, marking which steps it reproduces and why the rest are inapplicable to the carrier layout. Add layout guards in TVMFFIPyWrapDealloc that fail loudly at import if a carrier ever gains a tp_finalize (__del__) or becomes weak-referenceable, since the slot omits those steps.
1 parent 8bfec2e commit 5fbcc67

1 file changed

Lines changed: 45 additions & 9 deletions

File tree

python/tvm_ffi/cython/tvm_ffi_python_helpers.h

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -834,8 +834,27 @@ inline void TVMFFIPyDeleteSpace(void* ptr) {
834834
// transition(o); if GC: GC_UnTrack(o); if has-dict: clear dict; tp_free(o);
835835
//
836836
// where ``transition`` is the binding transition ``__dealloc__`` used to run. The GC/dict steps
837-
// are guarded so they no-op where they do not apply, making one slot exact for all six carriers
838-
// (see ``TVMFFIPyTpDeallocSlot`` below for the per-step detail).
837+
// are guarded so they no-op where they do not apply, making one slot exact for all six carriers.
838+
//
839+
// CYTHON STEP MAP -- Cython's generated ``tp_dealloc`` (``generate_dealloc_function`` in
840+
// Cython/Compiler/ModuleNode.py) emits this fixed, guarded sequence. ``[N]`` tags mark the matching
841+
// lines in ``TVMFFIPyTpDeallocSlot``; each "no" is inapplicable to today's carriers and is enforced
842+
// by the layout guard in ``TVMFFIPyWrapDealloc``, so this map cannot silently drift:
843+
//
844+
// [0] typed self cast n/a slot works off Py_TYPE(o)/chandle, no typed field
845+
// [1] tp_finalize/__del__ no no carrier has __del__ (a heap subclass's runs in subtype_dealloc first)
846+
// [2] GC untrack YES guarded on PyObject_IS_GC(o)
847+
// [3] trashcan begin no needs_trashcan() false; the graph recurses through the C++
848+
// iterative deleter, not nested Python tp_dealloc
849+
// [4] weakref clear no no carrier is weak-referenceable (tp_weaklistoffset == 0)
850+
// [5] user __dealloc__ YES == the transition; runs FIRST, no resurrection bump (why this slot exists)
851+
// [6] __dict__ clear YES guarded on _PyObject_GetDictPtr (Function only)
852+
// [7] C++ member dtor YES folded into the transition's chandle release
853+
// [8] py-attr Py_CLEAR xN none carriers own no Python member but a GC type's dict ([6])
854+
// [9] base chain + tp_free YES tp_free(o) runs; no base chain (base is object / a carrier the
855+
// transition covers); Cython's trailing Py_DECREF(tp) is moot --
856+
// carrier types are immortal on FT (ob_ref_local == _Py_IMMORTAL_REFCNT_LOCAL)
857+
// [10] trashcan end no pairs with [3]
839858
//
840859
// Routing -- our slot always runs, but the call flow into it (and what ``o`` is) differs by how
841860
// the subtype was made; the difference is whether the base call is bound at COMPILE TIME or by a
@@ -872,20 +891,27 @@ inline void TVMFFIPyDeleteSpace(void* ptr) {
872891
// fires the C++ deleter cannot clobber an exception pending at wrapper death -- Cython's own
873892
// thunk fetches/restores around ``__dealloc__``, and replacing the thunk we must do the same.
874893
extern "C" inline void TVMFFIPyTpDeallocSlot(PyObject* o) {
894+
// [N] tags refer to the CYTHON STEP MAP in the section header (steps [1][3][4][10] absent by design).
895+
// [5] the transition runs FIRST, against the wrapper's true (un-bumped) refcount -- that is what makes
896+
// a concurrent TryIncRef fail on a dying wrapper. Bracketed by PyErr_Get/SetRaisedException so a
897+
// DecRef firing the C++ deleter cannot clobber an exception pending at wrapper death (as Cython's
898+
// thunk does too).
875899
PyObject* saved_exc = PyErr_GetRaisedException();
876-
TVMFFIPyTpDeallocImpl(TVMFFICyObjectGetCHandlePtr(o), o);
900+
TVMFFIPyTpDeallocImpl(TVMFFICyObjectGetCHandlePtr(o), o); // [5] (+ [7] C++ member dtor via chandle)
877901
PyErr_SetRaisedException(saved_exc);
878-
// GC untrack (Function + any GC subtype; idempotent no-op on the non-GC plain carriers and on
879-
// an already-untracked subtype). Must precede the free so the GC never walks freed memory.
902+
// [2] GC untrack (Function + any GC subtype; idempotent no-op on the non-GC plain carriers and on
903+
// an already-untracked subtype). Must precede the free so the GC never walks freed memory.
880904
if (PyObject_IS_GC(o)) PyObject_GC_UnTrack(o);
881-
// Generic ``__dict__`` teardown -- fires only for a type with a real ``tp_dictoffset``
882-
// (Function; ``dictptr == nullptr`` on the five plain carriers). Mirrors Cython's inlined
883-
// ``PyDict_Clear`` + ``Py_CLEAR`` exactly (a fixed-offset dict, not a managed dict).
905+
// [6] Generic ``__dict__`` teardown -- fires only for a type with a real ``tp_dictoffset``
906+
// (Function; ``dictptr == nullptr`` on the five plain carriers). Mirrors Cython's inlined
907+
// ``PyDict_Clear`` + ``Py_CLEAR`` exactly (a fixed-offset dict, not a managed dict).
884908
PyObject** dictptr = _PyObject_GetDictPtr(o);
885909
if (dictptr != nullptr && *dictptr != nullptr) {
886910
PyDict_Clear(*dictptr);
887911
Py_CLEAR(*dictptr);
888912
}
913+
// [9] reclaim memory. base-chain n/a (base is object / a carrier the transition covers); Cython's
914+
// trailing Py_DECREF(tp) is moot -- carrier types are immortal on FT (see map [9]).
889915
(*Py_TYPE(o)->tp_free)(o);
890916
}
891917

@@ -910,9 +936,19 @@ extern "C" inline void TVMFFIPyWrapDealloc(PyObject* type_obj) {
910936
// and a GC carrier's owned state must be exactly its dict (cleared above). A future owned member
911937
// breaks one of these -- fail loudly here instead of leaking at runtime.
912938
bool is_gc = PyType_HasFeature(tp, Py_TPFLAGS_HAVE_GC);
913-
if (!is_gc && tp->tp_dictoffset != 0) {
939+
if (!is_gc && tp->tp_dictoffset != 0) { // guards step [8]
914940
Py_FatalError("tvm_ffi: non-GC carrier gained a __dict__; hand-built tp_dealloc slot is stale");
915941
}
942+
// Enforce the two other STEP MAP omissions that have a checkable runtime field, so a future Cython
943+
// change that would need a step we drop fails loudly here instead of silently misbehaving:
944+
// step [1] -- the slot never runs tp_finalize; safe only while no carrier defines __del__.
945+
if (tp->tp_finalize != nullptr) {
946+
Py_FatalError("tvm_ffi: carrier gained tp_finalize (__del__); hand-built tp_dealloc slot omits it");
947+
}
948+
// step [4] -- the slot never clears weakrefs; safe only while no carrier is weak-referenceable.
949+
if (tp->tp_weaklistoffset != 0) {
950+
Py_FatalError("tvm_ffi: carrier became weak-referenceable; hand-built tp_dealloc slot omits weakref clear");
951+
}
916952
tp->tp_dealloc = &TVMFFIPyTpDeallocSlot;
917953
}
918954

0 commit comments

Comments
 (0)