Skip to content
Merged
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/6198.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a memory leak when deallocating `#[pyclass(dict)]` instances with a populated `__dict__`.
7 changes: 7 additions & 0 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/pycell/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ impl<T: PyClassImpl> PyClassObjectContents<T> {
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) };
}
}
Expand Down
27 changes: 27 additions & 0 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Comment thread
ImFeH2 marked this conversation as resolved.
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 {
Expand Down