Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leak when calling Python from Rust #1806

Merged
merged 3 commits into from
Aug 18, 2021

Conversation

pschafhalter
Copy link
Contributor

@pschafhalter pschafhalter commented Aug 17, 2021

Python::run_code transforms the Rust code string to an owned pointer. The reference count to the owned pointer isn't decremented, which results in a memory leak when calling Python from Rust.

This PR adds a missing Py_DECREF call on the owned pointer which fixes the memory leak.

Fixes #1801.

Thanks to @mejrs for the tip on the fix.

Copy link
Member

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

Thanks!

The owned pointer isn't dereferenced

That's not what Py_DECREF does (see https://docs.python.org/3/c-api/refcounting.html for more on refcounting).

What happened here was that the raw pointer never had its reference count decremented, so the inner value was leaked and never deallocated. It's similar to mem::forget'ing a Rc in Rust.

@pschafhalter
Copy link
Contributor Author

Thanks for the correction! Updated the comment on the PR

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@birkenfeld birkenfeld left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! @davidhewitt needs cherry-pick to 0.14...

Copy link
Member

@messense messense left a comment

Choose a reason for hiding this comment

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

Thanks!

@messense messense enabled auto-merge August 18, 2021 06:28
@messense messense merged commit 78b924d into PyO3:main Aug 18, 2021
@messense
Copy link
Member

auto-merge works! 🎉

@mejrs mejrs mentioned this pull request Aug 20, 2021
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.

Potential memory leak when calling Python from Rust
4 participants