Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/6234.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
55 changes: 54 additions & 1 deletion src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,69 @@ impl<T: PyClass> PyClassInitializer<T> {

// SAFETY: `obj` is constructed using `T::Layout` but has not been initialized yet
let contents = unsafe { <T as PyClassImpl>::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 = {
Comment on lines +171 to +172

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For abi3 builds should we also consider doing a runtime gate? e.g. otherwise a 3.12 abi3 build still does this twiddling on 3.13+. It's probably harmless to have it not gated, but might be good to be thorough. (See bpo_45315_workaround as an example.)

Also I wonder if this can somehow be skipped for cases when T::Dict is PyClassDummySlot (i.e. no dict).

@ngoldbaum ngoldbaum Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a eagerly_created_dict_possible helper that does the limited API runtime python version check and and the check for a PyClassDummySlot.

let mut new_contents = new_contents;
if eagerly_created_dict_possible::<T>(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`
Ok(unsafe { obj.assume_owned(py).cast_into_unchecked() })
}
}

/// 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<T: PyClassImpl>(py: Python<'_>) -> bool {
if core::mem::size_of::<T::Dict>() == 0 {
return false;
}
#[cfg(Py_LIMITED_API)]
{
use crate::sync::PyOnceLock;
static IS_PYTHON_3_13: PyOnceLock<bool> = PyOnceLock::new();
!*IS_PYTHON_3_13.get_or_init(py, || py.version_info() >= (3, 13))
}
#[cfg(not(Py_LIMITED_API))]
{
let _ = py;
true
}
Comment on lines +208 to +218

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight preference to use cfg_select! here for newer conditional code. Also now that we're differentiating abi3 and non-abi3 pathways we can more properly account for the < 3.11 cases, maybe as the following:

Suggested change
#[cfg(Py_LIMITED_API)]
{
use crate::sync::PyOnceLock;
static IS_PYTHON_3_13: PyOnceLock<bool> = PyOnceLock::new();
!*IS_PYTHON_3_13.get_or_init(py, || py.version_info() >= (3, 13))
}
#[cfg(not(Py_LIMITED_API))]
{
let _ = py;
true
}
cfg_select! {
Py_LIMITED_API => {
use crate::sync::PyOnceLock;
static IS_PYTHON_3_11_OR_3_12: PyOnceLock<bool> = PyOnceLock::new();
IS_PYTHON_3_11_OR_3_12.get_or_init(py, || {
let version_info = py.version_info();
version_info >= (3, 11) && version_info < (3, 13)
})
},
not(Py_LIMITED_API) => {
let _ = py;
cfg!(Py_3_11)
}
}

}

impl<T: PyClass> PyObjectInit<T> for PyClassInitializer<T> {
unsafe fn into_new_object(
self,
Expand Down
51 changes: 51 additions & 0 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<u8>()
.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 {
Expand Down