gh-149216: Notify type watchers on heap type deallocation#149236
gh-149216: Notify type watchers on heap type deallocation#149236markshannon merged 7 commits intopython:mainfrom
Conversation
When a watched heap type is deallocated, type watcher callbacks were never invoked. The JIT optimizer relies on type watchers plus pointer comparisons on watched types; if a type is freed and a new type is allocated at the same address, stale JIT code could crash. Call the registered watcher callbacks from type_dealloc(), using the same _PyObject_ResurrectStart/_PyObject_ResurrectEnd pattern that dict_dealloc() already uses. The notification happens before any teardown, so callbacks can safely inspect the type object.
Documentation build overview
54 files changed ·
|
| i++; | ||
| bits >>= 1; | ||
| } | ||
| if (_PyObject_ResurrectEnd(self)) { |
There was a problem hiding this comment.
| if (_PyObject_ResurrectEnd(self)) { | |
| assert(Py_REFCNT(self) == 0); |
Watchers aren't allowed to make changes like this.
There was a problem hiding this comment.
Done, replaced with the assert. Thanks for the review.
| i++; | ||
| bits >>= 1; | ||
| } | ||
| assert(Py_REFCNT(self) == 0); |
There was a problem hiding this comment.
Sorry to keep changing my mind, but now I think about this, it might not be safe to pass type to the watcher with refcount == 0.
The docs say very little about what a watcher can or cannot do.
So I think we need to increase the refcount to 1 before calling the watchers, then reduce it by one (not using Py_DECREF to avoid recursion), then finally check if it has been resurrected, as you did before.
There was a problem hiding this comment.
Reverted the changes.
3 participants
Summary
type_dealloc()before teardown, using the_PyObject_ResurrectStart/_PyObject_ResurrectEndresurrection pattern (same asdict_dealloc())_testcapi/watchers.cthat records the type's name as a string (avoids resurrecting the type during dealloc testing)test_watch_type_deallocandtest_watch_type_dealloc_errortestsPyType_WatchCallbackmay be called during deallocationFixes #149216
CC: @markshannon
PyType_Watchdoes not report deallocations #149216