gh-127582: Make object resurrection thread-safe for free threading.#127612
gh-127582: Make object resurrection thread-safe for free threading.#127612colesbury merged 4 commits intopython:mainfrom
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
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
Overall, the change LGTM, I just have a suggestion.
| 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 <colesbury@gmail.com>
|
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.
5 participants
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