Skip to content

gh-90872: Handle negative timeouts for wait on Windows #32079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jkloth
Copy link
Contributor

@jkloth jkloth commented Mar 23, 2022

Update Windows wait timeout handling to match the behavior for POSIX. This PR also catches overly values that would conflict with the INFINITE timeout value used in _winapi.WaitForSingleObject.

These changes should be backported to the maintenance branches.

https://bugs.python.org/issue46716

@eryksun
Copy link
Contributor

eryksun commented Mar 23, 2022

Did you want to create an issue to fix _winapi.WaitForSingleObject() or the DWORD converter in _winapi? I suppose all call sites of _winapi functions that take DWORD parameters will have to be checked.

@jkloth
Copy link
Contributor Author

jkloth commented Mar 23, 2022

I was thinking to the DWORD converter change as a separate PR that would not be a candidate for backport. While _winapi contents are considered "private", I know that when I write scripts only for use on Windows, I'll reach for that module for functions before considering using ctypes. All the needed "wrapping stuff" (exceptions for errors, reference counting, etc) is handled for me.

The change of negative values to exceptions, while probably not relied upon, would be a breaking change for a dot release. Being limited to a new release (3.11) would be easier to reason about downstream rather than a "random" bugfix release (say, 3.9.12). IMO.

@serhiy-storchaka
Copy link
Member

Please add tests and a NEWS entry.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Feb 23, 2024
@serhiy-storchaka serhiy-storchaka changed the title bpo-46716: Handle negative timeouts for wait on Windows gh-90872: Handle negative timeouts for wait on Windows Feb 23, 2024
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

How difficult is to write tests?

@eryksun
Copy link
Contributor

eryksun commented Mar 19, 2024

How difficult is to write tests?

Tests can spawn a child process that exits after a few seconds. With this PR, a timeout that's larger than 4294967.295 should raise OverflowError. For example, the current implementation mistakenly wraps 4294968 around to 0.704, leading to TimeoutExpired. With this PR, a negative timeout should be handled as 0. For example, the current implementation wraps -1 to 4294966.296, and the wait will succeed when the child exits, instead of raising TimeoutExpired.

@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes and removed needs backport to 3.11 only security fixes labels May 9, 2024
@serhiy-storchaka serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants