Index: Python/thread_pthread.h =================================================================== --- Python/thread_pthread.h (revision 82754) +++ Python/thread_pthread.h (working copy) @@ -315,16 +315,17 @@ return (status == -1) ? errno : status; } -int -PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds) +PyLockStatus +PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds, + int intr_flag) { - int success; + PyLockStatus success; sem_t *thelock = (sem_t *)lock; int status, error = 0; struct timespec ts; - dprintf(("PyThread_acquire_lock_timed(%p, %lld) called\n", - lock, microseconds)); + dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) called\n", + lock, microseconds, intr_flag)); if (microseconds > 0) MICROSECONDS_TO_TIMESPEC(microseconds, ts); @@ -335,33 +336,38 @@ status = fix_status(sem_trywait(thelock)); else status = fix_status(sem_wait(thelock)); - } while (status == EINTR); /* Retry if interrupted by a signal */ + /* Retry if interrupted by a signal, unless the caller wants to be + notified. */ + } while (!intr_flag && status == EINTR); - if (microseconds > 0) { - if (status != ETIMEDOUT) - CHECK_STATUS("sem_timedwait"); + /* Don't check the status if we're stopping because of an interrupt. */ + if (!(intr_flag && status == EINTR)) { + if (microseconds > 0) { + if (status != ETIMEDOUT) + CHECK_STATUS("sem_timedwait"); + } + else if (microseconds == 0) { + if (status != EAGAIN) + CHECK_STATUS("sem_trywait"); + } + else { + CHECK_STATUS("sem_wait"); + } } - else if (microseconds == 0) { - if (status != EAGAIN) - CHECK_STATUS("sem_trywait"); + + if (status == 0) { + success = PY_LOCK_ACQUIRED; + } else if (intr_flag && status == EINTR) { + success = PY_LOCK_INTR; + } else { + success = PY_LOCK_FAILURE; } - else { - CHECK_STATUS("sem_wait"); - } - success = (status == 0) ? 1 : 0; - - dprintf(("PyThread_acquire_lock_timed(%p, %lld) -> %d\n", - lock, microseconds, success)); + dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) -> %d\n", + lock, microseconds, intr_flag, success)); return success; } -int -PyThread_acquire_lock(PyThread_type_lock lock, int waitflag) -{ - return PyThread_acquire_lock_timed(lock, waitflag ? -1 : 0); -} - void PyThread_release_lock(PyThread_type_lock lock) { @@ -435,21 +441,25 @@ free((void *)thelock); } -int -PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds) +PyLockStatus +PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds, + int intr_flag) { - int success; + PyLockStatus success; pthread_lock *thelock = (pthread_lock *)lock; int status, error = 0; - dprintf(("PyThread_acquire_lock_timed(%p, %lld) called\n", - lock, microseconds)); + dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) called\n", + lock, microseconds, intr_flag)); status = pthread_mutex_lock( &thelock->mut ); CHECK_STATUS("pthread_mutex_lock[1]"); - success = thelock->locked == 0; - if (!success && microseconds != 0) { + if (thelock->locked == 0) { + success = PY_LOCK_ACQUIRED; + } else if (microseconds == 0) { + success = PY_LOCK_FAILURE; + } else { struct timespec ts; if (microseconds > 0) MICROSECONDS_TO_TIMESPEC(microseconds, ts); @@ -457,7 +467,8 @@ /* mut must be locked by me -- part of the condition * protocol */ - while (thelock->locked) { + success = PY_LOCK_FAILURE; + while (success == PY_LOCK_FAILURE) { if (microseconds > 0) { status = pthread_cond_timedwait( &thelock->lock_released, @@ -472,25 +483,30 @@ &thelock->mut); CHECK_STATUS("pthread_cond_wait"); } + + if (intr_flag && status == 0 && thelock->locked) { + /* We were woken up, but didn't get the lock. We probably received + * a signal. Return PY_LOCK_INTR to allow the caller to handle + * it and retry. */ + success = PY_LOCK_INTR; + break; + } else if (status == 0 && !thelock->locked) { + success = PY_LOCK_ACQUIRED; + } else { + success = PY_LOCK_FAILURE; + } } - success = (status == 0); } - if (success) thelock->locked = 1; + if (success == PY_LOCK_ACQUIRED) thelock->locked = 1; status = pthread_mutex_unlock( &thelock->mut ); CHECK_STATUS("pthread_mutex_unlock[1]"); - if (error) success = 0; - dprintf(("PyThread_acquire_lock_timed(%p, %lld) -> %d\n", - lock, microseconds, success)); + if (error) success = PY_LOCK_FAILURE; + dprintf(("PyThread_acquire_lock_timed(%p, %lld, %d) -> %d\n", + lock, microseconds, intr_flag, success)); return success; } -int -PyThread_acquire_lock(PyThread_type_lock lock, int waitflag) -{ - return PyThread_acquire_lock_timed(lock, waitflag ? -1 : 0); -} - void PyThread_release_lock(PyThread_type_lock lock) { @@ -514,6 +530,12 @@ #endif /* USE_SEMAPHORES */ +int +PyThread_acquire_lock(PyThread_type_lock lock, int waitflag) +{ + return PyThread_acquire_lock_timed(lock, waitflag ? -1 : 0, /*intr_flag=*/0); +} + /* set the thread stack size. * Return 0 if size is valid, -1 if size is invalid, * -2 if setting stack size is not supported. Index: Python/thread_nt.h =================================================================== --- Python/thread_nt.h (revision 82754) +++ Python/thread_nt.h (working copy) @@ -238,10 +238,13 @@ * and 0 if the lock was not acquired. This means a 0 is returned * if the lock has already been acquired by this thread! */ -int -PyThread_acquire_lock_timed(PyThread_type_lock aLock, PY_TIMEOUT_T microseconds) +PyLockStatus +PyThread_acquire_lock_timed(PyThread_type_lock aLock, + PY_TIMEOUT_T microseconds, int intr_flag) { - int success ; + /* Fow now, intr_flag does nothing on Windows, and lock acquires are + * uninterruptible. */ + PyLockStatus success; PY_TIMEOUT_T milliseconds; if (microseconds >= 0) { @@ -258,7 +261,13 @@ dprintf(("%ld: PyThread_acquire_lock_timed(%p, %lld) called\n", PyThread_get_thread_ident(), aLock, microseconds)); - success = aLock && EnterNonRecursiveMutex((PNRMUTEX) aLock, (DWORD) milliseconds) == WAIT_OBJECT_0 ; + if (aLock && EnterNonRecursiveMutex((PNRMUTEX)aLock, + (DWORD)milliseconds) == WAIT_OBJECT_0) { + success = PY_LOCK_ACQUIRED; + } + else { + success = PY_LOCK_FAILURE; + } dprintf(("%ld: PyThread_acquire_lock(%p, %lld) -> %d\n", PyThread_get_thread_ident(), aLock, microseconds, success)); @@ -268,7 +277,7 @@ int PyThread_acquire_lock(PyThread_type_lock aLock, int waitflag) { - return PyThread_acquire_lock_timed(aLock, waitflag ? -1 : 0); + return PyThread_acquire_lock_timed(aLock, waitflag ? -1 : 0, 0); } void Index: Include/pythread.h =================================================================== --- Include/pythread.h (revision 82754) +++ Include/pythread.h (working copy) @@ -9,6 +9,14 @@ extern "C" { #endif +/* Return status codes for Python lock acquisition. Chosen for maximum + * backwards compatibility, ie failure -> 0, success -> 1. */ +typedef enum PyLockStatus { + PY_LOCK_FAILURE = 0, + PY_LOCK_ACQUIRED = 1, + PY_LOCK_INTR +} PyLockStatus; + PyAPI_FUNC(void) PyThread_init_thread(void); PyAPI_FUNC(long) PyThread_start_new_thread(void (*)(void *), void *); PyAPI_FUNC(void) PyThread_exit_thread(void); @@ -49,11 +57,18 @@ even when the lock can't be acquired. If microseconds > 0, the call waits up to the specified duration. If microseconds < 0, the call waits until success (or abnormal failure) - + microseconds must be less than PY_TIMEOUT_MAX. Behaviour otherwise is - undefined. */ -PyAPI_FUNC(int) PyThread_acquire_lock_timed(PyThread_type_lock, - PY_TIMEOUT_T microseconds); + undefined. + + If intr_flag is true and the acquire is interrupted by a signal, then the + call will return PY_LOCK_INTR. The caller may reattempt to acquire the + lock. +*/ +PyAPI_FUNC(PyLockStatus) PyThread_acquire_lock_timed(PyThread_type_lock, + PY_TIMEOUT_T microseconds, + int intr_flag); + PyAPI_FUNC(void) PyThread_release_lock(PyThread_type_lock); PyAPI_FUNC(size_t) PyThread_get_stacksize(void); Index: Doc/whatsnew/3.2.rst =================================================================== --- Doc/whatsnew/3.2.rst (revision 82754) +++ Doc/whatsnew/3.2.rst (working copy) @@ -143,7 +143,13 @@ Similarly, :meth:`threading.Semaphore.acquire` also gains a *timeout* argument. (Contributed by Torsten Landschoff; :issue:`850728`.) +* Regular and recursive lock acquires can now be interrupted by signals on + platforms using pthreads. This means that Python programs that deadlock while + acquiring locks can be successfully killed by repeatedly sending SIGINT to the + process (ie, by pressing Ctl+C in most shells). + (Contributed by Reid Kleckner; :issue:`8844`.) + Optimizations ============= Index: Doc/library/threading.rst =================================================================== --- Doc/library/threading.rst (revision 82754) +++ Doc/library/threading.rst (working copy) @@ -386,6 +386,9 @@ .. versionchanged:: 3.2 The *timeout* parameter is new. + .. versionchanged:: 3.2 + Lock acquires can now be interrupted by signals on POSIX. + .. method:: Lock.release() Release a lock. Index: Lib/test/test_threadsignals.py =================================================================== --- Lib/test/test_threadsignals.py (revision 82754) +++ Lib/test/test_threadsignals.py (working copy) @@ -6,6 +6,7 @@ import sys from test.support import run_unittest, import_module thread = import_module('_thread') +import time if sys.platform[:3] in ('win', 'os2') or sys.platform=='riscos': raise unittest.SkipTest("Can't test signal on %s" % sys.platform) @@ -66,7 +67,42 @@ def spawnSignallingThread(self): thread.start_new_thread(send_signals, ()) + def alarm_interrupt(self, sig, frame): + raise KeyboardInterrupt + def test_lock_acquire_interruption(self): + # Mimic receiving a SIGINT (KeyboardInterrupt) with SIGALRM while stuck + # in a deadlock. + oldalrm = signal.signal(signal.SIGALRM, self.alarm_interrupt) + try: + lock = thread.allocate_lock() + lock.acquire() + signal.alarm(1) + self.assertRaises(KeyboardInterrupt, lock.acquire) + finally: + signal.signal(signal.SIGALRM, oldalrm) + + def test_rlock_acquire_interruption(self): + # Mimic receiving a SIGINT (KeyboardInterrupt) with SIGALRM while stuck + # in a deadlock. + oldalrm = signal.signal(signal.SIGALRM, self.alarm_interrupt) + try: + rlock = thread.RLock() + # For reentrant locks, the initial acquisition must be in another + # thread. + def other_thread(): + rlock.acquire() + thread.start_new_thread(other_thread, ()) + # Wait we can't acquire it without blocking... + while rlock.acquire(blocking=False): + rlock.release() + time.sleep(0.01) + signal.alarm(1) + self.assertRaises(KeyboardInterrupt, rlock.acquire) + finally: + signal.signal(signal.SIGALRM, oldalrm) + + def test_main(): global signal_blackboard Index: Modules/_threadmodule.c =================================================================== --- Modules/_threadmodule.c (revision 82754) +++ Modules/_threadmodule.c (working copy) @@ -46,7 +46,7 @@ int blocking = 1; double timeout = -1; PY_TIMEOUT_T microseconds; - int r; + PyLockStatus r; if (!PyArg_ParseTupleAndKeywords(args, kwds, "|id:acquire", kwlist, &blocking, &timeout)) @@ -76,10 +76,18 @@ microseconds = (PY_TIMEOUT_T) timeout; } - Py_BEGIN_ALLOW_THREADS - r = PyThread_acquire_lock_timed(self->lock_lock, microseconds); - Py_END_ALLOW_THREADS + do { + Py_BEGIN_ALLOW_THREADS + r = PyThread_acquire_lock_timed(self->lock_lock, microseconds, 1); + Py_END_ALLOW_THREADS + /* Run signal handlers if we were interrupted. Propagate exceptions from + * signal handlers, such as KeyboardInterrupt. */ + if (r == PY_LOCK_INTR && Py_MakePendingCalls() < 0) { + return NULL; + } + } while (r == PY_LOCK_INTR); /* Retry if we were interrupted. */ + return PyBool_FromLong(r); } @@ -92,7 +100,7 @@ the lock, and return None once the lock is acquired.\n\ With an argument, this will only block if the argument is true,\n\ and the return value reflects whether the lock is acquired.\n\ -The blocking operation is not interruptible."); +The blocking operation is interruptible."); static PyObject * lock_PyThread_release_lock(lockobject *self) @@ -217,7 +225,7 @@ double timeout = -1; PY_TIMEOUT_T microseconds; long tid; - int r = 1; + PyLockStatus r = PY_LOCK_ACQUIRED; if (!PyArg_ParseTupleAndKeywords(args, kwds, "|id:acquire", kwlist, &blocking, &timeout)) @@ -265,10 +273,16 @@ Py_RETURN_FALSE; } Py_BEGIN_ALLOW_THREADS - r = PyThread_acquire_lock_timed(self->rlock_lock, microseconds); + r = PyThread_acquire_lock_timed(self->rlock_lock, microseconds, 1); Py_END_ALLOW_THREADS + + /* Run signal handlers if we were interrupted. Propagate exceptions from + * signal handlers, such as KeyboardInterrupt. */ + if (r == PY_LOCK_INTR && Py_MakePendingCalls() < 0) { + return NULL; + } } - if (r) { + if (r == PY_LOCK_ACQUIRED) { assert(self->rlock_count == 0); self->rlock_owner = tid; self->rlock_count = 1; @@ -286,7 +300,7 @@ immediately. If `blocking` is True and another thread holds\n\ the lock, the method will wait for the lock to be released,\n\ take it and then return True.\n\ -(note: the blocking operation is not interruptible.)\n\ +(note: the blocking operation is interruptible.)\n\ \n\ In all other cases, the method will return True immediately.\n\ Precisely, if the current thread already holds the lock, its\n\