Make _Py_TryIncref public as an unstable API as PyUnstable_TryIncref()

Feature or enhancement

We should make _Py_TryIncref public as function with the following signature:

int PyUnstable_TryIncref(PyObject *op);

EDIT: Renamed to PyUnstable_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);
        return 1;
    }
    return 0;

Additionally, we should make _PyObject_SetMaybeWeakref public as PyUnstable_Object_EnableTryIncRef. This function has no equivalent in the GIL-enabled build (it's a no-op), but it's important for making TryIncref work reliably with our biased reference counting implementation.

Motivation

The TryIncref 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 the TryIncref.

We use it internally in three subsystems:

To implement weak references
In asyncio to access the borrowed/unowned list of tasks
In the MRO cache, to safely access the borrowed/unowned cached PyObject * entries.

Recently, we discovered a thread safety bug in pybind11 related to the use of borrowed/unowned references. Using _Py_TryIncref in place of Py_INCREF would fix the bug. I think nanobind probably has a similar issue.

Alternatives

Use actual weak reference objects instead of borrowed/unowned references. This is cleaner, but is not practical for performance reasons in the above use cases. Using PyWeakRef objects increases the overhead of pybind11 bindings by 30% in some simple tests.
Implement something like _Py_TryIncref in 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.

Linked PRs

gh-128844: Make _Py_TryIncref public as an unstable API. #128926

@encukou

cc @encukou @vstinner

Since Py_INCREF() and Py_NewRef() omit "PyObject" in their name, I suggest PyUnstable_TryIncref() name for the new function.

Additionally, we should make _PyObject_SetMaybeWeakref public as PyUnstable_Object_EnableTryIncRef.

Would you mind to elaborate this function? What is its API? What is its usage?

One thing is not clear to me.
Once the refcount hits zero, the object can start getting deallocated, and the memory can be reused for something else, right? If that happens, and the memory that was the refcount now holds an unrelated nonzero integer, couldn't PyUnstable_Object_TryIncref increment that integer?

I suspect there's a mechanism to prevent that in free-threaded builds, but I'd like to know what it is and how it applies.

Would you mind to elaborate this function? What is its API? What is its usage?

When called on an object, PyUnstable_Object_EnableTryIncRef allows subsequent calls to PyUnstable_TryIncref. Without it, calls to PyUnstable_TryIncref() may spuriously return 0 in the free threading build. The implementation is a no-op in the default build. Like most Python C API functions, you must hold a strong reference to the object when calling PyUnstable_Object_EnableTryIncRef, so it can't be folded into PyUnstable_TryIncref(), which can take a borrowed/unowned reference.

The API is a bit of a "wart", but I don't see a way to avoid it.

// Enable subsequent calls to `PyUnstable_TryIncref` on `op`
void PyUnstable_Object_EnableTryIncRef(PyObject *op);

Once the refcount hits zero, the object can start getting deallocated, and the memory can be reused for something else, right?

In the asyncio and pybind11 use cases, there are already locks during deallocation and around the _Py_TryIncref() call, so the object isn't deallocated yet. Avoiding locking (like in our dictionary implementation) is out of scope for now -- it would need some extra APIs to be exposed.

Here's an example in psuedo-code based on pybind11's use case. Note that when TryIncref() is called, any concurrent deallocation can't have gone past the lock acquisition in the first line of wrapper_dealloc. (Otherwise, the wrapper would have already been removed from registered_instances).

The TryIncref() is still needed (in the free threading build) so that we don't incref an object with zero refcount that is subsequently, unconditionally deallocated.

// map from C++ pointers to Python wrapper objects
std::unordered_map<void *, PyObject *> registered_instances;
std::mutex mutex;


// Find an existing Python wrapper for a C++ pointer
PyObject *
get_wrapper(void *ptr)
{
    std::unique_lock<std::mutex> lk(mutex);
    if (auto it = registered_instances.find(ptr); it != registered_instances.end()) {
        PyObject *wrapper = it->second;
        if (_Py_TryIncref(wrapper)) { // NOTE: `Py_INCREF()` here would not be safe in the free threading build!
            return wrapper;
        }
    }
}

void
wrapper_dealloc(PyObject *wrapper)
{
    std::unique_lock<std::mutex> lk(mutex);
    void *ptr = get_cpp_ptr_from_wrapper(wrapper);
    registered_instances.erase(ptr);

    ...
}

Ok, I'm fine with adding these two PyUnstable functions.

Thank you for the example! It makes sense to me as well. +1.

pythongh-128844: Make `_Py_TryIncref` public as an unstable API.

gh-128844: Make `_Py_TryIncref` public as an unstable API. (#128926)

Navigation Menu

Repository navigation

Make `_Py_TryIncref` public as an unstable API as `PyUnstable_TryIncref()` #128844

Feature or enhancement

Motivation

Alternatives

See also

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Make _Py_TryIncref public as an unstable API as PyUnstable_TryIncref() #128844

Description

Feature or enhancement

Motivation

Alternatives

See also

Linked PRs

Activity

corona10 commented on Jan 15, 2025

vstinner commented on Jan 15, 2025

encukou commented on Jan 15, 2025

colesbury commented on Jan 15, 2025

vstinner commented on Jan 15, 2025

encukou commented on Jan 16, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Make `_Py_TryIncref` public as an unstable API as `PyUnstable_TryIncref()` #128844