GeezerVison is showing github.com — [3.15] gh-161712: Read the clock once in ThreadHandle_join() by aozmekik · Pull Request #161771 · python/cpython
Conversation
aozmekik
commented
Nov 10, 2026
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Py_MAX() evaluates _PyDeadline_Get() twice. If the deadline passes between the two calls, the first value passes the > 0 check and the second, negative one becomes the timeout, which PyEvent_WaitTimed() takes as no timeout, so Thread.join(timeout) blocks forever. main has read the clock once since gh-157495 (GH-157591), which wasn't backported because it was a cleanup. This is that hunk, with a NEWS entry.
The window is too small to hit from a test, so I forced it under gdb: a signal wakes the join loop of join(3.0) after a second, and the second clock read in that loop reports the deadline as passed. On 3.15 as it is, the join was still blocked after 30 seconds. With this change it returns after 3 seconds. test_threading and test_thread pass.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Py_MAX() evaluated _PyDeadline_Get() twice. If the deadline passed
between the two calls, the negative second value was used as the
timeout, which means no timeout, and Thread.join(timeout) could block
forever. main already reads it once since pythongh-157495.
|
This looks good, but please open a new PR against the main branch; even though main isn't affected, it's nice to have consistency across branches (and it makes backporting easier). I think there was talk of reverting #157591 anyway, so it'd be good to have future-proofing in place (especially if there's no test case).
I'm going to close this so you're not tempted to change the base or head branch (doing so would ping all the codeowners!). Feel free to tag me on the new PR, and I'll merge it.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, I didn't realize that this already happened on main a little while ago.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks @aozmekik for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
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.
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.
|