Skip to content

Replace PyList_GET_ITEM usages on the free-threaded build #1434

@ngoldbaum

Description

@ngoldbaum

The C extension uses PyList_GET_ITEM in three spots:

± rg PyList_GET_ITEM
src/repository.c
602:        py_commit_oid = PyList_GET_ITEM(py_commit_oids, i);
1055:        py_parent = PyList_GET_ITEM(py_parents, i);
1138:        py_parent = PyList_GET_ITEM(py_parents, i);

This function returns a borrowed reference to the returned object, which can lead to use-after-free issues if the object is shared between threads and the list is mutated by another thread. I think that is possible here if a repository object is shared between threads.

The fix is to replace this function with PyList_GetItemRef, which returns a strong reference to the underlying object.

The implementation is using the macro variants of these function, which do no error checking, possibly for speed. If it turns out these calls are load-bearing for performance, we can explore other options too.

Note that you might be tempted to use e.g. Py_NewRef or Py_INCREF to convert the borrowed reference to an owned reference, but that isn't safe. It's only safe if you get a strong reference from the C API, since CPython can synchronize that internally.

See https://docs.python.org/3/howto/free-threading-extensions.html#borrowed-references and https://py-free-threading.github.io/porting-extensions/#unsafe-apis-returning-borrowed-references for more about this.

Ping @2bndy5

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions