bpo-39847: win32: don't over-wait for mutex after tickcount overflow #18780
Conversation
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
The 32-bit (49-day) TickCount relied on in EnterNonRecursiveMutex can overflow in the gap between the 'target' time and the 'now' time WaitForSingleObjectEx returns, causing the loop to think it needs to wait another 49 days. This is most likely to happen when the machine is hibernated during WaitForSingleObjectEx. This makes acquiring a lock/event/etc from the _thread or threading module appear to never timeout. Replace with GetTickCount64 - this is OK now we no longer support XP which lacks it, and is in use for time.monotonic. Approaches not taken: - forcing signed arithmetic for the now/target comparison (this would only partially solve the problem with GetTickCount as you could still break it by hibernating for 24 days; it's unnecessary for 64-bit which takes however many hundred million years to roll over)
181d6de to
1026d8d
Compare
|
cc @pitrou who loves lock if I recall correctly :-D |
|
Sorry @bobince and @vstinner, I had trouble checking out the |
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Mar 11, 2020
…8780) The 32-bit (49-day) TickCount relied on in EnterNonRecursiveMutex can overflow in the gap between the 'target' time and the 'now' time WaitForSingleObjectEx returns, causing the loop to think it needs to wait another 49 days. This is most likely to happen when the machine is hibernated during WaitForSingleObjectEx. This makes acquiring a lock/event/etc from the _thread or threading module appear to never timeout. Replace with GetTickCount64 - this is OK now Python no longer supports XP which lacks it, and is in use for time.monotonic(). Co-authored-by: And Clover <and.clover@bromium.com> (cherry picked from commit 64838ce) Co-authored-by: bobince <and+github@doxdesk.com>
|
GH-18945 is a backport of this pull request to the 3.8 branch. |
miss-islington
added a commit
that referenced
this pull request
Mar 11, 2020
The 32-bit (49-day) TickCount relied on in EnterNonRecursiveMutex can overflow in the gap between the 'target' time and the 'now' time WaitForSingleObjectEx returns, causing the loop to think it needs to wait another 49 days. This is most likely to happen when the machine is hibernated during WaitForSingleObjectEx. This makes acquiring a lock/event/etc from the _thread or threading module appear to never timeout. Replace with GetTickCount64 - this is OK now Python no longer supports XP which lacks it, and is in use for time.monotonic(). Co-authored-by: And Clover <and.clover@bromium.com> (cherry picked from commit 64838ce) Co-authored-by: bobince <and+github@doxdesk.com>
bobince
added a commit
to bobince/cpython
that referenced
this pull request
Mar 12, 2020
…onGH-18780) The 32-bit (49-day) TickCount relied on in EnterNonRecursiveMutex can overflow in the gap between the 'target' time and the 'now' time WaitForSingleObjectEx returns, causing the loop to think it needs to wait another 49 days. This is most likely to happen when the machine is hibernated during WaitForSingleObjectEx. This makes acquiring a lock/event/etc from the _thread or threading module appear to never timeout. Replace with GetTickCount64 - this is OK now Python no longer supports XP which lacks it, and is in use for time.monotonic(). Co-authored-by: And Clover <and.clover@bromium.com> (cherry picked from commit 64838ce) Co-authored-by: bobince <and+github@doxdesk.com>
|
GH-18959 is a backport of this pull request to the 3.7 branch. |
|
Yep, ta. I couldn't see any conflict with my human eyes but the cherry-pick doesn't care. |
miss-islington
pushed a commit
that referenced
this pull request
Mar 12, 2020
…8780) (GH-18959) The 32-bit (49-day) TickCount relied on in EnterNonRecursiveMutex can overflow in the gap between the 'target' time and the 'now' time WaitForSingleObjectEx returns, causing the loop to think it needs to wait another 49 days. This is most likely to happen when the machine is hibernated during WaitForSingleObjectEx. This makes acquiring a lock/event/etc from the _thread or threading module appear to never timeout. Replace with GetTickCount64 - this is OK now Python no longer supports XP which lacks it, and is in use for time.monotonic(). Co-authored-by: And Clover <and.clover@bromium.com> (cherry picked from commit 64838ce) Co-authored-by: bobince <and+github@doxdesk.com>
Thanks. I merged your 3.7 backport. |
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.
The 32-bit (49-day) TickCount relied on in EnterNonRecursiveMutex can overflow in the gap between the 'target' time and the 'now' time WaitForSingleObjectEx returns, causing the loop to think it needs to wait another 49 days. This is most likely to happen when the machine is hibernated during WaitForSingleObjectEx.
This makes acquiring a lock/event/etc from the _thread or threading module appear to never timeout.
Replace with GetTickCount64. This is OK now we no longer support WinXP which lacks it; it is already in use for time.monotonic.
Approaches not taken:
forcing signed arithmetic for the now/target comparison (this would only partially solve the problem with GetTickCount as you could still break it by hibernating for 24 days; it's unnecessary for 64-bit which takes however many hundred million years to roll over);
doing anything to cope with the possibility of the new milliseconds being out of range for DWORD; this shouldn't be possible given the target-now time should never be more than the original milliseconds DWORD passed in, and PyCOND_TIMEDWAIT can't accept a longer time anyway;
I haven't been able to add a representative test, as it would mean waiting 49 days or some pretty nasty syscall hooking which seems like overkill. Hopefully change is fairly transparently harmless.
https://bugs.python.org/issue39847