Repository navigation
[3.15] gh-161712: Read the clock once in ThreadHandle_join() - #161771
Conversation
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. |
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
Sorry, I didn't realize that this already happened on main a little while ago.
|
Thanks @aozmekik for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
GH-161843 is a backport of this pull request to the 3.14 branch. |
Py_MAX()evaluates_PyDeadline_Get()twice. If the deadline passes between the two calls, the first value passes the> 0check and the second, negative one becomes the timeout, whichPyEvent_WaitTimed()takes as no timeout, soThread.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.