From 35e02b0aa43e2dd60bb77dee8a773612c4d5f0b3 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 3 Apr 2026 11:19:36 +0100 Subject: [PATCH 1/5] update benchmarks to use borrowed function args --- pyo3-benches/benches/bench_call.rs | 38 ++++++++++++++---------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/pyo3-benches/benches/bench_call.rs b/pyo3-benches/benches/bench_call.rs index 909e5761149..fa41bb12de5 100644 --- a/pyo3-benches/benches/bench_call.rs +++ b/pyo3-benches/benches/bench_call.rs @@ -33,14 +33,14 @@ fn bench_call_1(b: &mut Bencher<'_>) { let foo_module = &module.getattr("foo").unwrap(); let args = ( - 1.into_pyobject(py).unwrap(), + 1i32.into_pyobject(py).unwrap(), "s".into_pyobject(py).unwrap(), - 1.23.into_pyobject(py).unwrap(), + 1.23f64.into_pyobject(py).unwrap(), ); b.iter(|| { for _ in 0..1000 { - black_box(foo_module).call1(args.clone()).unwrap(); + black_box(foo_module).call1(&args).unwrap(); } }); }) @@ -52,17 +52,15 @@ fn bench_call(b: &mut Bencher<'_>) { let foo_module = &module.getattr("foo").unwrap(); let args = ( - 1.into_pyobject(py).unwrap(), + 1i32.into_pyobject(py).unwrap(), "s".into_pyobject(py).unwrap(), - 1.23.into_pyobject(py).unwrap(), + 1.23f64.into_pyobject(py).unwrap(), ); let kwargs = [("d", 1), ("e", 42)].into_py_dict(py).unwrap(); b.iter(|| { for _ in 0..1000 { - black_box(foo_module) - .call(args.clone(), Some(&kwargs)) - .unwrap(); + black_box(foo_module).call(&args, Some(&kwargs)).unwrap(); } }); }) @@ -77,7 +75,7 @@ fn bench_call_one_arg(b: &mut Bencher<'_>) { b.iter(|| { for _ in 0..1000 { - black_box(foo_module).call1((arg.clone(),)).unwrap(); + black_box(foo_module).call1((&arg,)).unwrap(); } }); }) @@ -116,17 +114,16 @@ class Foo: ); let foo_module = &module.getattr("Foo").unwrap().call0().unwrap(); + let meth = "foo".into_pyobject(py).unwrap(); let args = ( - 1.into_pyobject(py).unwrap(), + 1i32.into_pyobject(py).unwrap(), "s".into_pyobject(py).unwrap(), - 1.23.into_pyobject(py).unwrap(), + 1.23f64.into_pyobject(py).unwrap(), ); b.iter(|| { for _ in 0..1000 { - black_box(foo_module) - .call_method1("foo", args.clone()) - .unwrap(); + black_box(foo_module).call_method1(&meth, &args).unwrap(); } }); }) @@ -144,17 +141,19 @@ class Foo: ); let foo_module = &module.getattr("Foo").unwrap().call0().unwrap(); + let meth = "foo".into_pyobject(py).unwrap(); + let args = ( - 1.into_pyobject(py).unwrap(), + 1i32.into_pyobject(py).unwrap(), "s".into_pyobject(py).unwrap(), - 1.23.into_pyobject(py).unwrap(), + 1.23f64.into_pyobject(py).unwrap(), ); let kwargs = [("d", 1), ("e", 42)].into_py_dict(py).unwrap(); b.iter(|| { for _ in 0..1000 { black_box(foo_module) - .call_method("foo", args.clone(), Some(&kwargs)) + .call_method(&meth, &args, Some(&kwargs)) .unwrap(); } }); @@ -173,13 +172,12 @@ class Foo: ); let foo_module = &module.getattr("Foo").unwrap().call0().unwrap(); + let meth = "foo".into_pyobject(py).unwrap(); let arg = 1i32.into_pyobject(py).unwrap(); b.iter(|| { for _ in 0..1000 { - black_box(foo_module) - .call_method1("foo", (arg.clone(),)) - .unwrap(); + black_box(foo_module).call_method1(&meth, (&arg,)).unwrap(); } }); }) From 052a62db11a1e665ed2908dfda0369b569d18429 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 3 Apr 2026 12:02:05 +0100 Subject: [PATCH 2/5] avoid incrementing reference count of function args where possible --- src/types/tuple.rs | 52 +++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/src/types/tuple.rs b/src/types/tuple.rs index acc8be104aa..e7c44d0b2fd 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -2,7 +2,7 @@ use crate::ffi::{self, Py_ssize_t}; use crate::ffi_ptr_ext::FfiPtrExt; #[cfg(feature = "experimental-inspect")] use crate::inspect::{type_hint_subscript, PyStaticExpr}; -use crate::instance::Borrowed; +use crate::instance::{Borrowed, BoundObject}; use crate::internal_tricks::get_ssize_index; #[cfg(feature = "experimental-inspect")] use crate::type_object::PyTypeInfo; @@ -657,9 +657,9 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ) -> PyResult> { let py = function.py(); // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. - let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; + let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_VectorcallDict( function.as_ptr(), @@ -678,22 +678,21 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ _: crate::call::private::Token, ) -> PyResult> { let py = function.py(); - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); #[cfg(not(Py_LIMITED_API))] if $length == 1 { return unsafe { ffi::PyObject_CallOneArg( function.as_ptr(), - args_bound[0].as_ptr() + args_objects.0.as_ptr() ) .assume_owned_or_err(py) }; } // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. - let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; + let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_Vectorcall( function.as_ptr(), @@ -713,28 +712,14 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ _: crate::call::private::Token, ) -> PyResult> { let py = object.py(); - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; - - #[cfg(not(Py_LIMITED_API))] - if $length == 1 { - return unsafe { - ffi::PyObject_CallMethodOneArg( - object.as_ptr(), - method_name.as_ptr(), - args_bound[0].as_ptr(), - ) - .assume_owned_or_err(py) - }; - } + let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); - let mut args = [object.as_ptr(), $(args_bound[$n].as_ptr()),*]; + let mut args = [object.as_ptr(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_VectorcallMethod( method_name.as_ptr(), args.as_mut_ptr(), - // +1 for the receiver. - 1 + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + args.len() | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -785,10 +770,9 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ _: crate::call::private::Token, ) -> PyResult> { let py = function.py(); - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. - let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; + let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_VectorcallDict( function.as_ptr(), @@ -807,22 +791,21 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ _: crate::call::private::Token, ) -> PyResult> { let py = function.py(); - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); #[cfg(not(Py_LIMITED_API))] if $length == 1 { return unsafe { ffi::PyObject_CallOneArg( function.as_ptr(), - args_bound[0].as_ptr() + args_objects.0.as_ptr() ) .assume_owned_or_err(py) }; } // Prepend one null argument for `PY_VECTORCALL_ARGUMENTS_OFFSET`. - let mut args = [std::ptr::null_mut(), $(args_bound[$n].as_ptr()),*]; + let mut args = [std::ptr::null_mut(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_Vectorcall( function.as_ptr(), @@ -842,8 +825,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ _: crate::call::private::Token, ) -> PyResult> { let py = object.py(); - // We need this to drop the arguments correctly. - let args_bound = [$(self.$n.into_bound_py_any(py)?,)*]; + let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); #[cfg(not(Py_LIMITED_API))] if $length == 1 { @@ -851,13 +833,13 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_CallMethodOneArg( object.as_ptr(), method_name.as_ptr(), - args_bound[0].as_ptr(), + args_objects.0.as_ptr(), ) .assume_owned_or_err(py) }; } - let mut args = [object.as_ptr(), $(args_bound[$n].as_ptr()),*]; + let mut args = [object.as_ptr(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_VectorcallMethod( method_name.as_ptr(), From 8333e10ac1759f12e1edf348cc178f06c8e78ca0 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 3 Apr 2026 12:55:38 +0100 Subject: [PATCH 3/5] clippy + cleanup --- src/types/tuple.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/types/tuple.rs b/src/types/tuple.rs index e7c44d0b2fd..c2e61569060 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -2,11 +2,16 @@ use crate::ffi::{self, Py_ssize_t}; use crate::ffi_ptr_ext::FfiPtrExt; #[cfg(feature = "experimental-inspect")] use crate::inspect::{type_hint_subscript, PyStaticExpr}; -use crate::instance::{Borrowed, BoundObject}; +use crate::instance::Borrowed; use crate::internal_tricks::get_ssize_index; #[cfg(feature = "experimental-inspect")] use crate::type_object::PyTypeInfo; use crate::types::{sequence::PySequenceMethods, PyList, PySequence}; +#[cfg(all( + not(any(PyPy, GraalPy)), + any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12) +))] +use crate::BoundObject; use crate::{ exceptions, Bound, FromPyObject, IntoPyObject, IntoPyObjectExt, PyAny, PyErr, PyResult, Python, }; @@ -664,7 +669,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_VectorcallDict( function.as_ptr(), args.as_mut_ptr().add(1), - $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, kwargs.as_ptr(), ) .assume_owned_or_err(py) @@ -697,7 +702,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_Vectorcall( function.as_ptr(), args.as_mut_ptr().add(1), - $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -719,7 +724,8 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_VectorcallMethod( method_name.as_ptr(), args.as_mut_ptr(), - args.len() | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + // +1 for the receiver + const { (1 + $length) | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -777,7 +783,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_VectorcallDict( function.as_ptr(), args.as_mut_ptr().add(1), - $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, kwargs.as_ptr(), ) .assume_owned_or_err(py) @@ -810,7 +816,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_Vectorcall( function.as_ptr(), args.as_mut_ptr().add(1), - $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -845,7 +851,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ method_name.as_ptr(), args.as_mut_ptr(), // +1 for the receiver. - 1 + $length + ffi::PY_VECTORCALL_ARGUMENTS_OFFSET, + const { (1 + $length) | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, std::ptr::null_mut(), ) .assume_owned_or_err(py) From 569ee0009075ceb3973f9a811d795a25272238a8 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 3 Apr 2026 12:57:04 +0100 Subject: [PATCH 4/5] newsfragment --- newsfragments/5941.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/5941.changed.md diff --git a/newsfragments/5941.changed.md b/newsfragments/5941.changed.md new file mode 100644 index 00000000000..fa8ac39fe2f --- /dev/null +++ b/newsfragments/5941.changed.md @@ -0,0 +1 @@ +Avoid temporary reference count cycles inside `PyAnyMethods::call`, `PyAnyMethods::call1` and `PyAnyMethods::call_method1` for arguments passed as Rust tuples containing borrowed references to Python objects. From 9d0ee047f89432a21f5acc565edb6cb47e2b9e2c Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Fri, 3 Apr 2026 13:12:04 +0100 Subject: [PATCH 5/5] cleanup, const overflow checks --- src/types/tuple.rs | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/types/tuple.rs b/src/types/tuple.rs index c2e61569060..29a35264021 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -19,6 +19,12 @@ use std::iter::FusedIterator; #[cfg(feature = "nightly")] use std::num::NonZero; +#[cfg(all( + not(any(PyPy, GraalPy)), + any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12) +))] +use libc::size_t; + #[inline] #[track_caller] fn try_new_from_iter<'py>( @@ -669,7 +675,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_VectorcallDict( function.as_ptr(), args.as_mut_ptr().add(1), - const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, + const { with_vectorcall_arguments_offset($length) }, kwargs.as_ptr(), ) .assume_owned_or_err(py) @@ -702,7 +708,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_Vectorcall( function.as_ptr(), args.as_mut_ptr().add(1), - const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, + const { with_vectorcall_arguments_offset($length) }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -719,13 +725,25 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ let py = object.py(); let args_objects = ($(self.$n.into_pyobject_or_pyerr(py)?),*,); + #[cfg(not(Py_LIMITED_API))] + if $length == 1 { + return unsafe { + ffi::PyObject_CallMethodOneArg( + object.as_ptr(), + method_name.as_ptr(), + args_objects.0.as_ptr() + ) + .assume_owned_or_err(py) + }; + } + let mut args = [object.as_ptr(), $(args_objects.$n.as_ptr()),*]; unsafe { ffi::PyObject_VectorcallMethod( method_name.as_ptr(), args.as_mut_ptr(), - // +1 for the receiver - const { (1 + $length) | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, + // +1 for the receiver. + const { with_vectorcall_arguments_offset(1 + $length) }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -783,7 +801,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_VectorcallDict( function.as_ptr(), args.as_mut_ptr().add(1), - const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, + const { with_vectorcall_arguments_offset($length) }, kwargs.as_ptr(), ) .assume_owned_or_err(py) @@ -816,7 +834,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ ffi::PyObject_Vectorcall( function.as_ptr(), args.as_mut_ptr().add(1), - const { $length | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, + const { with_vectorcall_arguments_offset($length) }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -851,7 +869,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ method_name.as_ptr(), args.as_mut_ptr(), // +1 for the receiver. - const { (1 + $length) | ffi::PY_VECTORCALL_ARGUMENTS_OFFSET }, + const { with_vectorcall_arguments_offset(1 + $length) }, std::ptr::null_mut(), ) .assume_owned_or_err(py) @@ -930,6 +948,18 @@ fn array_into_tuple<'py, const N: usize>( } } +/// Add `PY_VECTORCALL_ARGUMENTS_OFFSET` to the given number, checking for overflow at compile time. +/// +/// Guarantees that we don't accidentally overflow a `size_t` should this get changed in the future. +#[cfg(all( + not(any(PyPy, GraalPy)), + any(all(Py_3_9, not(Py_LIMITED_API)), Py_3_12) +))] +const fn with_vectorcall_arguments_offset(n: size_t) -> size_t { + n.checked_add(ffi::PY_VECTORCALL_ARGUMENTS_OFFSET) + .expect("overflow adding PY_VECTORCALL_ARGUMENTS_OFFSET") +} + tuple_conversion!(1, (ref0, 0, T0)); tuple_conversion!(2, (ref0, 0, T0), (ref1, 1, T1)); tuple_conversion!(3, (ref0, 0, T0), (ref1, 1, T1), (ref2, 2, T2));