From 6069680c8a0ec61b60c945099adb25db80ab3970 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 9 Jul 2024 08:20:03 +0100 Subject: [PATCH 1/2] use FFI calls for refcounting on all abi3 versions --- newsfragments/4324.changed.md | 1 + pyo3-ffi/src/object.rs | 85 ++++++++++++++--------------------- 2 files changed, 34 insertions(+), 52 deletions(-) create mode 100644 newsfragments/4324.changed.md diff --git a/newsfragments/4324.changed.md b/newsfragments/4324.changed.md new file mode 100644 index 00000000000..2818159dda3 --- /dev/null +++ b/newsfragments/4324.changed.md @@ -0,0 +1 @@ +Use FFI function calls for reference counting on all abi3 versions. diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index 7acd0897217..5055d146ee5 100644 --- a/pyo3-ffi/src/object.rs +++ b/pyo3-ffi/src/object.rs @@ -509,35 +509,23 @@ extern "C" { #[inline(always)] pub unsafe fn Py_INCREF(op: *mut PyObject) { - #[cfg(any( - GraalPy, - all(Py_LIMITED_API, Py_3_12), - all( - py_sys_config = "Py_REF_DEBUG", - Py_3_10, - not(all(Py_3_12, not(Py_LIMITED_API))) - ) - ))] + // On limited API or with refcount debugging, let the interpreter do refcounting + #[cfg(any(Py_LIMITED_API, py_sys_config = "Py_REF_DEBUG", GraalPy))] { - _Py_IncRef(op); - } + // _Py_IncRef was added to the ABI in 3.10; skips null checks + #[cfg(Py_3_10)] + { + _Py_IncRef(op); + } - #[cfg(all(py_sys_config = "Py_REF_DEBUG", not(Py_3_10)))] - { - return Py_IncRef(op); + #[cfg(not(Py_3_10))] + { + Py_IncRef(op); + } } - #[cfg(any( - all(Py_LIMITED_API, not(Py_3_12)), - all( - not(Py_LIMITED_API), - not(GraalPy), - any( - not(py_sys_config = "Py_REF_DEBUG"), - all(py_sys_config = "Py_REF_DEBUG", Py_3_12), - ) - ), - ))] + // version-specific builds are allowed to directly manipulate the reference count + #[cfg(not(any(any(Py_LIMITED_API, py_sys_config = "Py_REF_DEBUG", GraalPy))))] { #[cfg(all(Py_3_12, target_pointer_width = "64"))] { @@ -564,9 +552,6 @@ pub unsafe fn Py_INCREF(op: *mut PyObject) { // Skipped _Py_INCREF_STAT_INC - if anyone wants this, please file an issue // or submit a PR supporting Py_STATS build option and pystats.h - - #[cfg(all(py_sys_config = "Py_REF_DEBUG", Py_3_12))] - _Py_INCREF_IncRefTotal(); } } @@ -576,35 +561,31 @@ pub unsafe fn Py_INCREF(op: *mut PyObject) { track_caller )] pub unsafe fn Py_DECREF(op: *mut PyObject) { + // On limited API or with refcount debugging, let the interpreter do refcounting + // On 3.12+ we implement refcount debugging to get better assertion locations on negative refcounts #[cfg(any( - GraalPy, - all(Py_LIMITED_API, Py_3_12), - all( - py_sys_config = "Py_REF_DEBUG", - Py_3_10, - not(all(Py_3_12, not(Py_LIMITED_API))) - ) + Py_LIMITED_API, + all(py_sys_config = "Py_REF_DEBUG", not(Py_3_12)), + GraalPy ))] { - _Py_DecRef(op); - } + // _Py_DecRef was added to the ABI in 3.10; skips null checks + #[cfg(Py_3_10)] + { + _Py_DecRef(op); + } - #[cfg(all(py_sys_config = "Py_REF_DEBUG", not(Py_3_10)))] - { - return Py_DecRef(op); + #[cfg(not(Py_3_10))] + { + Py_DecRef(op); + } } - #[cfg(any( - all(Py_LIMITED_API, not(Py_3_12)), - all( - not(Py_LIMITED_API), - not(GraalPy), - any( - not(py_sys_config = "Py_REF_DEBUG"), - all(py_sys_config = "Py_REF_DEBUG", Py_3_12), - ) - ), - ))] + #[cfg(not(any( + Py_LIMITED_API, + all(py_sys_config = "Py_REF_DEBUG", not(Py_3_12)), + GraalPy + )))] { #[cfg(Py_3_12)] if _Py_IsImmortal(op) != 0 { @@ -614,7 +595,7 @@ pub unsafe fn Py_DECREF(op: *mut PyObject) { // Skipped _Py_DECREF_STAT_INC - if anyone needs this, please file an issue // or submit a PR supporting Py_STATS build option and pystats.h - #[cfg(all(py_sys_config = "Py_REF_DEBUG", Py_3_12))] + #[cfg(py_sys_config = "Py_REF_DEBUG")] _Py_DECREF_DecRefTotal(); #[cfg(Py_3_12)] From a71b6c2b90c044fc7454df25ee746e6875161c4c Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 12 Jul 2024 14:20:00 +0100 Subject: [PATCH 2/2] fix implementation on PyPy --- pyo3-ffi/src/object.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index 5055d146ee5..21161a34d3a 100644 --- a/pyo3-ffi/src/object.rs +++ b/pyo3-ffi/src/object.rs @@ -490,11 +490,9 @@ extern "C" { #[cfg_attr(GraalPy, link_name = "_Py_DecRef")] pub fn Py_DecRef(o: *mut PyObject); - #[cfg(Py_3_10)] - #[cfg_attr(PyPy, link_name = "_PyPy_IncRef")] + #[cfg(all(Py_3_10, not(PyPy)))] pub fn _Py_IncRef(o: *mut PyObject); - #[cfg(Py_3_10)] - #[cfg_attr(PyPy, link_name = "_PyPy_DecRef")] + #[cfg(all(Py_3_10, not(PyPy)))] pub fn _Py_DecRef(o: *mut PyObject); #[cfg(GraalPy)] @@ -513,12 +511,12 @@ pub unsafe fn Py_INCREF(op: *mut PyObject) { #[cfg(any(Py_LIMITED_API, py_sys_config = "Py_REF_DEBUG", GraalPy))] { // _Py_IncRef was added to the ABI in 3.10; skips null checks - #[cfg(Py_3_10)] + #[cfg(all(Py_3_10, not(PyPy)))] { _Py_IncRef(op); } - #[cfg(not(Py_3_10))] + #[cfg(any(not(Py_3_10), PyPy))] { Py_IncRef(op); } @@ -570,12 +568,12 @@ pub unsafe fn Py_DECREF(op: *mut PyObject) { ))] { // _Py_DecRef was added to the ABI in 3.10; skips null checks - #[cfg(Py_3_10)] + #[cfg(all(Py_3_10, not(PyPy)))] { _Py_DecRef(op); } - #[cfg(not(Py_3_10))] + #[cfg(any(not(Py_3_10), PyPy))] { Py_DecRef(op); }