Commit 38ca72d
committed
[FEAT][Python] Tie Python wrapper lifetime to underlying C++ FFI object
Make `a.x is a.x`, `id(a.x)` stable across drop+refetch, and `f(x) is x`
for FFI returns, on both the GIL and free-threaded (3.14t) builds, by
binding one Python wrapper to one C++ FFI object ("chandle") for the
object's lifetime. Implements the PyObjectTying design.
## Allocation layout
A two-layer custom-allocator hook lives in core libtvm_ffi.so:
`TVMFFIObjectAllocHeader { delete_space }`, `TVMFFICustomAllocator
{ allocate, context }`, plus `TVMFFIGetCustomAllocator` /
`TVMFFISetCustomAllocator`. libtvm_ffi installs a builtin default at
registry init, so every Object always carries the 8-byte base header and
`TVMFFIGetCustomAllocator` never returns NULL. The Python Cython module
overrides the global default at module load with `TVMFFIPyAllocate`,
which prepends a 16-byte `PyCustomAllocHeader` encoding the wrapper
binding. `make_object` / `make_inplace_array_object` / `PyClassDeleter`
and the Rust `ObjectArc::new[_with_extra_items]` paths all funnel through
the registry, so Python-defined types and Rust-allocated objects share
the layout and lifetime semantics.
## Binding state machine
State is concentrated in `tvm_ffi_python_helpers.h`. Each header word
(`tagged_pyobj`) holds the wrapper back-pointer plus low tag bits, giving
a four-state machine:
Detached: no wrapper bound
Active: wrapper bound and owns a +1 on the chandle
Inactive: wrapper dead but its allocation is cached for in-place revival
InTransit: a dealloc is mid-flight (handshake bit)
Every FFI return funnels through `make_ret_object` (C++ entry
`TVMFFIPyMakeRetObject`), which returns the canonical wrapper for a
chandle when one exists, reviving an Inactive cached allocation in place
so a re-fetched wrapper keeps a stable `id()` at the same address. The
cache-vs-free handshake spans three slots -- a pre-bump `tp_dealloc`
opens it, `tp_free` settles it, and the C++ weak deleter
(`TVMFFIPyDeleteSpace`) reclaims the block -- coordinated so a chandle
that outlives its wrapper keeps the cached bytes, and a genuinely dead
chandle frees them exactly once. Frontend-allocation is detected by
`delete_space` pointer comparison (`TVMFFIPyIsCanonical`), avoiding a
flag bit on `TVMFFIObject`; chandles created before the Python allocator
is registered (statically-initialized global functions in libtvm_ffi.so)
carry only the base header and are skipped.
## Free-threaded build
The same tying runs on Py_GIL_DISABLED. The `tagged_pyobj` word doubles
as a per-word spin-lock (a Locked tag bit, `__atomic_*` CAS) that
serializes every binding transition; the lock is held only across short,
park-free word/header edits, while alloc/revival run lock-released. The
wait/back-off (`TVMFFIPyLockYield`) detaches the thread state so a
concurrent stop-the-world GC is never starved.
The free-threaded revival hazard is Cython's generated `tp_dealloc`,
which bumps the wrapper refcount before running `__dealloc__`: that bump
makes `PyUnstable_TryIncRef` spuriously succeed on a wrapper being torn
down, so a concurrent `make_ret` Active-hit could revive a corpse
(borrowed-ref UAF). The fix replaces Cython's `tp_dealloc` on each cdef
CObject-family carrier with one hand-built `TVMFFIPyTpDeallocSlot` that
runs the binding transition (bracketed by
`PyErr_Get/SetRaisedException`) before any bump, then -- stripped of the
now-dead bump -- GC-untrack (guarded by GC-ness), a generic `__dict__`
clear (guarded by a real `tp_dictoffset`), and `tp_free`. A plain
carrier runs exactly `transition; tp_free`; Function fires both guards;
both are faithful to Cython's originals minus the bump. The six carriers
(CObject, CContainerBase, OpaquePyObject, Error, Tensor, Function) are a
closed compile-time set, each wrapped once at init via
`TVMFFIPyWrapDealloc`; every heap subtype is covered for free through
`subtype_dealloc`'s base-walk to the nearest carrier. An import-time
layout guard `Py_FatalError`s if a non-GC carrier ever gains a
`__dict__`, turning silent owned-member drift into a loud failure.
Active-hit revival uses `PyUnstable_TryIncRef` / `EnableTryIncRef` to
close the borrowed-read UAF. The GIL build is byte-identical: all
free-threaded machinery is behind `#ifdef Py_GIL_DISABLED`, and on the
GIL build the post-bump `__dealloc__` hook runs the transition directly
(safe under the GIL) while the slot installer is a no-op stub.
## Robustness
A double-free in the builtin allocator is fixed along the way: allocate
offset the body by `round_up(header, alignment)` but free subtracted a
fixed `sizeof(header)`, symmetric only for alignment <= 8 -- a 16-aligned
reflection dataclass (alignof(max_align_t)=16) was freed 8 bytes early.
Both sides now use a fixed `alignof(max_align_t)` body offset. A shutdown
guard (`TVMFFIPyMarkPythonFinalizing`, wired to atexit) flips an
atomic flag read before any `PyGILState_Ensure` to avoid acquiring the
GIL after Python finalization has begun; wrapper bytes on chandles still
alive at exit are intentionally leaked.
## `_move()` semantics
Under universal cache-on, callback args alias the caller's wrapper and
FFI returns of the same chandle alias the caller's wrapper (one wrapper,
one chandle ref). `_move()` is kept as an API: the rvalue setter on
either side eager-detaches the canonical binding before the C++
`AnyViewToOwnedAny` transfer nulls the source chandle.
## Tests
New `tests/python/test_pyobject_tying.py` covers Active/Inactive/InTransit
transitions, cache-on aliasing, `_move()` under cache-on, pickle stress,
threading stress, GC integration, multi-chandle isolation, the weakref
limitation, free-threaded concurrent carrier-type stress
(Function/Error/multi-level-heap), and OpaquePyObject roundtrip/leak.
`test_function.py::test_rvalue_ref` is refactored for the new
aliasing-aware use_count expectations. Verified: FT crash oracle clean,
FT suite 2321 passed, GIL suite 2346 passed, tying tests 43/43 on both
builds; Rust suite passes.1 parent 807b17a commit 38ca72d
18 files changed
Lines changed: 2432 additions & 130 deletions
File tree
- include/tvm/ffi
- python/tvm_ffi/cython
- rust
- tvm-ffi-sys/src
- tvm-ffi/src
- src/ffi
- extra
- tests/python
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
| 73 | + | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
| |||
274 | 275 | | |
275 | 276 | | |
276 | 277 | | |
277 | | - | |
278 | | - | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
291 | 283 | | |
292 | 284 | | |
293 | 285 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
582 | 582 | | |
583 | 583 | | |
584 | 584 | | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
585 | 645 | | |
586 | 646 | | |
587 | 647 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| 30 | + | |
30 | 31 | | |
31 | 32 | | |
32 | 33 | | |
| |||
79 | 80 | | |
80 | 81 | | |
81 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
82 | 116 | | |
83 | 117 | | |
84 | 118 | | |
| |||
168 | 202 | | |
169 | 203 | | |
170 | 204 | | |
171 | | - | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
172 | 210 | | |
173 | 211 | | |
174 | 212 | | |
| |||
187 | 225 | | |
188 | 226 | | |
189 | 227 | | |
190 | | - | |
| 228 | + | |
| 229 | + | |
191 | 230 | | |
192 | 231 | | |
193 | 232 | | |
| |||
215 | 254 | | |
216 | 255 | | |
217 | 256 | | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
218 | 260 | | |
219 | 261 | | |
220 | 262 | | |
221 | 263 | | |
222 | 264 | | |
223 | | - | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
224 | 268 | | |
225 | 269 | | |
226 | 270 | | |
| |||
239 | 283 | | |
240 | 284 | | |
241 | 285 | | |
242 | | - | |
| 286 | + | |
| 287 | + | |
243 | 288 | | |
244 | 289 | | |
245 | 290 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1141 | 1141 | | |
1142 | 1142 | | |
1143 | 1143 | | |
| 1144 | + | |
| 1145 | + | |
| 1146 | + | |
| 1147 | + | |
| 1148 | + | |
| 1149 | + | |
| 1150 | + | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
1144 | 1154 | | |
1145 | 1155 | | |
1146 | 1156 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
232 | 232 | | |
233 | 233 | | |
234 | 234 | | |
235 | | - | |
236 | | - | |
237 | | - | |
238 | | - | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
239 | 245 | | |
240 | 246 | | |
241 | 247 | | |
242 | | - | |
243 | 248 | | |
244 | 249 | | |
245 | 250 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
361 | 361 | | |
362 | 362 | | |
363 | 363 | | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
364 | 379 | | |
365 | 380 | | |
366 | 381 | | |
| |||
542 | 557 | | |
543 | 558 | | |
544 | 559 | | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
545 | 566 | | |
546 | 567 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
122 | 122 | | |
123 | 123 | | |
124 | 124 | | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
125 | 129 | | |
126 | 130 | | |
127 | 131 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
545 | 545 | | |
546 | 546 | | |
547 | 547 | | |
548 | | - | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
549 | 557 | | |
550 | | - | |
| 558 | + | |
551 | 559 | | |
552 | 560 | | |
553 | 561 | | |
| |||
1075 | 1083 | | |
1076 | 1084 | | |
1077 | 1085 | | |
| 1086 | + | |
| 1087 | + | |
| 1088 | + | |
| 1089 | + | |
1078 | 1090 | | |
1079 | 1091 | | |
1080 | 1092 | | |
| |||
1089 | 1101 | | |
1090 | 1102 | | |
1091 | 1103 | | |
| 1104 | + | |
1092 | 1105 | | |
1093 | 1106 | | |
1094 | 1107 | | |
| |||
0 commit comments