[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Gregory P. Smith]

Gregory P. Smith added the comment:

As a data point confirming that: we tried backporting the much nicer 
non-polling condition implementation 
(https://github.com/python/cpython/commit/15801a1d52c25fa2a19d649ea2671080f138fca1.patch)
 to our Python 2.7 interpreter at work but ran into actual code that failed as 
a result of the interruption behavior changing.

While it is a nice goal, it is a non-trivial change.  Not something we could 
ever do within a stable release (2.7).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[STINNER Victor]

STINNER Victor added the comment:

> Is it possible to backport this patch to 2.7?

No.

Python 2.7 supports many implementations of threads:

$ ls Python/thread_*h -1
Python/thread_atheos.h
Python/thread_beos.h
Python/thread_cthread.h
Python/thread_foobar.h
Python/thread_lwp.h
Python/thread_nt.h
Python/thread_os2.h
Python/thread_pth.h
Python/thread_pthread.h
Python/thread_sgi.h
Python/thread_solaris.h
Python/thread_wince.h

Supporting signals on all implementations would be a huge work.

Handling correctly signals is also a complex task. For example, the PEP 475 
changed how Python 3.5 handles signals (retry interrupted syscall), the change 
is larger than just the threading module. This PEP has an impact on the whole 
CPython code base (C and Python).

In Python, unit tests are important. The change b750c45a772c added many unit 
tests, I expect that it will be tricky to make them reliable on all platforms 
supported by Python 2.7. I dislike the idea of backporting the feature just for 
a few platforms.

I mean that the change b750c45a772c alone is not enough, this change depends on 
many earlier changes used to cleanup/prepare the code to support this change. 
It is also followed by many changes to enhance how Python handles signals. It 
took multiple years to fix all subtle issues, race conditions, etc.

There is also a significant risk of breaking applications relying on the 
current behaviour of Python 2.7 (locks are not interrupted by signals).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Sergei Lebedev]

Sergei Lebedev added the comment:

Is it possible to backport this patch to 2.7?

--
nosy: +superbobry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Yuri Bochkarev]

Changes by Yuri Bochkarev baltazar...@gmail.com:


--
nosy: +Yuri.Bochkarev

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[STINNER Victor]

STINNER Victor victor.stin...@haypocalc.com added the comment:

The test fails on FreeBSD 6.2 (x86 FreeBSD py3k buildbot) since r87292 = 
#10720. I reopen the issue.

--
nosy: +haypo
resolution: fixed - 
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

Well, since #10720 exists, while do you reopen this one?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
assignee: pitrou - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
dependencies:  -Make gettimeofday available in time module

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

Committed in r87292. Thank you for doing this!

--
resolution: accepted - fixed
stage: commit review - committed/rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 Added a patch that adds support for recomputing the timeout, plus a test for 
 it.
 
 Can this still make it into 3.2, or is it too disruptive at this point
 in the release process?

No problem at this point, we're not yet in beta phase.
I haven't looked at the patch itself, but thank you.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Added a patch that adds support for recomputing the timeout, plus a test for it.

Can this still make it into 3.2, or is it too disruptive at this point in the 
release process?

--
Added file: http://bugs.python.org/file18536/lock-interrupt-v4.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Waiting until the portability hacks for gettimeofday make it into core Python.

--
dependencies: +Make gettimeofday available in time module

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

Actually, there is a problem in Lock.acquire and RLock.acquire. If a signal 
occurs and signal handling returns successfully, acquiring the lock will be 
retried without decrementing the timeout first. Therefore, we may end up 
waiting longer than the user wanted.

I'm not sure how to tackle that: either we accept that an incoming signal will 
make the wait longer, or we fix it by properly decrementing the timeout (which 
will complicate things a bit, especially for cross-platform time querying - but 
see issue9079 which might help us).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Gregory P. Smith]

Gregory P. Smith g...@krypto.org added the comment:

Accepting that the timeout is not perfect in the face of signals as a caveat is 
fine (document the possibility) and can be improved later.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Alternatively, do you think it would be better to ignore interrupts when a 
timeout is passed?  If a timeout is passed, the lock acquire will eventually 
fail in a deadlock situation, and the signal will be handled in the eval loop.

However, if the timeout is sufficiently long, this is still a problem.

I'd prefer to do that or use gettimeofday from _time than leave this as is.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

The latest patch looks good to me.

--
assignee:  - pitrou
resolution:  - accepted
stage:  - commit review

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Here's a patch that makes Python-level lock acquisitions interruptible for 
py3k.  There are many users of the C-level lock API, most of whom are not set 
up to deal with lock acquisition failure.  I decided to make a new API function 
and leave the others alone.

If possible, I think this should go out with 3.2.

In that case, I was wondering if I should merge PyThread_acquire_lock_timed 
with my new PyThread_acquire_lock_timed_intr, since PyThread_acquire_lock_timed 
wasn't available in 3.1.  Although it did go out in 2.7, we don't promise C API 
compatibility with the 2.x series, so I don't think it matters.

I've tested this patch on Mac OS X and Linux.  The whole test suite passes on 
both, along with the test that I added to test_threadsignals.py.

I added a noop compatibility wrapper to thread_nt.h, but I haven't tested it or 
built it.  When I get around to testing/fixing the subprocess patch on Windows, 
I'll make sure this works and the test is skipped.

--
keywords: +patch
Added file: http://bugs.python.org/file17929/lock-interrupt.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

Oh, nice!

 I was wondering if I should merge PyThread_acquire_lock_timed with my
 new PyThread_acquire_lock_timed_intr, since PyThread_acquire_lock_timed 
 wasn't available in 3.1.

Yes, I think you should.

I haven't tried the patch, but it seems you got the logic right. There's a 
problem sometimes that you're using 2 spaces for indent rather than 4. Also, 
you forgot to update the RLock implementation in _threadmodule.c (and perhaps 
add another test for it).

(there are other modules which use the PyThread_acquire_lock API, but most of 
the time the locks are held for a short time (and shouldn't deadlock), which 
makes converting them less of a priority)

--
versions:  -Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Here is a new version of a patch that updates recursive locks to have the same 
behavior.  The pure Python RLock implementaiton should be interruptible by 
virtue of the base lock acquire primitive being interruptible.

I've also updated the relevant documentation I could find.  I've surely missed 
some, though.

I also got rid of the _intr version of lock acquisition and simply added a new 
parameter to the _timed variant.  The only two callers of it are the ones I 
updated in the _thread module.

--
Added file: http://bugs.python.org/file17931/lock-interrupt.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

The RLock version gets different semantics in your patch: it doesn't retry 
acquiring when a signal is caught. Is there a reason for that?

By the way, your patch works fine under Windows.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Oops, copy/paste oversight.  =/  I wrote a test to verify that it handles 
signals, and then retries the lock acquire.

--
Added file: http://bugs.python.org/file17935/lock-interrupt.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

Also, thanks for the quick reviews!

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 I'm imagining (for POSIX platforms) adding some kind of check for
 signals when the system call returns EINTR.  If the signal handler
 raises an exception, like an interrupt should raise a
 KeyboardInterrupt, we can just give a different return code and
 propagate the exception.

Yes, this is what I'm proposing too.

 It also seems like this behavior can be extended gradually to
 different platforms, since I don't have the resources to change and
 test every threading implementation.

There is only one active POSIX threading implementation in 3.2, in
Python/thread_pthread.h.
(and the only non-POSIX one is for Windows)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Mike Hobbs]

New submission from Mike Hobbs mho...@alvenda.com:

Condition.wait() without a timeout will never raise a KeyboardInterrupt:

cond = threading.Condition()
cond.acquire()
cond.wait()

*** Pressing Ctrl-C now does nothing ***



If you pass a timeout to Condition.wait(), however, it does behave as expected:

cond.wait()

^CTraceback (most recent call last):
  File /usr/lib/python3.1/threading.py, line 242, in wait
_sleep(delay)
KeyboardInterrupt



This may affect other problems reported with multiprocessing pools. Most 
notably:
http://bugs.python.org/issue8296
http://stackoverflow.com/questions/1408356

--
components: Library (Lib)
messages: 106678
nosy: hobb0001
priority: normal
severity: normal
status: open
title: Condition.wait() doesn't raise KeyboardInterrupt
type: behavior
versions: Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

In 3.2, even using a timeout doesn't make the call interruptible.
The solution would be to fix the internal locking APIs so that they handle 
incoming signals properly, and are able to return an error status.

--
nosy: +gregory.p.smith, jyasskin, pitrou, rnk
versions: +Python 3.2

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

3.2 is interesting in that it introduces a new internal API: 
PyThread_acquire_lock_timed(). We can therefore change that API again before 
release without risking any compatibility breakage. Reid, would you want to 
work on this?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8844] Condition.wait() doesn't raise KeyboardInterrupt

[Reid Kleckner]

Reid Kleckner r...@mit.edu added the comment:

I'd like to fix it, but I don't know if I'll be able to in time.  It was 
something that bugged me while running the threading tests while working on 
Unladen.

I'm imagining (for POSIX platforms) adding some kind of check for signals when 
the system call returns EINTR.  If the signal handler raises an exception, like 
an interrupt should raise a KeyboardInterrupt, we can just give a different 
return code and propagate the exception.

It also seems like this behavior can be extended gradually to different 
platforms, since I don't have the resources to change and test every threading 
implementation.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8844
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com