Skip to content
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

atexit: move hook before precompile output #51849

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Oct 24, 2023

To show how this is used, this updates the profile_printing_listener background job to use this mechanism.

Copy link
Sponsor Member

@JeffBezanson JeffBezanson left a comment

Choose a reason for hiding this comment

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

I think this is the right thing to do. But noting that we should properly expose _wait and _trywait since there are clearly good use cases for them.

@vtjnash

This comment was marked as outdated.

@vtjnash vtjnash force-pushed the jn/earlier-atexit-hook branch 2 times, most recently from 99b2d96 to 5a0a467 Compare October 25, 2023 21:07
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 31, 2023
@vtjnash vtjnash removed the merge me PR is reviewed. Merge when all tests are passing label Nov 2, 2023
@vtjnash vtjnash added merge me PR is reviewed. Merge when all tests are passing and removed merge me PR is reviewed. Merge when all tests are passing labels Nov 8, 2023
Exposed by the presence of any atexit hooks in the new process, such as added in this PR.
To show how this is used, this updates the profile_printing_listener
background job to use this mechanism.

Also ensure mktemp and mktempdir cleanup even on unexpected exit:
Previously this was relying on all Tasks running to completion, which is
not a good assumption. Add the atexit hook, so that even if they do not
run to completion, they still get cleaned up.
@vtjnash vtjnash added merge me PR is reviewed. Merge when all tests are passing don't squash Don't squash merge labels Nov 14, 2023
@vtjnash vtjnash merged commit 539ca89 into master Nov 15, 2023
7 of 9 checks passed
@vtjnash vtjnash deleted the jn/earlier-atexit-hook branch November 15, 2023 01:34
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Nov 15, 2023
@johnnychen94
Copy link
Sponsor Member

johnnychen94 commented Nov 16, 2023

Just asking, will this be backported to 1.10?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
don't squash Don't squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants