Use _winapi.WaitForMultipleObjects in Popen.wait()

[eryksun]

BPO	28168
Nosy	@pfmoore, @vstinner, @tjguk, @zware, @eryksun, @zooba
Files	issue_28168_03.patch issue_28168_04.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-03-20.02:32:58.316>
created_at = <Date 2016-09-15.10:42:49.806>
labels = ['3.7', 'type-feature', 'library', 'OS-windows']
title = 'Use _winapi.WaitForMultipleObjects in Popen.wait()'
updated_at = <Date 2021-03-20.02:32:58.316>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2021-03-20.02:32:58.316>
actor = 'eryksun'
assignee = 'none'
closed = True
closed_date = <Date 2021-03-20.02:32:58.316>
closer = 'eryksun'
components = ['Library (Lib)', 'Windows']
creation = <Date 2016-09-15.10:42:49.806>
creator = 'eryksun'
dependencies = []
files = ['44698', '44700']
hgrepos = []
issue_num = 28168
keywords = ['patch']
message_count = 6.0
messages = ['276544', '276546', '276760', '276763', '276764', '389140']
nosy_count = 6.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28168'
versions = ['Python 3.6', 'Python 3.7']

[eryksun]

On Unix, waiting on an instance of subprocess.Popen in the main thread is interruptible by Ctrl+C. On Windows, it currently calls _winapi.WaitForSingleObject, which isn't interruptible. It should instead call _winapi.WaitForMultipleObjects, which automatically adds the SIGINT event object from _PyOS_SigintEvent() when called from the main thread.

[eryksun]

This patch makes the trivial change to the Popen.wait() method and also modifies _winapi.WaitForMultipleObjects to restart the wait if SIGINT is ignored or the handler didn't raise an exception.

[eryksun]

Hopefully this is my last change until someone reviews this. I decided to make it an error to pass an empty sequence, which previously would implicitly wait on the SIGINT event. This way simplifies the code and prevents someone from accidentally waiting forever on SIGINT when it's ignored or doesn't raise an exception. There should always be at least 1 other handle in the wait list.

[vstinner]

Oh nice, you implemented the PEP-475 for _winapi.WaitForMultipleObjects()! I missed this function when implementing this PEP :-)

[vstinner]

I decided to make it an error to pass an empty sequence

It makes sense since the original WaitForMultipleObjects() also requires at least one object:

https://msdn.microsoft.com/en-us/library/windows/desktop/ms687025(v=vs.85).aspx

nCount [in]

The number of object handles in the array pointed to by lpHandles. The maximum number of object handles is MAXIMUM_WAIT_OBJECTS. This parameter cannot be zero.

[eryksun]

I'm no longer interested in solving the SIGINT event problem locally at the wait site. A common implementation of _Py_Sleep, _Py_WaitForSingleObject, and _Py_WaitForMultipleObjects is needed. _winapi.WaitForSingleObject would call _Py_WaitForSingleObject and automatically support the SIGINT event on the main thread.