Make IO_fork children stoppable when GAP is embedded - #137
Merged
Conversation
A program embedding GAP may block termination signals, set them to SIG_IGN, or catch them with handlers that depend on threads which do not survive a fork; julia (GAP.jl) does all three. Children created by IO_fork then cannot be stopped by IO_kill with SIGTERM, so e.g. the test suites of curlInterface and utils, which fork an HTTP test server, hang in IO_WaitPid until killed from outside. Reset the mask and dispositions of the common termination signals before forking (restoring them in the parent afterwards; resetting in the child instead would race against a parent that signals the child immediately, and a SIGTERM arriving before the reset would be discarded). Caught handlers are reset to the default action, SIG_IGN is kept except for SIGTERM, whose delivery is part of IO_fork's contract. Also make IO_WaitPid return instead of looping forever when waitpid reports ECHILD, i.e. when the waited-for process is not an unreaped child of ours. Add a regression test; it fails under GAP.jl without the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With line-by-line profiling active (as in CI coverage runs), GAP opens a separate profile file for each forked child. The child in forksignal.tst is killed by SIGTERM, so its file stayed truncated and the coverage merge failed with "Some profiles are covers, some are time profiles". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Closing the inherited profile output in the child raced against the parent's SIGTERM; on CI runners the parent won, leaving the truncated per-child coverage file again. Let the child report readiness through a pipe before it is signalled. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
2 participants
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 program embedding GAP may block termination signals, set them to
SIG_IGN, or catch them with handlers that depend on threads which do not survive a fork; julia (GAP.jl) does all three. Children created byIO_forkthen cannot be stopped byIO_killwithSIGTERM, so e.g. the test suites of curlInterface and utils, which fork an HTTP test server, hang inIO_WaitPiduntil killed from outside.Reset the mask and dispositions of the common termination signals before forking (restoring them in the parent afterwards; resetting in the child instead would race against a parent that signals the child immediately, and a
SIGTERMarriving before the reset would be discarded). Caught handlers are reset to the default action,SIG_IGNis kept except forSIGTERM, whose delivery is part ofIO_fork's contract.Also make
IO_WaitPidreturn instead of looping forever whenwaitpidreportsECHILD, i.e. when the waited-for process is not an unreaped child of ours.Add a regression test; it fails under GAP.jl without the fix.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com