Add PyTuple_GetItemRef(), similar to PyTuple_GetItem() but return a strong reference

[vstinner]

API: PyObject* PyTuple_GetItemRef(PyObject *p, Py_ssize_t pos).

While Free Threading (PEP 703) doesn't need this function, I propose to add functions which return strong references to proactively promote the usage of strong references.

Borrowed references have a long history of bugs and crashes. Usually, it's hard to trigger them, but if you're affected, it's hard to workaround them. The fix is usually to use a strong reference.

Example from _asyncio.Future.remove_done_callback holding explicitly a strong reference to prevent a possible crash:

        Py_INCREF(item);
        ret = PyObject_RichCompareBool(PyTuple_GET_ITEM(item, 0), fn, Py_EQ);
        Py_DECREF(item);

The code explicitly holds a strong reference to the item tuple while calling PyObject_RichCompareBool(), since PyObject_RichCompareBool() can call arbitrary Python code in __eq__() methods.

These INCREF/DECREF were added by https://github.com/python/cpython/commit/bca4939d806170c3ca5d05f23710d11a8f1669cf "Fixed miscellaneous errors in asyncio speedup module".

[gvanrossum]

I feel that promoting good hygiene is not a sufficient reason to add a new API, especially one that can already be expressed as PySequence_GetItem.

[vstinner]

I feel that promoting good hygiene is not a sufficient reason to add a new API, especially one that can already be expressed as PySequence_GetItem.

Does it look useful to add a sentence to such functions to clearly mention that they return borrowed references, and suggest an alternative returning strong references?

For example, for PyTuple_GetItem:

Return a borrowed reference. Use Py_NewRef() or PySequence_GetItem()
to get a strong reference.

[encukou]

IMO it would be useful to say what the reference is borrowed from, something like:

Return a borrowed reference. The reference is borrowed from *p* (that is: it is only valid
as long as you hold a reference to *p*).

To get a strong reference, use `PySequence_GetItem(...)` or `Py_NewRef(PyTuple_GetItem(...))`.

[vstinner]

IMO it would be useful to say what the reference is borrowed from, something like:

Good idea. I created https://github.com/python/cpython/pull/117920 for the doc.

[encukou]

Let's (gradually) start using this kind of wording for borrowed references throughout the docs.

• “The returned reference is borrowed from p” -- the lender is crucial information
• “that is” -- emphasizes that “borrowed from p” and “only valid as long as you hold a reference to p” are equivalent
• “only valid as long as you hold a reference to p” -- IMO, this should be spelled out rather than e.g. hidden behind a glossary link.
• “To get a strong reference” -- there should always be an alternative that gives you a strong ref.

[erlend-aasland]

@encukou: I think that is a good idea. Let's make sure we get the first doc amendment right, so it will be a good example to follow.

[vstinner]

I feel that promoting good hygiene is not a sufficient reason to add a new API, especially one that can already be expressed as PySequence_GetItem.

It's not the first time that I propose adding PyTuple_GetItemRef(). The consensus is against adding such function.

I will continue the road of better documentation, it cannot hurt :-) I close this issue.