{{ message }}
gh-157184: Scale the join() alarm in the multiprocessing kill tests - #157185
Open
iamsharduld wants to merge 2 commits into
Open
gh-157184: Scale the join() alarm in the multiprocessing kill tests#157185iamsharduld wants to merge 2 commits into
iamsharduld wants to merge 2 commits into
Conversation
…ests _kill_process() interrupts the join() of the killed child with SIGALRM so that a blocked waitpid() becomes a readable error instead of a hang, but the alarm has been a fixed 10 seconds since 2013. On a build slow enough that reaping the child legitimately takes longer, it fires on a healthy run: seen on the UBSan CI job, where the alarm interrupted os.waitpid() itself. Use support.LONG_TIMEOUT, which is documented for detecting hangs and is scaled by regrtest for slow workers, as the same function already does for its event wait.
sharktide
reviewed
Sep 8, 2026
sharktide
left a comment
Contributor
There was a problem hiding this comment.
The code implementation looks good to me, although I've been gone a few months and coming back, I don't remember if we used to do news entries on purely test fail fixes which are non-user facing changes. If something changed recently, please let me know!
Contributor
Author
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
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.

_kill_process()guards thejoin()of the killed child with aSIGALRMso that a blockedwaitpid()becomes a readable error instead of a hang. The alarm has been a literal10seconds since it was added in 2013 (cc5c728), so on a build slow enough that reaping the child legitimately takes longer than that, it fires on a healthy run and fails the test. That is what happened on the UBSan job of 57594aa (traceback in the issue): the alarm interruptedos.waitpid()itself.test.support.LONG_TIMEOUTis documented for this exact purpose, "Timeout in seconds to detect when a test hangs [...] It should not be used to mark a test as failed if the test takes 'too long'", and regrtest scales it from--timeoutfor slow workers. TheSHORT_TIMEOUTdocs point the same way: "If a test usingSHORT_TIMEOUTstarts to fail randomly on slow buildbots, useLONG_TIMEOUTinstead."_kill_process()already usessupport.SHORT_TIMEOUTa few lines above for its event wait.math.ceil()keeps the alarm from being cancelled outright if a very short--timeoutscales the value below one second.Four tests go through this helper, in every start-method variant:
test_interrupt,test_interrupt_no_handler,test_terminateandtest_kill.Verified by making the child slow to die (a
SIGINThandler that sleeps 12 seconds), which is what a loaded machine looks like from the parent's side, and running the realtest_interrupt. On Linux with the fork start method, the configuration that failed on CI, and on macOS with spawn:signal.alarm(10)join took too long— FAILUREjoin took too long— FAILUREWithout the injected delay,
test_multiprocessing_spawn,test_multiprocessing_forkserver,test_multiprocessing_fork,test_multiprocessing_main_handlingandtest_concurrent_futurespass (1427 tests).