Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.1k
Description
Feature or enhancement
We should make_Py_TryIncref public as function with the following signature:
intPyUnstable_TryIncref(PyObject*op);
EDIT: Renamed toPyUnstable_TryIncref in accordance with Victor's suggestion.
The function increments the reference count if it's not zero in a thread-safe way. It's logically equivalent to the following snippet and in the default (GIL-enabled) build it's implemented as such:
if (Py_REFCNT(op)>0) {Py_INCREF(op);return1; }return0;
Additionally, we should make_PyObject_SetMaybeWeakref public asPyUnstable_Object_EnableTryIncRef. This function has no equivalent in the GIL-enabled build (it's a no-op), but it's important for makingTryIncref work reliably with our biased reference counting implementation.
Motivation
TheTryIncref primitive is a building block for handling borrowed and unowned references. It addresses an issue that generally cannot be solved by adding extra synchronization like mutexes because it handles the race between the reference count reaching zero (which is outside developers' control) and theTryIncref.
We use it internally in three subsystems:
- To implementweak references
- Inasyncio to access the borrowed/unowned list of tasks
- In theMRO cache, to safely access the borrowed/unowned cached
PyObject *entries.
Recently, we discovered athread safety bug in pybind11 related to the use of borrowed/unowned references. Using_Py_TryIncref in place ofPy_INCREF would fix the bug. I think nanobind probably has a similar issue.
Alternatives
- Use actualweak reference objects instead of borrowed/unowned references. This is cleaner, but is not practical for performance reasons in the above use cases. Using
PyWeakRefobjects increases the overhead of pybind11 bindings by 30% in some simple tests. - Implement something like
_Py_TryIncrefin extensions. I think this is much worse than making the function public as an unstable API because it requires direct access to the reference count fields -- the implementation is tied to the implementation of biased reference counting -- and I'd like to avoid extensions depending directly on those details.
See also
- Add a public, fail-able
Py_INCREFfor the free-threaded build #113920 - [BUG]: Data race when using static variables with free-threaded Python pybind/pybind11#5489