GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-157184: Scale the join() alarm in the multiprocessing kill tests by iamsharduld · Pull Request #157185 · python/cpython

Conversation

@iamsharduld

@iamsharduld iamsharduld commented Sep 8, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

_kill_process() guards the join() of the killed child with a SIGALRM so that a blocked waitpid() becomes a readable error instead of a hang. The alarm has been a literal 10 seconds 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 interrupted os.waitpid() itself.

test.support.LONG_TIMEOUT is 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 --timeout for slow workers. The SHORT_TIMEOUT docs point the same way: "If a test using SHORT_TIMEOUT starts to fail randomly on slow buildbots, use LONG_TIMEOUT instead." _kill_process() already uses support.SHORT_TIMEOUT a few lines above for its event wait. math.ceil() keeps the alarm from being cancelled outright if a very short --timeout scales the value below one second.

Four tests go through this helper, in every start-method variant: test_interrupt, test_interrupt_no_handler, test_terminate and test_kill.

Verified by making the child slow to die (a SIGINT handler that sleeps 12 seconds), which is what a loaded machine looks like from the parent's side, and running the real test_interrupt. On Linux with the fork start method, the configuration that failed on CI, and on macOS with spawn:

Linux, fork macOS, spawn
signal.alarm(10) join took too long — FAILURE join took too long — FAILURE
this change SUCCESS SUCCESS

Without the injected delay, test_multiprocessing_spawn, test_multiprocessing_forkserver, test_multiprocessing_fork, test_multiprocessing_main_handling and test_concurrent_futures pass (1427 tests).

  • Issue: _test_multiprocessing._kill_process() uses a fixed 10 s alarm, failing on slow builds #157184

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@iamsharduld
pythongh-157184: Scale the join() alarm in the multiprocessing kill t…
fe2551d
…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.
@iamsharduld
iamsharduld requested a review from gpshead as a code owner September 8, 2026 13:58
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Sep 8, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Sep 8, 2026
_test_multiprocessing._kill_process() uses a fixed 10 s alarm, failing on slow builds #157184
Closed
@bedevere-app bedevere-app Bot added the awaiting review label Sep 8, 2026
sharktide
sharktide reviewed Sep 8, 2026
View reviewed changes

@sharktide sharktide left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@iamsharduld
Remove the NEWS entry
f1e0152
@iamsharduld

iamsharduld commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Nothing changed; you are right and I was wrong to add one. The devguide lists test changes among the cases that do not need a NEWS entry, and on a sibling PR of mine (#157158) @kumaraditya303 asked for the entry to be removed and applied the skip news label. I have removed it here too, so bedevere/news will need that label from a triager.

The same applies to my other test-only PRs that still carry an entry (#157162, #157171); I will leave those alone unless a triager prefers them removed as well, to avoid churn on PRs nobody has looked at yet.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Oct 22, 2026

Copy link
Copy Markdown
Member

I wrote a different fix which logs the timeout in the error message: #160281. I close this PR, thanks for your contribution anyway!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner vstinner closed this Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner
1 more reviewer
@sharktide sharktide sharktide left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@iamsharduld @vstinner @sharktide
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.