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

py: fix reference count bug in From(Py<T>) for PyObject #1297

Merged
merged 1 commit into from
Nov 28, 2020

Conversation

davidhewitt
Copy link
Member

I have just discovered a reference counting issue in one of our From implementations. It leads to a reference count being decreased when it should not. Unfortunately this was an implementation introduced by me since 0.12 🤦

The offending line is here:

let Py(ptr, _) = other;

I incorrectly thought the destructuring would mean that Py::drop() is not run for other. What actually happens is that ptr implements Copy, so it is copied from other, and then other is dropped.

This means ptr has a reference count decreased when it should not.

This PR corrects the issue and adds a test. The test fails on master.

cc @kngwyu - I think I will have to backport a bugfix 0.12.4 and yank the rest of the 0.12 releases.

@kngwyu
Copy link
Member

kngwyu commented Nov 28, 2020

Good catch, thank you!

What actually happens is that ptr implements Copy, so it is copied from other, and then other is dropped.

Oops, I also didn't notice this 😨

I think I will have to backport a bugfix 0.12.4 and yank the rest of the 0.12 releases.

👍

@kngwyu kngwyu merged commit 1ee3961 into PyO3:master Nov 28, 2020
@davidhewitt davidhewitt deleted the pyobject-from-py branch December 24, 2021 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants