Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
gh-74929: Fix an extra DECREF for PEP 667 implementation#118583
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-74929: Fix an extra DECREF for PEP 667 implementation#118583
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe you want to delete or update the comment in the test?
Lib/test/test_frame.py Outdated
| self.assertEqual(o, 'a.b.c') | ||
| def test_update_with_self(self): | ||
| # Make sure reference is not leaking here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The problem was not a leak though -- it was a free-after-use (the opposite of a leak, really :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Right. I was imaging a pool of reference and a hole that's leaking the reference without people knowing it so the pool is drained, but yeah leak is not a good word choice here. I just deleted it, the test itself makes sense without any comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ooh, that's actually a nice image. :-) I will merge now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for the quick review!
Uh oh!
There was an error while loading.Please reload this page.
In the PEP 667 implementation, when I copied the code from
_PyFrame_LocalsToFast, I did not realize thevalueis a new reference so it was decrefed in the original code. This resulted in an extra DECREF in__setitem__. Unfortunately I did not catch it because the test cases used were all immortals.Two test cases were added:
f_localsupdating itself multiple times - this will crash the interpreter without the fix