Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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.
29 changes: 28 additions & 1 deletion src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,36 @@ 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;
// 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 = core::ptr::addr_of!((*contents_ptr).dict);
new_contents.dict = core::ptr::read(dict_ptr);
Comment thread
davidhewitt marked this conversation as resolved.
Outdated
}
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`
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