diff --git a/newsfragments/6198.fixed.md b/newsfragments/6198.fixed.md new file mode 100644 index 00000000000..79b5ebd6e85 --- /dev/null +++ b/newsfragments/6198.fixed.md @@ -0,0 +1 @@ +Fix a memory leak when deallocating `#[pyclass(dict)]` instances with a populated `__dict__`. diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index aafd2163ef0..c1d911c6388 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -65,6 +65,9 @@ pub trait PyClassDict: sealed::Sealed { /// Empties the dictionary of its key-value pairs. #[inline] fn clear_dict(&self, _py: Python<'_>) {} + /// Releases the owned reference to the dictionary. + #[inline] + fn release_dict(&mut self, _py: Python<'_>) {} /// Visits the `__dict__`, if any, on behalf of `tp_traverse`. /// /// # Safety @@ -115,6 +118,10 @@ impl PyClassDict for PyClassDictSlot { } } #[inline] + fn release_dict(&mut self, _py: Python<'_>) { + unsafe { ffi::Py_CLEAR(&raw mut self.0) } + } + #[inline] unsafe fn traverse_dict(&self, visit: ffi::visitproc, arg: *mut c_void) -> c_int { if self.0.is_null() { 0 diff --git a/src/pycell/impl_.rs b/src/pycell/impl_.rs index cc2ef775baa..ad765121f0a 100644 --- a/src/pycell/impl_.rs +++ b/src/pycell/impl_.rs @@ -380,7 +380,7 @@ impl PyClassObjectContents { if self.thread_checker.can_drop(py) { unsafe { ManuallyDrop::drop(&mut self.value) }; } - self.dict.clear_dict(py); + self.dict.release_dict(py); unsafe { self.weakref.clear_weakrefs(py_object, py) }; } } diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index ae9c71bf3e4..ad0b7cf193c 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -475,6 +475,33 @@ fn access_dunder_dict() { }); } +#[test] +fn dunder_dict_is_released() { + Python::attach(|py| { + let inst = Py::new( + py, + DunderDictSupport { + _pad: *b"DEADBEEFDEADBEEFDEADBEEFDEADBEEF", + }, + ) + .unwrap(); + + inst.setattr(py, "a", 1).unwrap(); + + let dict = inst.bind(py).getattr("__dict__").unwrap(); + let get_refcnt = || { + // SAFETY: `dict` holds a valid reference while its reference count is read. + unsafe { pyo3::ffi::Py_REFCNT(dict.as_ptr()) } + }; + let refcnt = get_refcnt(); + + drop(inst); + + assert_eq!(get_refcnt(), refcnt - 1); + py_assert!(py, dict, "dict == {'a': 1}"); + }); +} + // If the base class has dict support, child class also has dict #[pyclass(extends=DunderDictSupport)] struct InheritDict {