threading.Lock.acquire() not interruptible on Windows

[pitrou]

BPO	29971
Nosy	@tim-one, @pfmoore, @pitrou, @kristjanvalur, @tjguk, @zware, @eryksun, @zooba, @MojoVampire, @maggyero

^{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 = None
created_at = <Date 2017-04-03.14:43:38.613>
labels = ['3.9', 'extension-modules', 'type-feature', 'interpreter-core', '3.10', '3.11', 'library', 'OS-windows']
title = 'threading.Lock.acquire() not interruptible on Windows'
updated_at = <Date 2022-03-12.00:30:51.171>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2022-03-12.00:30:51.171>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'Interpreter Core', 'Library (Lib)', 'Windows']
creation = <Date 2017-04-03.14:43:38.613>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 29971
keywords = []
message_count = 7.0
messages = ['291072', '291094', '291113', '402715', '402719', '402722', '402731']
nosy_count = 10.0
nosy_names = ['tim.peters', 'paul.moore', 'pitrou', 'kristjan.jonsson', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'josh.r', 'maggyero']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue29971'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[pitrou]

On Windows, Lock.acquire() (and other synchronization primitives derived from it, such as queue.Queue) cannot be interrupted with Ctrl-C, which makes it difficult to interrupt a process waiting on such a primitive.

Judging by the code in Python/_thread_nt.h, it should be relatively easy to add such support for the "legacy" semaphore-based implementation (by using WaitForMultipleObjects instead of WaitForSingleObject), but it would be much hairier for the new condition variable-based implementation.

Of course, many other library calls are prone to this limitation (not being interruptible with Ctrl-C on Windows).

See dask/dask#2144 (comment) for original report.

[eryksun]

Alternatively we could use the SleepEx and WaitFor*Ex functions with alertable waits (i.e. using APCs) instead of the SIGINT Event. This avoids having to replace all single-object waits with multiple-object waits, and would even allow calling SleepEx in pysleep.

That said, bpo-29871 proposes to switch to the condition variable and SRW lock implementation, so first it needs to be decided whether to continue to use kernel waits or switch to conditional variables. Or maybe refactor to use condition variables in performance-critical code and otherwise use kernel waits, if that makes sense.

An orthogonal improvement is to have the signal handler call CancelSynchronousIo. This would entail handling ERROR_OPERATION_ABORTED in _winapi Readfile, WriteFile, and WaitNamedPipe by calling PyErr_CheckSignals. Also in _Py_Read and _Py_Write, if errno is EINVAL and the last Windows error is ERROR_OPERATION_ABORTED, it could manually set errno to EINTR.

Winsock waits (e.g. select) will remain a problem in any case. Winsock uses alertable waits, so a queued user-mode APC will be executed while it's waiting. But then it just resumes its original wait instead of failing with WSAEINTR.

[pitrou]

I am not competent enough to pronounce on the technical detail of what you are proposing, but:

Or maybe refactor to use condition variables in performance-critical code and otherwise use kernel waits, if that makes sense.

That can make sense IMHO. Lock and RLock are Python-facing objects, so I'm not sure using high-performance userspace primitives is really important there (after all, people will primarily suffer the evaluation cost of pure Python code so, unless you do something silly such as acquire and release a Python lock in a loop, the acquisition cost doesn't really matter). OTOH, the GIL may be more performance-critical (and needn't be interrupted), so can use userspace CV primitives.

That will however entail a complication of the internal locking API, since we basically need two separate PyThread lock APIs: an "interruptible lock" API and a "fast lock" API.

[vstinner]

Copy of Antoine Pitrou's msg316024 (bpo-21822):

multiprocessing semaphores support Ctrl-C under Windows, so it should be doable for regular locks as well (notice the sigint_event):
https://github.com/python/cpython/blob/master/Modules/_multiprocessing/semaphore.c#L109-L146

[vstinner]

See also bpo-21822: some comments are about this issue. It's a duplicate of bpo-45274 "Race condition in Thread._wait_for_tstate_lock()".