Skip to content

gh-106558: Fix multiprocessing manager exception ref cycle. #106559

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

Merged
merged 9 commits into from
Aug 11, 2023
Merged

gh-106558: Fix multiprocessing manager exception ref cycle. #106559

merged 9 commits into from
Aug 11, 2023

Conversation

pteromys
Copy link
Contributor

@pteromys pteromys commented Jul 9, 2023

If kind == '#ERROR', then result is an exception whose attached traceback includes the frame that calls convert_to_error() and has result in scope. So delete result from the scope as we raise it, to save the consumer of these exceptions from having to cope with a buildup of cyclic references.

(This is the same try/finally/del pattern that logging.Handler.handleError uses and the docs for the except E as N syntax hint at.)

Closes #106558.

If `kind == '#ERROR'`, then `result` is an exception whose attached
traceback includes the frame that calls `convert_to_error()` and has
`result` in scope. So delete `result` from the scope as we raise it,
to save the consumer of these exceptions from having to cope with
a buildup of cyclic references.
@iritkatriel
Copy link
Member

Yes, we need to do this sometimes with exceptions because the traceback cycles back to the exception through the frame locals sigh.

@iritkatriel
Copy link
Member

Can you add a unit test based on your example from the issue?

Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've confirmed that the test fails on main. Just a suggestion to move the comment into the class, so they stay together as things move around.

@ambv
Copy link
Contributor

ambv commented Aug 11, 2023

Closing and re-opening to retrigger CLA checks. Sorry for the noise.

@ambv ambv closed this Aug 11, 2023
@ambv ambv reopened this Aug 11, 2023
@iritkatriel iritkatriel enabled auto-merge (squash) August 11, 2023 16:43
@iritkatriel iritkatriel merged commit 5f7d4ec into python:main Aug 11, 2023
@pteromys pteromys deleted the fix-issue-106558 branch September 4, 2023 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multiprocessing Manager exceptions create cyclic references
4 participants