gh-127582: Make object resurrection thread-safe for free threading. #127612
Conversation
…ing. Objects may be temporarily "resurrected" in destructors when calling finalizers or watcher callbacks. We previously undid the resurrection by decrementing the reference count using `Py_SET_REFCNT`. This was not thread-safe because other threads might be accessing the object (modifying its reference count) if it was exposed by the finalizer, watcher callback, or temporarily accessed by a racy dictionary or list access. This adds internal-only thread-safe functions for temporary object resurrection during destructors.
colesbury
added
the
🔨 test-with-refleak-buildbots
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit a7b41c8 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
| return 0; | ||
| } | ||
| // Slow-path: object has a shared refcount or is not owned by this thread | ||
| return _PyObject_ResurrectEndSlow(op); |
There was a problem hiding this comment.
I presume the slow path isn't inline so that compiler will inline the fast path and code will be smaller?
There was a problem hiding this comment.
Yeah, I think that exact split of what's in the "fast-path" vs. "slow-path" doesn't matter too much here, but it would be unwieldy to try to put everything in the inline function. In particular, _Py_DecRefSharedIsDead is big and not exposed outside of object.c.
Include/internal/pycore_object.h
Outdated
| // Temporarily resurrects an object during deallocation. The refcount is set | ||
| // to one. | ||
| static inline void | ||
| _PyObject_Resurrect(PyObject *op) |
There was a problem hiding this comment.
How about naming it something like _PyObject_ResurrectStart? That would make its corresponding _PyObject_ResurrectEnd more obvious or maybe change _PyObject_ResurrectEnd -> _PyObject_UnResurrect?
There was a problem hiding this comment.
I changed it to _PyObject_ResurrectStart
colesbury
added
needs backport to 3.13
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Sorry, @colesbury, I could not cleanly backport this to |
… threading. (pythonGH-127612) Objects may be temporarily "resurrected" in destructors when calling finalizers or watcher callbacks. We previously undid the resurrection by decrementing the reference count using `Py_SET_REFCNT`. This was not thread-safe because other threads might be accessing the object (modifying its reference count) if it was exposed by the finalizer, watcher callback, or temporarily accessed by a racy dictionary or list access. This adds internal-only thread-safe functions for temporary object resurrection during destructors. (cherry picked from commit f4f5308) Co-authored-by: Sam Gross <[email protected]>
|
GH-127659 is a backport of this pull request to the 3.13 branch. |
…ding. (GH-127612) (GH-127659) Objects may be temporarily "resurrected" in destructors when calling finalizers or watcher callbacks. We previously undid the resurrection by decrementing the reference count using `Py_SET_REFCNT`. This was not thread-safe because other threads might be accessing the object (modifying its reference count) if it was exposed by the finalizer, watcher callback, or temporarily accessed by a racy dictionary or list access. This adds internal-only thread-safe functions for temporary object resurrection during destructors. (cherry picked from commit f4f5308)
…ing. (pythonGH-127612) Objects may be temporarily "resurrected" in destructors when calling finalizers or watcher callbacks. We previously undid the resurrection by decrementing the reference count using `Py_SET_REFCNT`. This was not thread-safe because other threads might be accessing the object (modifying its reference count) if it was exposed by the finalizer, watcher callback, or temporarily accessed by a racy dictionary or list access. This adds internal-only thread-safe functions for temporary object resurrection during destructors.
Objects may be temporarily "resurrected" in destructors when calling finalizers or watcher callbacks. We previously undid the resurrection by decrementing the reference count using
Py_SET_REFCNT. This was not thread-safe because other threads might be accessing the object (modifying its reference count) if it was exposed by the finalizer, watcher callback, or temporarily accessed by a racy dictionary or list access.This adds internal-only thread-safe functions for temporary object resurrection during destructors.
Py_SET_REFCNTin destructors #127582