From 93209dbe3ba07b928215b4e2de6b4f3c0f9b2a29 Mon Sep 17 00:00:00 2001 From: Christian Boos Date: Sat, 19 Sep 2026 11:25:00 +0200 Subject: [PATCH 1/2] odb backend: acquire the GIL in the callbacks The callbacks libgit2 calls for an OdbBackend implemented in Python called into Python without holding the GIL. That works when libgit2 is entered from the C extension, which keeps the GIL, but not from cffi, which releases it around every C call: Index.add on a repository with such a backend crashes the interpreter (access violation on Windows). Acquire the GIL with PyGILState_Ensure, like the filter callbacks do. Assisted-by: Claude Fable 5.1 --- src/odb_backend.c | 203 +++++++++++++++++++++++++-------------- test/test_odb_backend.py | 57 +++++++++++ 2 files changed, 186 insertions(+), 74 deletions(-) diff --git a/src/odb_backend.c b/src/odb_backend.c index 47d0a54e..18dd9ed2 100644 --- a/src/odb_backend.c +++ b/src/odb_backend.c @@ -56,39 +56,50 @@ typedef struct { PyObject *py_backend; } pgit_odb_backend; +/* + * The callbacks below may be called by libgit2 without the GIL: cffi releases + * it around every C call, and functions such as git_index_add_bypath end up + * here. So they must acquire it before calling into Python. + */ + static int pgit_odb_backend_read(void **ptr, size_t *sz, git_object_t *type, git_odb_backend *_be, const git_oid *oid) { pgit_odb_backend *be = (pgit_odb_backend *)_be; + PyObject *result = NULL; + int err = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject *py_oid = git_oid_to_python(oid); if (py_oid == NULL) - return GIT_EUSER; + goto done; - PyObject *result = PyObject_CallMethod(be->py_backend, "read_cb", "N", py_oid); - if (result == NULL) - return git_error_for_exc(); + result = PyObject_CallMethod(be->py_backend, "read_cb", "N", py_oid); + if (result == NULL) { + err = git_error_for_exc(); + goto done; + } const char *bytes; Py_ssize_t type_value; Py_ssize_t py_sz; - if (!PyArg_ParseTuple(result, "ny#", &type_value, &bytes, &py_sz) || !bytes) { - Py_DECREF(result); - return GIT_EUSER; - } + if (!PyArg_ParseTuple(result, "ny#", &type_value, &bytes, &py_sz) || !bytes) + goto done; *type = (git_object_t)type_value; *sz = (size_t)py_sz; *ptr = git_odb_backend_data_alloc(_be, *sz); - if (!*ptr) { - Py_DECREF(result); - return GIT_EUSER; - } + if (!*ptr) + goto done; memcpy(*ptr, bytes, *sz); - Py_DECREF(result); - return 0; + err = 0; + +done: + Py_XDECREF(result); + PyGILState_Release(gil); + return err; } static int @@ -101,34 +112,37 @@ pgit_odb_backend_read_prefix(git_oid *oid_out, void **ptr, size_t *sz, git_objec // Call callback pgit_odb_backend *be = (pgit_odb_backend *)_be; + int err = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject *result = PyObject_CallMethod(be->py_backend, "read_prefix_cb", "s#", short_id_hex, len); - if (result == NULL) - return git_error_for_exc(); + if (result == NULL) { + err = git_error_for_exc(); + goto done; + } // Parse output from callback PyObject *py_oid_out; Py_ssize_t type_value; Py_ssize_t py_sz; const char *bytes; - if (!PyArg_ParseTuple(result, "ny#O", &type_value, &bytes, &py_sz, &py_oid_out) || !bytes) { - Py_DECREF(result); - return GIT_EUSER; - } + if (!PyArg_ParseTuple(result, "ny#O", &type_value, &bytes, &py_sz, &py_oid_out) || !bytes) + goto done; *type = (git_object_t)type_value; *sz = (size_t)py_sz; *ptr = git_odb_backend_data_alloc(_be, *sz); - if (!*ptr) { - Py_DECREF(result); - return GIT_EUSER; - } + if (!*ptr) + goto done; memcpy(*ptr, bytes, *sz); - size_t oid_len = py_oid_to_git_oid(py_oid_out, oid_out); - Py_DECREF(result); - if (oid_len == 0) - return GIT_EUSER; - return 0; + if (py_oid_to_git_oid(py_oid_out, oid_out) == 0) + goto done; + err = 0; + +done: + Py_XDECREF(result); + PyGILState_Release(gil); + return err; } static int @@ -136,24 +150,30 @@ pgit_odb_backend_read_header(size_t *len, git_object_t *type, git_odb_backend *_be, const git_oid *oid) { pgit_odb_backend *be = (pgit_odb_backend *)_be; + PyObject *result = NULL; + int err = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject *py_oid = git_oid_to_python(oid); if (py_oid == NULL) - return GIT_EUSER; + goto done; - PyObject *result = PyObject_CallMethod(be->py_backend, "read_header_cb", "N", py_oid); - if (result == NULL) - return git_error_for_exc(); + result = PyObject_CallMethod(be->py_backend, "read_header_cb", "N", py_oid); + if (result == NULL) { + err = git_error_for_exc(); + goto done; + } Py_ssize_t type_value; - if (!PyArg_ParseTuple(result, "nn", &type_value, len)) { - Py_DECREF(result); - return GIT_EUSER; - } + if (!PyArg_ParseTuple(result, "nn", &type_value, len)) + goto done; *type = (git_object_t)type_value; + err = 0; - Py_DECREF(result); - return 0; +done: + Py_XDECREF(result); + PyGILState_Release(gil); + return err; } static int @@ -161,34 +181,50 @@ pgit_odb_backend_write(git_odb_backend *_be, const git_oid *oid, const void *data, size_t sz, git_object_t typ) { pgit_odb_backend *be = (pgit_odb_backend *)_be; + PyObject *result = NULL; + int err = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject *py_oid = git_oid_to_python(oid); if (py_oid == NULL) - return GIT_EUSER; + goto done; - PyObject *result = PyObject_CallMethod(be->py_backend, "write_cb", "Ny#n", py_oid, data, sz, typ); - if (result == NULL) - return git_error_for_exc(); + result = PyObject_CallMethod(be->py_backend, "write_cb", "Ny#n", py_oid, data, sz, typ); + if (result == NULL) { + err = git_error_for_exc(); + goto done; + } + err = 0; - Py_DECREF(result); - return 0; +done: + Py_XDECREF(result); + PyGILState_Release(gil); + return err; } static int pgit_odb_backend_exists(git_odb_backend *_be, const git_oid *oid) { pgit_odb_backend *be = (pgit_odb_backend *)_be; + PyObject *result = NULL; + int r = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject *py_oid = git_oid_to_python(oid); if (py_oid == NULL) - return GIT_EUSER; + goto done; + + result = PyObject_CallMethod(be->py_backend, "exists_cb", "N", py_oid); + if (result == NULL) { + r = git_error_for_exc(); + goto done; + } - PyObject *result = PyObject_CallMethod(be->py_backend, "exists_cb", "N", py_oid); - if (result == NULL) - return git_error_for_exc(); + r = PyObject_IsTrue(result); - int r = PyObject_IsTrue(result); - Py_DECREF(result); +done: + Py_XDECREF(result); + PyGILState_Release(gil); return r; } @@ -202,23 +238,33 @@ pgit_odb_backend_exists_prefix(git_oid *out, git_odb_backend *_be, // Call callback pgit_odb_backend *be = (pgit_odb_backend *)_be; + int err = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject *py_oid = PyObject_CallMethod(be->py_backend, "exists_prefix_cb", "s#", short_id_hex, len); - if (py_oid == NULL) - return git_error_for_exc(); + if (py_oid == NULL) { + err = git_error_for_exc(); + goto done; + } - size_t oid_len = py_oid_to_git_oid(py_oid, out); - Py_DECREF(py_oid); - if (oid_len == 0) - return GIT_EUSER; - return 0; + if (py_oid_to_git_oid(py_oid, out) == 0) + goto done; + err = 0; + +done: + Py_XDECREF(py_oid); + PyGILState_Release(gil); + return err; } static int pgit_odb_backend_refresh(git_odb_backend *_be) { pgit_odb_backend *be = (pgit_odb_backend *)_be; + PyGILState_STATE gil = PyGILState_Ensure(); PyObject_CallMethod(be->py_backend, "refresh_cb", NULL); - return git_error_for_exc(); + int err = git_error_for_exc(); + PyGILState_Release(gil); + return err; } static int @@ -226,43 +272,52 @@ pgit_odb_backend_foreach(git_odb_backend *_be, git_odb_foreach_cb cb, void *payload) { PyObject *item; + PyObject *iterator = NULL; git_oid oid; pgit_odb_backend *be = (pgit_odb_backend *)_be; + int err = GIT_EUSER; + PyGILState_STATE gil = PyGILState_Ensure(); /* Call the Python __iter__ method directly. PyObject_GetIter would invoke * the C tp_iter slot (OdbBackend_as_iter), which calls this function back * and causes infinite recursion for Python backends. */ PyObject *iter_method = PyObject_GetAttrString((PyObject *)be->py_backend, "__iter__"); - if (iter_method == NULL) - return git_error_for_exc(); + if (iter_method == NULL) { + err = git_error_for_exc(); + goto done; + } - PyObject *iterator = PyObject_CallObject(iter_method, NULL); + iterator = PyObject_CallObject(iter_method, NULL); Py_DECREF(iter_method); - if (iterator == NULL) - return git_error_for_exc(); + if (iterator == NULL) { + err = git_error_for_exc(); + goto done; + } while ((item = PyIter_Next(iterator))) { size_t len = py_oid_to_git_oid(item, &oid); Py_DECREF(item); - if (len == 0) { - Py_DECREF(iterator); - return GIT_EUSER; - } - if (cb(&oid, payload) != 0) { - Py_DECREF(iterator); - return GIT_EUSER; - } + if (len == 0) + goto done; + if (cb(&oid, payload) != 0) + goto done; } - Py_DECREF(iterator); - return git_error_for_exc(); + err = git_error_for_exc(); + +done: + Py_XDECREF(iterator); + PyGILState_Release(gil); + return err; } static void pgit_odb_backend_free(git_odb_backend *backend) { pgit_odb_backend *custom_backend = (pgit_odb_backend *)backend; + PyGILState_STATE gil = PyGILState_Ensure(); Py_DECREF(custom_backend->py_backend); + PyGILState_Release(gil); } int diff --git a/test/test_odb_backend.py b/test/test_odb_backend.py index 43a01d0c..17275a8d 100644 --- a/test/test_odb_backend.py +++ b/test/test_odb_backend.py @@ -27,6 +27,8 @@ # Standard Library import binascii +import subprocess +import sys from collections.abc import Generator, Iterator from pathlib import Path @@ -277,3 +279,58 @@ def test_foreach_cb_bad_oid(barerepo: Repository) -> None: odb.add_backend(backend, 1) with pytest.raises(pygit2.InvalidError): next(iter(odb)) + + +# +# Test a custom object backend called back from a cffi function. +# + +INDEX_ADD_SCRIPT = """ +import sys +import pygit2 +from pygit2.enums import ObjectType + + +class MemoryBackend(pygit2.OdbBackend): + def __init__(self): + super().__init__() + self.objects = {} + + def read_cb(self, oid): + return ObjectType.BLOB, self.objects[oid] + + def read_prefix_cb(self, prefix): + raise KeyError(prefix) + + def read_header_cb(self, oid): + return ObjectType.BLOB, len(self.objects[oid]) + + def exists_cb(self, oid): + return oid in self.objects + + def exists_prefix_cb(self, prefix): + raise KeyError(prefix) + + def refresh_cb(self): + pass + + def write_cb(self, oid, data, typ): + self.objects[oid] = data + + +repo = pygit2.Repository(sys.argv[1]) +backend = MemoryBackend() +repo.odb.add_backend(backend, 100) +repo.index.add('hello.txt') +assert backend.objects[repo.index['hello.txt'].id] == b'hello' +""" + + +def test_index_add(testrepo: Repository) -> None: + # Index.add calls libgit2 through cffi, which releases the GIL: the backend + # callbacks must acquire it. Without the GIL the interpreter crashes, so + # run in a subprocess. + (Path(testrepo.workdir) / 'hello.txt').write_bytes(b'hello') + subprocess.run( + [sys.executable, '-c', INDEX_ADD_SCRIPT, testrepo.workdir], check=True + ) From 663d3d793d0216f17294a3495848ee329abb4fe2 Mon Sep 17 00:00:00 2001 From: Christian Boos Date: Sat, 19 Sep 2026 11:29:08 +0200 Subject: [PATCH 2/2] odb backend: pass the object type to write_cb as an int The type was passed to PyObject_CallMethod with the "n" format, which reads a Py_ssize_t, but git_object_t is an int. On 64-bit Windows write_cb received garbage in the upper 32 bits, e.g. 2753074036739 (0x28100000003) instead of 3 for a blob. Assisted-by: Claude Fable 5.1 --- src/odb_backend.c | 2 +- test/test_odb_backend.py | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/odb_backend.c b/src/odb_backend.c index 18dd9ed2..82322c2f 100644 --- a/src/odb_backend.c +++ b/src/odb_backend.c @@ -189,7 +189,7 @@ pgit_odb_backend_write(git_odb_backend *_be, const git_oid *oid, if (py_oid == NULL) goto done; - result = PyObject_CallMethod(be->py_backend, "write_cb", "Ny#n", py_oid, data, sz, typ); + result = PyObject_CallMethod(be->py_backend, "write_cb", "Ny#i", py_oid, data, sz, (int)typ); if (result == NULL) { err = git_error_for_exc(); goto done; diff --git a/test/test_odb_backend.py b/test/test_odb_backend.py index 17275a8d..4a084c10 100644 --- a/test/test_odb_backend.py +++ b/test/test_odb_backend.py @@ -334,3 +334,25 @@ def test_index_add(testrepo: Repository) -> None: subprocess.run( [sys.executable, '-c', INDEX_ADD_SCRIPT, testrepo.workdir], check=True ) + + +class WriteBackend(pygit2.OdbBackend): + def __init__(self) -> None: + super().__init__() + self.written: list[tuple[Oid, bytes, int]] = [] + + def exists_cb(self, oid: Oid | str) -> bool: + return False + + def refresh_cb(self) -> None: + pass + + def write_cb(self, oid: Oid, data: bytes, typ: int) -> None: + self.written.append((oid, data, typ)) + + +def test_write_cb(testrepo: Repository) -> None: + backend = WriteBackend() + testrepo.odb.add_backend(backend, 100) + oid = testrepo.create_blob(b'hello') + assert backend.written == [(oid, b'hello', ObjectType.BLOB)]