Skip to content

fix: release pyclass dict during deallocation - #6198

Merged
ngoldbaum merged 1 commit into
PyO3:mainfrom
ImFeH2:issue-5953
Jul 25, 2026
Merged

fix: release pyclass dict during deallocation#6198
ngoldbaum merged 1 commit into
PyO3:mainfrom
ImFeH2:issue-5953

Conversation

@ImFeH2

@ImFeH2 ImFeH2 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #5953.

PyClassDictSlot::clear_dict previously called PyDict_Clear, which removed the contents of __dict__ but did not release the pyclass instance's reference to the dictionary itself. This leaked the empty dictionary when the instance was deallocated.

Use Py_CLEAR to clear the slot and decrement the dictionary's reference count.

@codspeed-hq

codspeed-hq Bot commented Jul 11, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 140 untouched benchmarks


Comparing ImFeH2:issue-5953 (ca796ad) with main (df72d66)

Open in CodSpeed

@ImFeH2

ImFeH2 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

The reported into_u128_max regression appears unrelated to this change, which only affects #[pyclass(dict)] deallocation. CodSpeed also reports that the comparison used different runtime environments.

@ngoldbaum ngoldbaum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a couple minor nits. I agree that this fixes a leak.

Comment thread src/impl_/pyclass.rs Outdated
Comment thread tests/test_class_basics.rs
@ngoldbaum

Copy link
Copy Markdown
Contributor

I opened #6234 which fixes a related issue that only happens on Python 3.11 and 3.12, I'd appreciate your eyes on that and the test I came up with there.

@ngoldbaum
ngoldbaum enabled auto-merge July 25, 2026 13:30
@ngoldbaum
ngoldbaum added this pull request to the merge queue Jul 25, 2026
Merged via the queue into PyO3:main with commit 1ed13a0 Jul 25, 2026
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory leak on dict pyclasses after setting dynamic attribute

2 participants