gh-129280: Avoid cyclic reference in contextlib.contextmanager by user202729 · Pull Request #129276 · python/cpython
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to make the appropriate change in asynccontextmanager, and add a test
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think that this is the right fix. Should not all local variables be cleared after leaving the function? If they are not, we have a larger issue.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the traceback of the exception refers to the local variables
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fix here is to wrap this whole __exit__ in a:
try: ... finally: del value, traceback
exc should be cleared automatically by leaving the except block
With a test case this should be a lot clearer, (have a look at the cyclic exception tests in test_taskgroup, (the ones that call no_other_refs)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@graingert or maybe this should be done one lever higher? Do other __exit__s have the same problem?
I agree with @serhiy-storchaka, this fix is not correct.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be pretty magical to automatically delete value and traceback from every __exit__ and __aexit__
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this was not an issue in 3.11?
I agree that finally with dels is better solution, but we should look if there is more fundamental issue, which affects not only contextmanager.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't entirely know why it was not an issue in Python 3.11, but concretely, if you add print(self.__traceback__.tb_frame.f_back.f_locals) in __del__ method, in Python 3.12 it prints out { ... 'value': MyException() ... } while in Python 3.11 it prints out AttributeError: 'NoneType' object has no attribute 'f_locals'.