Skip to content

atexit: move hook before precompile output#51849

Merged
vtjnash merged 4 commits intomasterfrom
jn/earlier-atexit-hook
Nov 15, 2023
Merged

atexit: move hook before precompile output#51849
vtjnash merged 4 commits intomasterfrom
jn/earlier-atexit-hook

Conversation

@vtjnash
Copy link
Copy Markdown
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
Copy Markdown
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 force-pushed the jn/earlier-atexit-hook branch from 3bba632 to 5944c7a Compare October 31, 2023 17:14
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 31, 2023
@vtjnash vtjnash force-pushed the jn/earlier-atexit-hook branch from 5944c7a to ec65b41 Compare November 1, 2023 16:57
@vtjnash vtjnash removed the merge me PR is reviewed. Merge when all tests are passing label Nov 2, 2023
@vtjnash vtjnash force-pushed the jn/earlier-atexit-hook branch from ec65b41 to b5ca46c Compare November 8, 2023 15:13
@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 force-pushed the jn/earlier-atexit-hook branch from b5ca46c to d8a410c Compare November 14, 2023 21:09
@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
@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
Copy Markdown
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