From 6dd381eb62278f75de7ba01626813de84cd248e7 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 21 Nov 2011 21:26:56 +0100 Subject: [PATCH] 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. --- Include/intrcheck.h | 6 +++ Misc/NEWS | 4 ++ Modules/_multiprocessing/multiprocessing.c | 35 -------------- Modules/_multiprocessing/multiprocessing.h | 1 - Modules/_multiprocessing/semaphore.c | 54 +++++++++------------- Modules/_multiprocessing/win32_functions.c | 18 +++++++- Modules/signalmodule.c | 36 +++++++++++++++ Modules/timemodule.c | 30 +----------- 8 files changed, 85 insertions(+), 99 deletions(-) diff --git a/Include/intrcheck.h b/Include/intrcheck.h index 3b67ed0d5a4..f53fee1a1ee 100644 --- a/Include/intrcheck.h +++ b/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 } diff --git a/Misc/NEWS b/Misc/NEWS index faa209d351f..6f7740b49b6 100644 --- a/Misc/NEWS +++ b/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 diff --git a/Modules/_multiprocessing/multiprocessing.c b/Modules/_multiprocessing/multiprocessing.c index 890b96d7dcd..ab2cd72d319 100644 --- a/Modules/_multiprocessing/multiprocessing.c +++ b/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 */ diff --git a/Modules/_multiprocessing/multiprocessing.h b/Modules/_multiprocessing/multiprocessing.h index ac0dfd77b26..e3de9baf1b3 100644 --- a/Modules/_multiprocessing/multiprocessing.h +++ b/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 diff --git a/Modules/_multiprocessing/semaphore.c b/Modules/_multiprocessing/semaphore.c index 6749f237ce8..e48936e94aa 100644 --- a/Modules/_multiprocessing/semaphore.c +++ b/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}; - - /* do the wait */ - Py_BEGIN_ALLOW_THREADS - 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); - } + /* 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 + if (sigint_event != NULL) + ResetEvent(sigint_event); + 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: diff --git a/Modules/_multiprocessing/win32_functions.c b/Modules/_multiprocessing/win32_functions.c index c017b2a6b7b..ddc496d9412 100644 --- a/Modules/_multiprocessing/win32_functions.c +++ b/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); } diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 45a7dfad3bc..c28f7afab03 100644 --- a/Modules/signalmodule.c +++ b/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 diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 85614a6e581..52aade4bda3 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -21,19 +21,6 @@ #include #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);