Skip atexit in forked task children rather than running post-fork handlers - #72035
Draft
seanmuth wants to merge 1 commit into
Draft
Skip atexit in forked task children rather than running post-fork handlers#72035seanmuth wants to merge 1 commit into
seanmuth wants to merge 1 commit into
Conversation
…dlers A forked task child could hang forever if any library registered an atexit handler that isn't fork-safe -- one that holds a native lock or thread that doesn't survive fork(). Task-SDK's custom exit path ran these handlers via atexit._run_exitfuncs() before calling os._exit(), so a single fork-unsafe handler could block that final os._exit() from ever being reached, leaving the child, and the pod hosting it, stuck until the pod's grace period expired. This was hit in production via pyarrow's S3 client finalizer deadlocking in its AWS-CRT teardown after pyiceberg registered it post-fork, but the same failure mode applies to any fork-unsafe atexit handler in any library. There's no way to tell a safe handler from an unsafe one in advance, so skip atexit entirely rather than try to special-case known offenders.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A forked task child could hang forever if any library registered an atexit handler that isn't fork-safe -- one that holds a native lock or thread that doesn't survive
fork().Task-SDK's custom exit path in
_fork_mainclears any atexit handlers inherited from the parent immediately after forking (atexit._clear()), then runs whatever gets registered during the child's own subsequent execution viaatexit._run_exitfuncs()before callingos._exit(). That's the actual gap: a library imported for the first time inside the forked child -- after the clear already ran -- re-registers its own atexit handler into what is by then an empty-then-repopulated registry, and that fresh, post-fork registration is what_run_exitfuncs()executes. The_clear()correctly protects against anything inherited from the parent; it does nothing to protect against a fork-unsafe handler the child registers on its own a moment later.This was hit in production exactly that way:
pyicebergimportspyarrow.fsfor the first time insidetarget()(task execution, i.e. after the clear), which callsatexit.register(ensure_s3_finalized)into the now-repopulating registry. When_run_exitfuncs()later runs it,ensure_s3_finalizeddeadlocks in pyarrow's AWS-CRT teardown -- confirmed via a symbolized gdb backtrace on a live wedged production process (atexit._run_exitfuncs() -> ensure_s3_finalized -> EnsureS3Finalized -> ... -> futex_waitforever).The same failure mode applies to any fork-unsafe atexit handler registered by any library during the child's post-fork execution, not just pyarrow's. This skips atexit entirely in the forked child rather than trying to special-case known offenders -- there's no way to tell a safe handler from an unsafe one in advance.
The prior post-fork-atexit behavior was a deliberate choice (see discussion on the internal tracking ticket) to let task code tidy up after the process finishes. This PR trades that intentional cleanup opportunity for guaranteed exit -- happy to discuss alternatives (e.g. running atexit with a timeout) if that tradeoff needs more thought.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Sonnet 5) following the guidelines