gh-129280: Avoid cyclic reference in contextlib.contextmanager

[user202729]

By deleting the cyclic reference between the frame object and the exception object, this allows the objects to be deleted more quickly (it doesn't need to wait until the next GC cycle)

This also has the practical impact of some packages (cysignals) using reference counting to detect whether the exception has been handled, which breaks in the presence of this situation. sagemath/sage#39142

• Issue: Cyclic reference lead to contextlib.contextmanager exception not being deleted #129280

[cpython-cla-bot]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[ZeroIntensity]

At an overview:

• This seems issue-worthy to me; could you make one, and then link to it in the PR title?
• Per the bot, we do need a blurb entry, as this is a user-facing change.
• A test case would be good too (see the devguide).

However, going back to the use-case:

This also has the practical impact of some packages (cysignals) using reference counting to detect whether the exception has been handled

I'd be very careful with that. We don't have any guarantees about reference counting, especially with built-in objects. (For example, on the free-threaded build, we have plenty of reference counting hacks that allow for multithreaded scalability--I suspect this kind of thing would break over there.)

[user202729]

We don't have any guarantees about reference counting,

I know that very well. But…

6 years ago, there was a difficult to debug issue with SageMath sagemath/sage#24986 , and the developers concluded using that hacky reference counting solution is the best available workaround.

Turns out it causes additional issues later: sagemath/sage#39224

There exists a better solution, see sagemath/cysignals#215 (comment) , but while it is not yet implemented (there are a lot of places that need to be changed in the code base) I try to make the best of what is available.

If the developers originally implemented the proper solution, we wouldn't be in this situation. But removing the workaround entirely before implementing the proper solution seems impractical at this point.

---

(in the meantime, I'll try to figure out how to write test case and blurb, but I'd appreciate any help if possible)

[ZeroIntensity]

(in the meantime, I'll try to figure out how to write test case and blurb, but I'd appreciate any help.)

Feel free to tag me if you need any help. The devguide should have everything you need, though :)

[graingert]

You'll need to make the appropriate change in asynccontextmanager, and add a test

[serhiy-storchaka]

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.

[graingert]

the traceback of the exception refers to the local variables

[graingert]

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)

[sobolevn]

@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.

[graingert]

It would be pretty magical to automatically delete value and traceback from every __exit__ and __aexit__

[serhiy-storchaka]

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.

[user202729]

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'.