diff --git a/newsfragments/6234.fixed.md b/newsfragments/6234.fixed.md new file mode 100644 index 00000000000..a5df69a8296 --- /dev/null +++ b/newsfragments/6234.fixed.md @@ -0,0 +1 @@ +Fix a memory leak on Python 3.11 and 3.12 where creating an instance of a `#[pyclass(dict)]` class leaked one empty dict per instance. \ No newline at end of file diff --git a/src/pyclass_init.rs b/src/pyclass_init.rs index 8e996d9b541..6b8ac028222 100644 --- a/src/pyclass_init.rs +++ b/src/pyclass_init.rs @@ -155,9 +155,38 @@ impl PyClassInitializer { // SAFETY: `obj` is constructed using `T::Layout` but has not been initialized yet let contents = unsafe { ::Layout::contents_uninit(obj) }; + + let new_contents = PyClassObjectContents::new(self.init); + + // CPython 3.11 and 3.12 eagerly create the instance dict for types with a nonzero + // `tp_dictoffset` in `_PyObject_InitializeDict`, storing an owned reference in + // the `__dict__` slot, which lives inside `contents`. Carry that value over + // instead of clobbering it below, otherwise it leaks. Python 3.13 returned to + // creating the instance dict lazily + // + // The condition is `not(Py_3_13)` rather than `all(Py_3_11, not(Py_3_13))` + // because an abi3 build with a lower minimum version can still run on 3.11 and + // 3.12; on 3.10 and older this is a harmless no-op (the slot is always null + // there). + #[cfg(not(Py_3_13))] + let new_contents = { + let mut new_contents = new_contents; + if eagerly_created_dict_possible::(py) { + // SAFETY: `tp_alloc` zero-initializes the object, so the slot contains either + // zeroes (a valid empty slot value) or a valid owned pointer stored by the base + // `tp_new` through the type's `tp_dictoffset`. + unsafe { + let contents_ptr = (*contents).as_mut_ptr(); + let dict_ptr = &raw const (*contents_ptr).dict; + new_contents.dict = core::ptr::read(dict_ptr); + } + } + new_contents + }; + // SAFETY: `contents` is a non-null pointer to the space allocated for our // `PyClassObjectContents` (either statically in Rust or dynamically by Python) - unsafe { (*contents).write(PyClassObjectContents::new(self.init)) }; + unsafe { (*contents).write(new_contents) }; // Safety: obj is a valid pointer to an object of type `target_type`, which` is a known // subclass of `T` @@ -165,6 +194,35 @@ impl PyClassInitializer { } } +/// Whether the running interpreter may have eagerly created an instance dict for `T` +/// during `tp_new` (CPython 3.11 and 3.12 only). +/// +/// For native builds the compile-time `not(Py_3_13)` gate at the call site is exact; abi3 +/// builds with a minimum version below 3.13 must check the interpreter version at runtime +#[cfg(not(Py_3_13))] +#[inline] +fn eagerly_created_dict_possible(py: Python<'_>) -> bool { + if core::mem::size_of::() == 0 { + return false; + } + cfg_select! { + Py_LIMITED_API => + { + use crate::sync::PyOnceLock; + static IS_PYTHON_3_11_OR_3_12: PyOnceLock = PyOnceLock::new(); + *IS_PYTHON_3_11_OR_3_12.get_or_init(py, || { + let version_info = py.version_info(); + matches!((version_info.major, version_info.minor), (3, 11) | (3, 12)) + }) + } + not(Py_LIMITED_API) => + { + let _ = py; + cfg!(Py_3_11) + } + } +} + impl PyObjectInit for PyClassInitializer { unsafe fn into_new_object( self, diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index ad0b7cf193c..6a840455b70 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -502,6 +502,57 @@ fn dunder_dict_is_released() { }); } +// The `__dict__` slot must hold exactly what CPython's `tp_new` left there: CPython 3.11 +// and 3.12 eagerly create the instance dict in `object.__new__` for types with a nonzero +// `tp_dictoffset`, and the pyclass contents initialization must preserve it rather than +// clobber it. All other versions create the dict lazily on first access. +#[cfg(all(not(Py_LIMITED_API), not(PyPy), not(GraalPy)))] +#[test] +fn instance_dict_slot_is_not_clobbered() { + Python::attach(|py| { + let inst = Py::new( + py, + DunderDictSupport { + _pad: *b"DEADBEEFDEADBEEFDEADBEEFDEADBEEF", + }, + ) + .unwrap(); + + // Read the `__dict__` slot directly (see `_PyObject_GetDictPtr`), *without* + // going through `__dict__`. + // SAFETY: `inst` is a valid object whose type has a positive `tp_dictoffset`. + let read_slot = || unsafe { + let offset = (*pyo3::ffi::Py_TYPE(inst.as_ptr())).tp_dictoffset; + assert!(offset > 0); + *inst + .as_ptr() + .cast::() + .offset(offset) + .cast::<*mut pyo3::ffi::PyObject>() + }; + + let slot_dict = read_slot(); + if cfg!(all(Py_3_11, not(Py_3_13))) { + // `object.__new__` created the dict; it must survive pyclass initialization. + assert!( + !slot_dict.is_null(), + "the eagerly created __dict__ was clobbered during pyclass initialization" + ); + // The slot holds the only reference to it. + // SAFETY: previous assert guarantees it's a valid PyObject + assert_eq!(unsafe { pyo3::ffi::Py_REFCNT(slot_dict) }, 1); + } else { + // No eager creation on these versions; the slot starts out empty. + assert!(slot_dict.is_null()); + } + + // Whichever way the dict comes into existence, `__dict__` must be the dict + // stored in the slot. + let dict_attr = inst.bind(py).getattr("__dict__").unwrap(); + assert_eq!(read_slot(), dict_attr.as_ptr()); + }); +} + // If the base class has dict support, child class also has dict #[pyclass(extends=DunderDictSupport)] struct InheritDict {