gh-74929: Fix an extra DECREF for PEP 667 implementation by gaogaotiantian · Pull Request #118583 · python/cpython
Conversation
In the PEP 667 implementation, when I copied the code from _PyFrame_LocalsToFast, I did not realize the value is 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:
- local object variable - this will trigger an assertion failure in debug mode
f_localsupdating itself multiple times - this will crash the interpreter without the fix
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?
| self.assertEqual(o, 'a.b.c') | ||
|
|
||
| def test_update_with_self(self): | ||
| # Make sure reference is not leaking here |
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 :-).
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.
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.
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!
Labels
2 participants
