From 9151b255c3178cc1ec859d3540134985f7ee1044 Mon Sep 17 00:00:00 2001 From: Matti Picus Date: Wed, 12 Feb 2025 14:54:24 +1100 Subject: [PATCH 1/6] Add PyPy3.11 (#4760) * allow pypy3.11 * run CI on pypy3.11 * change const declaration * change link name for PyExc_BaseExceptionGroup * PyObject_DelAttr* are inline functions on pypy3.11 * use nightly until official release * DOC: add a news fragment * conditionally compile 'use' statements * fix format * changes from review * pypy 3.11 released * fixes for PyPy * typo * exclude _PyInterpreterFrame on PyPy * more pypy fixes * more excluding _PyFrameEvalFunction on PyPy * fixes for PyPy struct differences * format * fix test --------- Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com> --- .github/workflows/ci.yml | 1 + newsfragments/4760.packaging.md | 1 + noxfile.py | 6 +++--- pyo3-ffi/build.rs | 2 +- pyo3-ffi/src/abstract_.rs | 10 ++++++++-- pyo3-ffi/src/cpython/abstract_.rs | 2 +- pyo3-ffi/src/cpython/genobject.rs | 2 +- pyo3-ffi/src/cpython/mod.rs | 2 +- pyo3-ffi/src/cpython/object.rs | 4 ++-- pyo3-ffi/src/cpython/objimpl.rs | 2 +- pyo3-ffi/src/cpython/pyframe.rs | 2 +- pyo3-ffi/src/cpython/pystate.rs | 6 +++--- pyo3-ffi/src/cpython/unicodeobject.rs | 4 ++-- pyo3-ffi/src/object.rs | 4 ++-- pyo3-ffi/src/pybuffer.rs | 6 +++++- pyo3-ffi/src/pyerrors.rs | 1 + src/ffi/tests.rs | 2 +- 17 files changed, 35 insertions(+), 22 deletions(-) create mode 100644 newsfragments/4760.packaging.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76c5ab06f29..8e16f814c13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -244,6 +244,7 @@ jobs: "3.13", "pypy3.9", "pypy3.10", + "pypy3.11", "graalpy24.0", ] platform: diff --git a/newsfragments/4760.packaging.md b/newsfragments/4760.packaging.md new file mode 100644 index 00000000000..e7fe53099d1 --- /dev/null +++ b/newsfragments/4760.packaging.md @@ -0,0 +1 @@ +add support for PyPy3.11 diff --git a/noxfile.py b/noxfile.py index ed7759f5f99..aa194a9e892 100644 --- a/noxfile.py +++ b/noxfile.py @@ -32,7 +32,7 @@ PYO3_GUIDE_TARGET = PYO3_TARGET / "guide" PYO3_DOCS_TARGET = PYO3_TARGET / "doc" PY_VERSIONS = ("3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13") -PYPY_VERSIONS = ("3.9", "3.10") +PYPY_VERSIONS = ("3.9", "3.10", "3.11") FREE_THREADED_BUILD = bool(sysconfig.get_config_var("Py_GIL_DISABLED")) @@ -673,8 +673,8 @@ def test_version_limits(session: nox.Session): config_file.set("PyPy", "3.8") _run_cargo(session, "check", env=env, expect_error=True) - assert "3.11" not in PYPY_VERSIONS - config_file.set("PyPy", "3.11") + assert "3.12" not in PYPY_VERSIONS + config_file.set("PyPy", "3.12") _run_cargo(session, "check", env=env, expect_error=True) diff --git a/pyo3-ffi/build.rs b/pyo3-ffi/build.rs index ea023de75fa..096614c7961 100644 --- a/pyo3-ffi/build.rs +++ b/pyo3-ffi/build.rs @@ -25,7 +25,7 @@ const SUPPORTED_VERSIONS_PYPY: SupportedVersions = SupportedVersions { min: PythonVersion { major: 3, minor: 9 }, max: PythonVersion { major: 3, - minor: 10, + minor: 11, }, }; diff --git a/pyo3-ffi/src/abstract_.rs b/pyo3-ffi/src/abstract_.rs index ce6c9b94735..c59d1603236 100644 --- a/pyo3-ffi/src/abstract_.rs +++ b/pyo3-ffi/src/abstract_.rs @@ -3,13 +3,19 @@ use crate::pyport::Py_ssize_t; use std::os::raw::{c_char, c_int}; #[inline] -#[cfg(all(not(Py_3_13), not(PyPy)))] // CPython exposed as a function in 3.13, in object.h +#[cfg(all( + not(Py_3_13), // CPython exposed as a function in 3.13, in object.h + not(all(PyPy, not(Py_3_11))) // PyPy exposed as a function until PyPy 3.10, macro in 3.11+ +))] pub unsafe fn PyObject_DelAttrString(o: *mut PyObject, attr_name: *const c_char) -> c_int { PyObject_SetAttrString(o, attr_name, std::ptr::null_mut()) } #[inline] -#[cfg(all(not(Py_3_13), not(PyPy)))] // CPython exposed as a function in 3.13, in object.h +#[cfg(all( + not(Py_3_13), // CPython exposed as a function in 3.13, in object.h + not(all(PyPy, not(Py_3_11))) // PyPy exposed as a function until PyPy 3.10, macro in 3.11+ +))] pub unsafe fn PyObject_DelAttr(o: *mut PyObject, attr_name: *mut PyObject) -> c_int { PyObject_SetAttr(o, attr_name, std::ptr::null_mut()) } diff --git a/pyo3-ffi/src/cpython/abstract_.rs b/pyo3-ffi/src/cpython/abstract_.rs index 477ad02b747..9264aeb3d40 100644 --- a/pyo3-ffi/src/cpython/abstract_.rs +++ b/pyo3-ffi/src/cpython/abstract_.rs @@ -1,5 +1,5 @@ use crate::{PyObject, Py_ssize_t}; -#[cfg(not(all(Py_3_11, GraalPy)))] +#[cfg(any(all(Py_3_8, not(any(PyPy, GraalPy))), not(Py_3_11)))] use std::os::raw::c_char; use std::os::raw::c_int; diff --git a/pyo3-ffi/src/cpython/genobject.rs b/pyo3-ffi/src/cpython/genobject.rs index 4be310a8c88..c9d419e3782 100644 --- a/pyo3-ffi/src/cpython/genobject.rs +++ b/pyo3-ffi/src/cpython/genobject.rs @@ -2,7 +2,7 @@ use crate::object::*; use crate::PyFrameObject; #[cfg(not(any(PyPy, GraalPy)))] use crate::_PyErr_StackItem; -#[cfg(all(Py_3_11, not(GraalPy)))] +#[cfg(all(Py_3_11, not(any(PyPy, GraalPy))))] use std::os::raw::c_char; use std::os::raw::c_int; use std::ptr::addr_of_mut; diff --git a/pyo3-ffi/src/cpython/mod.rs b/pyo3-ffi/src/cpython/mod.rs index fe909f0ceeb..f09d51d0e4e 100644 --- a/pyo3-ffi/src/cpython/mod.rs +++ b/pyo3-ffi/src/cpython/mod.rs @@ -71,7 +71,7 @@ pub use self::object::*; pub use self::objimpl::*; pub use self::pydebug::*; pub use self::pyerrors::*; -#[cfg(Py_3_11)] +#[cfg(all(Py_3_11, not(PyPy)))] pub use self::pyframe::*; #[cfg(all(Py_3_8, not(PyPy)))] pub use self::pylifecycle::*; diff --git a/pyo3-ffi/src/cpython/object.rs b/pyo3-ffi/src/cpython/object.rs index 75eef11aae3..4e6932da789 100644 --- a/pyo3-ffi/src/cpython/object.rs +++ b/pyo3-ffi/src/cpython/object.rs @@ -310,9 +310,9 @@ pub struct PyHeapTypeObject { pub ht_cached_keys: *mut c_void, #[cfg(Py_3_9)] pub ht_module: *mut object::PyObject, - #[cfg(Py_3_11)] + #[cfg(all(Py_3_11, not(PyPy)))] _ht_tpname: *mut c_char, - #[cfg(Py_3_11)] + #[cfg(all(Py_3_11, not(PyPy)))] _spec_cache: _specialization_cache, } diff --git a/pyo3-ffi/src/cpython/objimpl.rs b/pyo3-ffi/src/cpython/objimpl.rs index 98a19abeb81..14f7121a202 100644 --- a/pyo3-ffi/src/cpython/objimpl.rs +++ b/pyo3-ffi/src/cpython/objimpl.rs @@ -1,4 +1,4 @@ -#[cfg(not(all(Py_3_11, GraalPy)))] +#[cfg(not(all(Py_3_11, any(PyPy, GraalPy))))] use libc::size_t; use std::os::raw::c_int; diff --git a/pyo3-ffi/src/cpython/pyframe.rs b/pyo3-ffi/src/cpython/pyframe.rs index d0cfa0a2c6d..5e1e16a7d08 100644 --- a/pyo3-ffi/src/cpython/pyframe.rs +++ b/pyo3-ffi/src/cpython/pyframe.rs @@ -1,2 +1,2 @@ -#[cfg(Py_3_11)] +#[cfg(all(Py_3_11, not(PyPy)))] opaque_struct!(_PyInterpreterFrame); diff --git a/pyo3-ffi/src/cpython/pystate.rs b/pyo3-ffi/src/cpython/pystate.rs index 5481265b55d..650cd6a1f7f 100644 --- a/pyo3-ffi/src/cpython/pystate.rs +++ b/pyo3-ffi/src/cpython/pystate.rs @@ -69,21 +69,21 @@ extern "C" { pub fn PyThreadState_DeleteCurrent(); } -#[cfg(all(Py_3_9, not(Py_3_11)))] +#[cfg(all(Py_3_9, not(any(Py_3_11, PyPy))))] pub type _PyFrameEvalFunction = extern "C" fn( *mut crate::PyThreadState, *mut crate::PyFrameObject, c_int, ) -> *mut crate::object::PyObject; -#[cfg(Py_3_11)] +#[cfg(all(Py_3_11, not(PyPy)))] pub type _PyFrameEvalFunction = extern "C" fn( *mut crate::PyThreadState, *mut crate::_PyInterpreterFrame, c_int, ) -> *mut crate::object::PyObject; -#[cfg(Py_3_9)] +#[cfg(all(Py_3_9, not(PyPy)))] extern "C" { /// Get the frame evaluation function. pub fn _PyInterpreterState_GetEvalFrameFunc( diff --git a/pyo3-ffi/src/cpython/unicodeobject.rs b/pyo3-ffi/src/cpython/unicodeobject.rs index fae626b8d25..3527a5aeadb 100644 --- a/pyo3-ffi/src/cpython/unicodeobject.rs +++ b/pyo3-ffi/src/cpython/unicodeobject.rs @@ -1,4 +1,4 @@ -#[cfg(not(PyPy))] +#[cfg(any(Py_3_11, not(PyPy)))] use crate::Py_hash_t; use crate::{PyObject, Py_UCS1, Py_UCS2, Py_UCS4, Py_ssize_t}; use libc::wchar_t; @@ -251,7 +251,7 @@ impl From for u32 { pub struct PyASCIIObject { pub ob_base: PyObject, pub length: Py_ssize_t, - #[cfg(not(PyPy))] + #[cfg(any(Py_3_11, not(PyPy)))] pub hash: Py_hash_t, /// A bit field with various properties. /// diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index 3f086ac1e92..087cd32920c 100644 --- a/pyo3-ffi/src/object.rs +++ b/pyo3-ffi/src/object.rs @@ -436,7 +436,7 @@ extern "C" { arg2: *const c_char, arg3: *mut PyObject, ) -> c_int; - #[cfg(any(Py_3_13, PyPy))] // CPython defined in 3.12 as an inline function in abstract.h + #[cfg(any(Py_3_13, all(PyPy, not(Py_3_11))))] // CPython defined in 3.12 as an inline function in abstract.h #[cfg_attr(PyPy, link_name = "PyPyObject_DelAttrString")] pub fn PyObject_DelAttrString(arg1: *mut PyObject, arg2: *const c_char) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyObject_HasAttrString")] @@ -460,7 +460,7 @@ extern "C" { #[cfg_attr(PyPy, link_name = "PyPyObject_SetAttr")] pub fn PyObject_SetAttr(arg1: *mut PyObject, arg2: *mut PyObject, arg3: *mut PyObject) -> c_int; - #[cfg(any(Py_3_13, PyPy))] // CPython defined in 3.12 as an inline function in abstract.h + #[cfg(any(Py_3_13, all(PyPy, not(Py_3_11))))] // CPython defined in 3.12 as an inline function in abstract.h #[cfg_attr(PyPy, link_name = "PyPyObject_DelAttr")] pub fn PyObject_DelAttr(arg1: *mut PyObject, arg2: *mut PyObject) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyObject_HasAttr")] diff --git a/pyo3-ffi/src/pybuffer.rs b/pyo3-ffi/src/pybuffer.rs index 50bf4e6109c..de7067599ff 100644 --- a/pyo3-ffi/src/pybuffer.rs +++ b/pyo3-ffi/src/pybuffer.rs @@ -103,7 +103,11 @@ extern "C" { } /// Maximum number of dimensions -pub const PyBUF_MAX_NDIM: c_int = if cfg!(PyPy) { 36 } else { 64 }; +pub const PyBUF_MAX_NDIM: usize = if cfg!(all(PyPy, not(Py_3_11))) { + 36 +} else { + 64 +}; /* Flags for getting buffers */ pub const PyBUF_SIMPLE: c_int = 0; diff --git a/pyo3-ffi/src/pyerrors.rs b/pyo3-ffi/src/pyerrors.rs index 6c9313c4ab0..d341239a07b 100644 --- a/pyo3-ffi/src/pyerrors.rs +++ b/pyo3-ffi/src/pyerrors.rs @@ -116,6 +116,7 @@ extern "C" { #[cfg_attr(PyPy, link_name = "PyPyExc_BaseException")] pub static mut PyExc_BaseException: *mut PyObject; #[cfg(Py_3_11)] + #[cfg_attr(PyPy, link_name = "PyPyExc_BaseExceptionGroup")] pub static mut PyExc_BaseExceptionGroup: *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyExc_Exception")] pub static mut PyExc_Exception: *mut PyObject; diff --git a/src/ffi/tests.rs b/src/ffi/tests.rs index 3396e409368..b2d9e4d39cd 100644 --- a/src/ffi/tests.rs +++ b/src/ffi/tests.rs @@ -121,7 +121,7 @@ fn ascii_object_bitfield() { let mut o = PyASCIIObject { ob_base, length: 0, - #[cfg(not(PyPy))] + #[cfg(any(Py_3_11, not(PyPy)))] hash: 0, state: 0u32, #[cfg(not(Py_3_12))] From 09ac6765b5bdf7657c776a831a652c127078f47c Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Sun, 2 Feb 2025 19:53:33 +0000 Subject: [PATCH 2/6] Fix manual_ok_err clippy lint on nightly (#4886) https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err --- src/types/set.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/types/set.rs b/src/types/set.rs index d5e39ebc83d..60aa9428562 100644 --- a/src/types/set.rs +++ b/src/types/set.rs @@ -202,10 +202,7 @@ impl<'py> PySetMethods<'py> for Bound<'py, PySet> { fn pop(&self) -> Option> { let element = unsafe { ffi::PySet_Pop(self.as_ptr()).assume_owned_or_err(self.py()) }; - match element { - Ok(e) => Some(e), - Err(_) => None, - } + element.ok() } fn iter(&self) -> BoundSetIterator<'py> { From 80c4d05b894c288428dace1306fa79d3e54e135c Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 19 Feb 2025 05:49:35 -0700 Subject: [PATCH 3/6] fix multithreaded access to freelist pyclasses (#4902) * make FreeList explicitly a wrapper for *mut PyObject * fix multithreaded access to freelist pyclasses * add changelog entry * use a GILOnceCell to initialize the freelist * respond to code review * skip test on wasm --------- Co-authored-by: David Hewitt --- newsfragments/4902.fixed.md | 1 + pyo3-macros-backend/src/pyclass.rs | 16 +++++------- src/impl_/freelist.rs | 42 +++++++++++++++++------------- src/impl_/pyclass.rs | 15 ++++++++--- tests/test_gc.rs | 29 +++++++++++++++++++++ 5 files changed, 72 insertions(+), 31 deletions(-) create mode 100644 newsfragments/4902.fixed.md diff --git a/newsfragments/4902.fixed.md b/newsfragments/4902.fixed.md new file mode 100644 index 00000000000..e377ab018d7 --- /dev/null +++ b/newsfragments/4902.fixed.md @@ -0,0 +1 @@ +* Fixed thread-unsafe implementation of freelist pyclasses on the free-threaded build. \ No newline at end of file diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 7697ae4451f..d3e4e0edf18 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -2395,15 +2395,13 @@ impl<'a> PyClassImplsBuilder<'a> { quote! { impl #pyo3_path::impl_::pyclass::PyClassWithFreeList for #cls { #[inline] - fn get_free_list(py: #pyo3_path::Python<'_>) -> &mut #pyo3_path::impl_::freelist::FreeList<*mut #pyo3_path::ffi::PyObject> { - static mut FREELIST: *mut #pyo3_path::impl_::freelist::FreeList<*mut #pyo3_path::ffi::PyObject> = 0 as *mut _; - unsafe { - if FREELIST.is_null() { - FREELIST = ::std::boxed::Box::into_raw(::std::boxed::Box::new( - #pyo3_path::impl_::freelist::FreeList::with_capacity(#freelist))); - } - &mut *FREELIST - } + fn get_free_list(py: #pyo3_path::Python<'_>) -> &'static ::std::sync::Mutex<#pyo3_path::impl_::freelist::PyObjectFreeList> { + static FREELIST: #pyo3_path::sync::GILOnceCell<::std::sync::Mutex<#pyo3_path::impl_::freelist::PyObjectFreeList>> = #pyo3_path::sync::GILOnceCell::new(); + // If there's a race to fill the cell, the object created + // by the losing thread will be deallocated via RAII + &FREELIST.get_or_init(py, || { + ::std::sync::Mutex::new(#pyo3_path::impl_::freelist::PyObjectFreeList::with_capacity(#freelist)) + }) } } } diff --git a/src/impl_/freelist.rs b/src/impl_/freelist.rs index 0e611d421d6..713bb3f6464 100644 --- a/src/impl_/freelist.rs +++ b/src/impl_/freelist.rs @@ -8,31 +8,37 @@ //! //! [1]: https://en.wikipedia.org/wiki/Free_list +use crate::ffi; use std::mem; -/// Represents a slot of a [`FreeList`]. -pub enum Slot { +/// Represents a slot of a [`PyObjectFreeList`]. +enum PyObjectSlot { /// A free slot. Empty, /// An allocated slot. - Filled(T), + Filled(*mut ffi::PyObject), } -/// A free allocation list. +// safety: access is guarded by a per-pyclass mutex +unsafe impl Send for PyObjectSlot {} + +/// A free allocation list for PyObject ffi pointers. /// /// See [the parent module](crate::impl_::freelist) for more details. -pub struct FreeList { - entries: Vec>, +pub struct PyObjectFreeList { + entries: Box<[PyObjectSlot]>, split: usize, capacity: usize, } -impl FreeList { - /// Creates a new `FreeList` instance with specified capacity. - pub fn with_capacity(capacity: usize) -> FreeList { - let entries = (0..capacity).map(|_| Slot::Empty).collect::>(); +impl PyObjectFreeList { + /// Creates a new `PyObjectFreeList` instance with specified capacity. + pub fn with_capacity(capacity: usize) -> PyObjectFreeList { + let entries = (0..capacity) + .map(|_| PyObjectSlot::Empty) + .collect::>(); - FreeList { + PyObjectFreeList { entries, split: 0, capacity, @@ -40,26 +46,26 @@ impl FreeList { } /// Pops the first non empty item. - pub fn pop(&mut self) -> Option { + pub fn pop(&mut self) -> Option<*mut ffi::PyObject> { let idx = self.split; if idx == 0 { None } else { - match mem::replace(&mut self.entries[idx - 1], Slot::Empty) { - Slot::Filled(v) => { + match mem::replace(&mut self.entries[idx - 1], PyObjectSlot::Empty) { + PyObjectSlot::Filled(v) => { self.split = idx - 1; Some(v) } - _ => panic!("FreeList is corrupt"), + _ => panic!("PyObjectFreeList is corrupt"), } } } - /// Inserts a value into the list. Returns `Some(val)` if the `FreeList` is full. - pub fn insert(&mut self, val: T) -> Option { + /// Inserts a value into the list. Returns `Some(val)` if the `PyObjectFreeList` is full. + pub fn insert(&mut self, val: *mut ffi::PyObject) -> Option<*mut ffi::PyObject> { let next = self.split + 1; if next < self.capacity { - self.entries[self.split] = Slot::Filled(val); + self.entries[self.split] = PyObjectSlot::Filled(val); self.split = next; None } else { diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 7bb61442ec5..164673aafaa 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -2,7 +2,7 @@ use crate::{ exceptions::{PyAttributeError, PyNotImplementedError, PyRuntimeError, PyValueError}, ffi, impl_::{ - freelist::FreeList, + freelist::PyObjectFreeList, pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectLayout}, pyclass_init::PyObjectInit, pymethods::{PyGetterDef, PyMethodDefType}, @@ -20,6 +20,7 @@ use std::{ marker::PhantomData, os::raw::{c_int, c_void}, ptr::NonNull, + sync::Mutex, thread, }; @@ -910,7 +911,7 @@ use super::{pycell::PyClassObject, pymethods::BoundRef}; /// Do not implement this trait manually. Instead, use `#[pyclass(freelist = N)]` /// on a Rust struct to implement it. pub trait PyClassWithFreeList: PyClass { - fn get_free_list(py: Python<'_>) -> &mut FreeList<*mut ffi::PyObject>; + fn get_free_list(py: Python<'_>) -> &'static Mutex; } /// Implementation of tp_alloc for `freelist` classes. @@ -931,7 +932,9 @@ pub unsafe extern "C" fn alloc_with_freelist( // If this type is a variable type or the subtype is not equal to this type, we cannot use the // freelist if nitems == 0 && subtype == self_type { - if let Some(obj) = T::get_free_list(py).pop() { + let mut free_list = T::get_free_list(py).lock().unwrap(); + if let Some(obj) = free_list.pop() { + drop(free_list); ffi::PyObject_Init(obj, subtype); return obj as _; } @@ -951,7 +954,11 @@ pub unsafe extern "C" fn free_with_freelist(obj: *mut c_ T::type_object_raw(Python::assume_gil_acquired()), ffi::Py_TYPE(obj) ); - if let Some(obj) = T::get_free_list(Python::assume_gil_acquired()).insert(obj) { + let mut free_list = T::get_free_list(Python::assume_gil_acquired()) + .lock() + .unwrap(); + if let Some(obj) = free_list.insert(obj) { + drop(free_list); let ty = ffi::Py_TYPE(obj); // Deduce appropriate inverse of PyType_GenericAlloc diff --git a/tests/test_gc.rs b/tests/test_gc.rs index 4b293449b36..ea029a91d47 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -36,6 +36,35 @@ fn class_with_freelist() { }); } +#[pyclass(freelist = 2)] +#[cfg(not(target_arch = "wasm32"))] +struct ClassWithFreelistAndData { + data: Option, +} + +#[cfg(not(target_arch = "wasm32"))] +fn spin_freelist(py: Python<'_>, data: usize) { + for _ in 0..500 { + let inst1 = Py::new(py, ClassWithFreelistAndData { data: Some(data) }).unwrap(); + let inst2 = Py::new(py, ClassWithFreelistAndData { data: Some(data) }).unwrap(); + assert_eq!(inst1.borrow(py).data, Some(data)); + assert_eq!(inst2.borrow(py).data, Some(data)); + } +} + +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn multithreaded_class_with_freelist() { + std::thread::scope(|s| { + s.spawn(|| { + Python::with_gil(|py| spin_freelist(py, 12)); + }); + s.spawn(|| { + Python::with_gil(|py| spin_freelist(py, 0x4d3d3d3)); + }); + }); +} + /// Helper function to create a pair of objects that can be used to test drops; /// the first object is a guard that records when it has been dropped, the second /// object is a check that can be used to assert that the guard has been dropped. From fe10e2dd100420f7b8fa08dc6d3f4823d3439131 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 2 Feb 2025 20:35:41 +0200 Subject: [PATCH 4/6] avoid FailedHealthCheck in test_double (#4879) Co-authored-by: Ariel Ben-Yehuda --- newsfragments/4879.fixed.md | 1 + pytests/tests/test_othermod.py | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 newsfragments/4879.fixed.md diff --git a/newsfragments/4879.fixed.md b/newsfragments/4879.fixed.md new file mode 100644 index 00000000000..2ad659b0786 --- /dev/null +++ b/newsfragments/4879.fixed.md @@ -0,0 +1 @@ + * fixed spurious `test_double` failures. diff --git a/pytests/tests/test_othermod.py b/pytests/tests/test_othermod.py index ff67bba435c..f2dd9ad8fd2 100644 --- a/pytests/tests/test_othermod.py +++ b/pytests/tests/test_othermod.py @@ -3,11 +3,16 @@ from pyo3_pytests import othermod -INTEGER32_ST = st.integers(min_value=(-(2**31)), max_value=(2**31 - 1)) +INTEGER31_ST = st.integers(min_value=(-(2**30)), max_value=(2**30 - 1)) USIZE_ST = st.integers(min_value=othermod.USIZE_MIN, max_value=othermod.USIZE_MAX) -@given(x=INTEGER32_ST) +# If the full 32 bits are used here, then you can get failures that look like this: +# hypothesis.errors.FailedHealthCheck: It looks like your strategy is filtering out a lot of data. +# Health check found 50 filtered examples but only 7 good ones. +# +# Limit the range to 31 bits to avoid this problem. +@given(x=INTEGER31_ST) def test_double(x): expected = x * 2 assume(-(2**31) <= expected <= (2**31 - 1)) From 319eef09ce24c18e35a87961903691af28b3e63c Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 14 Feb 2025 11:34:35 +0100 Subject: [PATCH 5/6] bump `benchmarks` ci base image (#4912) --- .github/workflows/benches.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/benches.yml b/.github/workflows/benches.yml index 97b882dc858..62c64d9d113 100644 --- a/.github/workflows/benches.yml +++ b/.github/workflows/benches.yml @@ -15,8 +15,7 @@ concurrency: jobs: benchmarks: - # No support for 24.04, see https://github.com/CodSpeedHQ/runner/issues/42 - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 From 0f658520e367370ec452ba7eab059c7de17fff35 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 19 Feb 2025 09:24:37 -0700 Subject: [PATCH 6/6] Use a critical section to serialize adding builtins to globals (#4921) * add test that panics because __builtins__ isn't available * use a critical section to serialize adding __builtins__ to __globals__ * add release note * use safe APIs instead of PyDict_Contains --- newsfragments/4921.fixed.md | 1 + src/marker.rs | 93 +++++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 24 deletions(-) create mode 100644 newsfragments/4921.fixed.md diff --git a/newsfragments/4921.fixed.md b/newsfragments/4921.fixed.md new file mode 100644 index 00000000000..86a91fd727a --- /dev/null +++ b/newsfragments/4921.fixed.md @@ -0,0 +1 @@ +* Reenabled a workaround for situations where CPython incorrectly does not add `__builtins__` to `__globals__` in code executed by `Python::py_run`. \ No newline at end of file diff --git a/src/marker.rs b/src/marker.rs index 5962b47b60b..f98a725da0e 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -117,7 +117,6 @@ //! [`Rc`]: std::rc::Rc //! [`Py`]: crate::Py use crate::conversion::IntoPyObject; -#[cfg(any(doc, not(Py_3_10)))] use crate::err::PyErr; use crate::err::{self, PyResult}; use crate::ffi_ptr_ext::FfiPtrExt; @@ -649,30 +648,34 @@ impl<'py> Python<'py> { }; let locals = locals.unwrap_or(globals); - #[cfg(not(Py_3_10))] - { - // If `globals` don't provide `__builtins__`, most of the code will fail if Python - // version is <3.10. That's probably not what user intended, so insert `__builtins__` - // for them. - // - // See also: - // - https://github.com/python/cpython/pull/24564 (the same fix in CPython 3.10) - // - https://github.com/PyO3/pyo3/issues/3370 - let builtins_s = crate::intern!(self, "__builtins__").as_ptr(); - let has_builtins = unsafe { ffi::PyDict_Contains(globals.as_ptr(), builtins_s) }; - if has_builtins == -1 { - return Err(PyErr::fetch(self)); - } - if has_builtins == 0 { - // Inherit current builtins. - let builtins = unsafe { ffi::PyEval_GetBuiltins() }; - - // `PyDict_SetItem` doesn't take ownership of `builtins`, but `PyEval_GetBuiltins` - // seems to return a borrowed reference, so no leak here. - if unsafe { ffi::PyDict_SetItem(globals.as_ptr(), builtins_s, builtins) } == -1 { - return Err(PyErr::fetch(self)); + // If `globals` don't provide `__builtins__`, most of the code will fail if Python + // version is <3.10. That's probably not what user intended, so insert `__builtins__` + // for them. + // + // See also: + // - https://github.com/python/cpython/pull/24564 (the same fix in CPython 3.10) + // - https://github.com/PyO3/pyo3/issues/3370 + let builtins_s = crate::intern!(self, "__builtins__"); + let has_builtins = globals.contains(builtins_s)?; + if !has_builtins { + crate::sync::with_critical_section(globals, || { + // check if another thread set __builtins__ while this thread was blocked on the critical section + let has_builtins = globals.contains(builtins_s)?; + if !has_builtins { + // Inherit current builtins. + let builtins = unsafe { ffi::PyEval_GetBuiltins() }; + + // `PyDict_SetItem` doesn't take ownership of `builtins`, but `PyEval_GetBuiltins` + // seems to return a borrowed reference, so no leak here. + if unsafe { + ffi::PyDict_SetItem(globals.as_ptr(), builtins_s.as_ptr(), builtins) + } == -1 + { + return Err(PyErr::fetch(self)); + } } - } + Ok(()) + })?; } let code_obj = unsafe { @@ -1018,4 +1021,46 @@ mod tests { assert!(matches!(namespace.get_item("__builtins__"), Ok(Some(..)))); }) } + + #[cfg(feature = "macros")] + #[test] + fn test_py_run_inserts_globals_2() { + #[crate::pyclass(crate = "crate")] + #[derive(Clone)] + struct CodeRunner { + code: CString, + } + + impl CodeRunner { + fn reproducer(&mut self, py: Python<'_>) -> PyResult<()> { + let variables = PyDict::new(py); + variables.set_item("cls", Py::new(py, self.clone())?)?; + + py.run(self.code.as_c_str(), Some(&variables), None)?; + Ok(()) + } + } + + #[crate::pymethods(crate = "crate")] + impl CodeRunner { + fn func(&mut self, py: Python<'_>) -> PyResult<()> { + py.import("math")?; + Ok(()) + } + } + + let mut runner = CodeRunner { + code: CString::new( + r#" +cls.func() +"# + .to_string(), + ) + .unwrap(), + }; + + Python::with_gil(|py| { + runner.reproducer(py).unwrap(); + }); + } }