Skip to content

Conversation

2bndy5
Copy link
Contributor

@2bndy5 2bndy5 commented Oct 15, 2025

resolves #1434

Instead use PyList_GetItemRef() for python >= 3.13. Internally, this function is conditionally defined for python < 3.13.

PyList_GetItemRef() is part of the Stable ABI but also propagate errors. Whereas the previous PyList_GET_ITEM() did not do any error checking and was not part of the Stable ABI.

@2bndy5

This comment was marked as resolved.

@2bndy5 2bndy5 marked this pull request as draft October 15, 2025 06:39
@2bndy5 2bndy5 force-pushed the replace-PyList_GET_ITEM branch 2 times, most recently from 90569a5 to 8ada517 Compare October 19, 2025 05:34
@2bndy5 2bndy5 marked this pull request as ready for review October 19, 2025 05:35
@jdavid
Copy link
Member

jdavid commented Oct 19, 2025

PyList_GetItem returns a borrowed reference (like PyList_GET_ITEM) while PyList_GetItemRef returns a new reference, so you have to choose one, in this case use always PyList_GetItem regardless of the Python version.

@2bndy5
Copy link
Contributor Author

2bndy5 commented Oct 19, 2025

#1435 describes why PyList_GetItemRef is preferred for thread-safety. Unfortunately, that function was only introduced in Python 3.13 as part of the Stable ABI.

We could use a compatibility layer (pythoncapi-compat library) to extend use of strong references prior to Python 3.13. I just thought this would be a suitable way to transition.

@jdavid
Copy link
Member

jdavid commented Oct 19, 2025

I think we should add an implementation of PyList_GetItemRef for versions older than 3.13, then use it.
Since this increases the reference count, you will need to decrease it once the reference is not needed anymore.

@2bndy5 2bndy5 force-pushed the replace-PyList_GET_ITEM branch from 8ada517 to 465b44b Compare October 19, 2025 20:18
@2bndy5
Copy link
Contributor Author

2bndy5 commented Oct 19, 2025

Ok. I've added the conditional definition of PyList_GetItemRef and calls to Py_DecRef(). I had to track the data ownership/lifetime to be sure when the strong ref was no longer needed. It seems that these call sites are just converting Python objects into C objects.

I'm not well accustomed to using the Python C API directly. So, thank you for reviewing this and giving feedback.

resolves libgit2#1434

Instead use [`PyList_GetItemRef()`] for python >= 3.13. Internally, this function is conditionally defined for python < 3.13.

[`PyList_GetItemRef()`] is part of the Stable ABI but also propagate errors. Whereas the previous [`PyList_GET_ITEM()`] did not do any error checking and was not part of the Stable ABI.

[`PyList_GetItemRef()`]: https://docs.python.org/3/c-api/list.html#c.PyList_GetItemRef
[`PyList_GET_ITEM()`]: https://docs.python.org/3/c-api/list.html#c.PyList_GET_ITEM
@2bndy5 2bndy5 force-pushed the replace-PyList_GET_ITEM branch from 465b44b to 48c178f Compare October 19, 2025 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace PyList_GET_ITEM usages on the free-threaded build

2 participants