Issue #12328: Under Windows, refactor handling of Ctrl-C events and

make _multiprocessing.win32.WaitForMultipleObjects interruptible when
the wait_flag parameter is false.  Patch by sbt.

Author: pitrou

Include/intrcheck.h

@@ -8,6 +8,12 @@ extern "C" {
 PyAPI_FUNC(int) PyOS_InterruptOccurred(void);
 PyAPI_FUNC(void) PyOS_InitInterrupts(void);
 PyAPI_FUNC(void) PyOS_AfterFork(void);
+PyAPI_FUNC(int) _PyOS_IsMainThread(void);
+
+#ifdef MS_WINDOWS
+/* windows.h is not included by Python.h so use void* instead of HANDLE */
+PyAPI_FUNC(void*) _PyOS_SigintEvent(void);
+#endif
 
 #ifdef __cplusplus
 }

Misc/NEWS

@@ -387,6 +387,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #12328: Under Windows, refactor handling of Ctrl-C events and
+  make _multiprocessing.win32.WaitForMultipleObjects interruptible when
+  the wait_flag parameter is false.  Patch by sbt.
+
 - Issue #13322: Fix BufferedWriter.write() to ensure that BlockingIOError is
   raised when the wrapped raw file is non-blocking and the write would block.
   Previous code assumed that the raw write() would raise BlockingIOError, but

Modules/_multiprocessing/multiprocessing.c

@@ -53,30 +53,6 @@ mp_SetError(PyObject *Type, int num)
 }
 
 
-/*
- * Windows only
- */
-
-#ifdef MS_WINDOWS
-
-/* On Windows we set an event to signal Ctrl-C; compare with timemodule.c */
-
-HANDLE sigint_event = NULL;
-
-static BOOL WINAPI
-ProcessingCtrlHandler(DWORD dwCtrlType)
-{
-    SetEvent(sigint_event);
-    return FALSE;
-}
-
-#endif /* MS_WINDOWS */
-
-
-/*
- * All platforms
- */
-
 static PyObject*
 multiprocessing_address_of_buffer(PyObject *self, PyObject *obj)
 {
@@ -165,17 +141,6 @@ PyInit__multiprocessing(void)
     if (!temp)
         return NULL;
     PyModule_AddObject(module, "win32", temp);
-
-    /* Initialize the event handle used to signal Ctrl-C */
-    sigint_event = CreateEvent(NULL, TRUE, FALSE, NULL);
-    if (!sigint_event) {
-        PyErr_SetFromWindowsErr(0);
-        return NULL;
-    }
-    if (!SetConsoleCtrlHandler(ProcessingCtrlHandler, TRUE)) {
-        PyErr_SetFromWindowsErr(0);
-        return NULL;
-    }
 #endif
 
     /* Add configuration macros */

Modules/_multiprocessing/multiprocessing.h

@@ -100,7 +100,6 @@ PyObject *mp_SetError(PyObject *Type, int num);
 extern PyObject *BufferTooShort;
 extern PyTypeObject SemLockType;
 extern PyTypeObject PipeConnectionType;
-extern HANDLE sigint_event;
 
 /*
  * Miscellaneous

Modules/_multiprocessing/semaphore.c

@@ -62,7 +62,8 @@ semlock_acquire(SemLockObject *self, PyObject *args, PyObject *kwds)
     int blocking = 1;
     double timeout;
     PyObject *timeout_obj = Py_None;
-    DWORD res, full_msecs, msecs, start, ticks;
+    DWORD res, full_msecs, nhandles;
+    HANDLE handles[2], sigint_event;
 
     static char *kwlist[] = {"block", "timeout", NULL};
 
@@ -96,53 +97,40 @@ semlock_acquire(SemLockObject *self, PyObject *args, PyObject *kwds)
         Py_RETURN_TRUE;
     }
 
-    /* check whether we can acquire without blocking */
+    /* check whether we can acquire without releasing the GIL and blocking */
     if (WaitForSingleObject(self->handle, 0) == WAIT_OBJECT_0) {
         self->last_tid = GetCurrentThreadId();
         ++self->count;
         Py_RETURN_TRUE;
     }
 
-    msecs = full_msecs;
-    start = GetTickCount();
-
-    for ( ; ; ) {
-        HANDLE handles[2] = {self->handle, sigint_event};
+    /* prepare list of handles */
+    nhandles = 0;
+    handles[nhandles++] = self->handle;
+    if (_PyOS_IsMainThread()) {
+        sigint_event = _PyOS_SigintEvent();
+        assert(sigint_event != NULL);
+        handles[nhandles++] = sigint_event;
+    }
 
-        /* do the wait */
-        Py_BEGIN_ALLOW_THREADS
+    /* do the wait */
+    Py_BEGIN_ALLOW_THREADS
+    if (sigint_event != NULL)
         ResetEvent(sigint_event);
-        res = WaitForMultipleObjects(2, handles, FALSE, msecs);
-        Py_END_ALLOW_THREADS
-
-        /* handle result */
-        if (res != WAIT_OBJECT_0 + 1)
-            break;
-
-        /* got SIGINT so give signal handler a chance to run */
-        Sleep(1);
-
-        /* if this is main thread let KeyboardInterrupt be raised */
-        if (PyErr_CheckSignals())
-            return NULL;
-
-        /* recalculate timeout */
-        if (msecs != INFINITE) {
-            ticks = GetTickCount();
-            if ((DWORD)(ticks - start) >= full_msecs)
-                Py_RETURN_FALSE;
-            msecs = full_msecs - (ticks - start);
-        }
-    }
+    res = WaitForMultipleObjects(nhandles, handles, FALSE, full_msecs);
+    Py_END_ALLOW_THREADS
 
     /* handle result */
     switch (res) {
     case WAIT_TIMEOUT:
         Py_RETURN_FALSE;
-    case WAIT_OBJECT_0:
+    case WAIT_OBJECT_0 + 0:
         self->last_tid = GetCurrentThreadId();
         ++self->count;
         Py_RETURN_TRUE;
+    case WAIT_OBJECT_0 + 1:
+        errno = EINTR;
+        return PyErr_SetFromErrno(PyExc_IOError);
     case WAIT_FAILED:
         return PyErr_SetFromWindowsErr(0);
     default:

Modules/_multiprocessing/win32_functions.c

@@ -679,6 +679,7 @@ win32_WaitForMultipleObjects(PyObject* self, PyObject* args)
     DWORD result;
     PyObject *handle_seq;
     HANDLE handles[MAXIMUM_WAIT_OBJECTS];
+    HANDLE sigint_event = NULL;
     Py_ssize_t nhandles, i;
     int wait_flag;
     int milliseconds = INFINITE;
@@ -696,10 +697,10 @@ win32_WaitForMultipleObjects(PyObject* self, PyObject* args)
     nhandles = PySequence_Length(handle_seq);
     if (nhandles == -1)
         return NULL;
-    if (nhandles < 0 || nhandles >= MAXIMUM_WAIT_OBJECTS) {
+    if (nhandles < 0 || nhandles >= MAXIMUM_WAIT_OBJECTS - 1) {
         PyErr_Format(PyExc_ValueError,
                      "need at most %zd handles, got a sequence of length %zd",
-                     MAXIMUM_WAIT_OBJECTS, nhandles);
+                     MAXIMUM_WAIT_OBJECTS - 1, nhandles);
         return NULL;
     }
     for (i = 0; i < nhandles; i++) {
@@ -711,14 +712,27 @@ win32_WaitForMultipleObjects(PyObject* self, PyObject* args)
             return NULL;
         handles[i] = h;
     }
+    /* If this is the main thread then make the wait interruptible
+       by Ctrl-C unless we are waiting for *all* handles */
+    if (!wait_flag && _PyOS_IsMainThread()) {
+        sigint_event = _PyOS_SigintEvent();
+        assert(sigint_event != NULL);
+        handles[nhandles++] = sigint_event;
+    }
 
     Py_BEGIN_ALLOW_THREADS
+    if (sigint_event != NULL)
+        ResetEvent(sigint_event);
     result = WaitForMultipleObjects((DWORD) nhandles, handles,
                                     (BOOL) wait_flag, (DWORD) milliseconds);
     Py_END_ALLOW_THREADS
 
     if (result == WAIT_FAILED)
         return PyErr_SetExcFromWindowsErr(PyExc_IOError, 0);
+    else if (sigint_event != NULL && result == WAIT_OBJECT_0 + nhandles - 1) {
+        errno = EINTR;
+        return PyErr_SetFromErrno(PyExc_IOError);
+    }
 
     return PyLong_FromLong((int) result);
 }

Modules/signalmodule.c

@@ -109,6 +109,10 @@ static PyObject *IntHandler;
 
 static PyOS_sighandler_t old_siginthandler = SIG_DFL;
 
+#ifdef MS_WINDOWS
+static HANDLE sigint_event = NULL;
+#endif
+
 #ifdef HAVE_GETITIMER
 static PyObject *ItimerError;
 
@@ -229,6 +233,11 @@ signal_handler(int sig_num)
     /* Issue #10311: asynchronously executing signal handlers should not
        mutate errno under the feet of unsuspecting C code. */
     errno = save_errno;
+
+#ifdef MS_WINDOWS
+    if (sig_num == SIGINT)
+        SetEvent(sigint_event);
+#endif
 }
 
 
@@ -1253,6 +1262,11 @@ PyInit_signal(void)
     Py_DECREF(x);
 #endif
 
+#ifdef MS_WINDOWS
+    /* Create manual-reset event, initially unset */
+    sigint_event = CreateEvent(NULL, TRUE, FALSE, FALSE);
+#endif
+
     if (PyErr_Occurred()) {
         Py_DECREF(m);
         m = NULL;
@@ -1397,3 +1411,25 @@ PyOS_AfterFork(void)
     PyThread_ReInitTLS();
 #endif
 }
+
+int
+_PyOS_IsMainThread(void)
+{
+#ifdef WITH_THREAD
+    return PyThread_get_thread_ident() == main_thread;
+#else
+    return 1;
+#endif
+}
+
+#ifdef MS_WINDOWS
+void *_PyOS_SigintEvent(void)
+{
+    /* Returns a manual-reset event which gets tripped whenever
+       SIGINT is received.
+
+       Python.h does not include windows.h so we do cannot use HANDLE
+       as the return type of this function.  We use void* instead. */
+    return sigint_event;
+}
+#endif

Modules/timemodule.c

@@ -21,19 +21,6 @@
 #include <windows.h>
 #include "pythread.h"
 
-/* helper to allow us to interrupt sleep() on Windows*/
-static HANDLE hInterruptEvent = NULL;
-static BOOL WINAPI PyCtrlHandler(DWORD dwCtrlType)
-{
-    SetEvent(hInterruptEvent);
-    /* allow other default handlers to be called.
-       Default Python handler will setup the
-       KeyboardInterrupt exception.
-    */
-    return FALSE;
-}
-static long main_thread;
-
 #if defined(__BORLANDC__)
 /* These overrides not needed for Win32 */
 #define timezone _timezone
@@ -955,15 +942,6 @@ PyInit_time(void)
     /* Set, or reset, module variables like time.timezone */
     PyInit_timezone(m);
 
-#ifdef MS_WINDOWS
-    /* Helper to allow interrupts for Windows.
-       If Ctrl+C event delivered while not sleeping
-       it will be ignored.
-    */
-    main_thread = PyThread_get_thread_ident();
-    hInterruptEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
-    SetConsoleCtrlHandler( PyCtrlHandler, TRUE);
-#endif /* MS_WINDOWS */
     if (!initialized) {
         PyStructSequence_InitType(&StructTimeType,
                                   &struct_time_type_desc);
@@ -1036,18 +1014,14 @@ floatsleep(double secs)
          * by Guido, only the main thread can be interrupted.
          */
         ul_millis = (unsigned long)millisecs;
-        if (ul_millis == 0 ||
-            main_thread != PyThread_get_thread_ident())
+        if (ul_millis == 0 || !_PyOS_IsMainThread())
             Sleep(ul_millis);
         else {
             DWORD rc;
+            HANDLE hInterruptEvent = _PyOS_SigintEvent();
             ResetEvent(hInterruptEvent);
             rc = WaitForSingleObject(hInterruptEvent, ul_millis);
             if (rc == WAIT_OBJECT_0) {
-                /* Yield to make sure real Python signal
-                 * handler called.
-                 */
-                Sleep(1);
                 Py_BLOCK_THREADS
                 errno = EINTR;
                 PyErr_SetFromErrno(PyExc_IOError);