[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Andrew Svetlov]

Andrew Svetlov  added the comment:


New changeset f08a191882f75bb79d42a49039892105b2212fb9 by Andrew Svetlov in 
branch 'main':
bpo-39622: Interrupt the main asyncio task on Ctrl+C (GH-32105)
https://github.com/python/cpython/commit/f08a191882f75bb79d42a49039892105b2212fb9


--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Andrew Svetlov]

Change by Andrew Svetlov :


--
pull_requests: +30184
pull_request: https://github.com/python/cpython/pull/32105

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Andrew Svetlov]

Change by Andrew Svetlov :


--
assignee:  -> asvetlov

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

Hey there, it's been more than a week since I pushed the last changes, can 
anyone review them?

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Change by Maor Kleinberger :


--
versions: +Python 3.8, Python 3.9

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

I have pushed an update to my PR. While implementing the new solution I made 
the following list:

Examples for code segments where KeyboardInterrupt must not be raised:
 - Between popping and running a handle from the ready queue (a handle is lost)
 - After running a handle and before a subsequent call_soon finished running 
(the handle that should have been added by call_soon is lost)
 - After a handle is popped from the _scheduled heap and added to the _ready 
queue (a handle is lost)

Code segments where KeyboardInterrupt must be raised:
 - During the select call (can be achieved using signal.default_int_handler or 
signal.set_wakeup_fd)
 - During a running callback (can be achieved using signal.default_int_handler)

I think that while the loop is running, the signal handler should not raise a 
KeyboardInterrupt by default, as there are at least 3 unsafe code segments, and 
more might be added in the future. Currently, the only two segments that have 
to be directly interrupted by a SIGINT are the ones listed above, and I think 
this behavior should be allowed explicitly.
This is what I did in the new revision of my PR.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

While looking into this proposal, I realized that this will not wake up the 
loop from the select call. I think the safest solution would be to use the 
wakeup fd mechanism.

An additional easy, but less secure solution would be to define an internal 
'signal safe' context manager that will be used only in the critical parts.

What do you think?

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

Damn, good catch. How about the following idea - register a normal signal 
handler (using signal.signal) that does something like:

def sigint_handler():
get_running_loop().interrupt()

# in class BaseEventLoop
def interrupt(self):
# Might be a generally useful thread-safe way to interrupt a loop
if self._is_inside_callback():
_thread.interrupt_main() # All this behavior is only relevant to the 
main thread anyway
else:
self._interrupted = True

And inside BaseEventLoop._run_once() add the following check:

# in class BaseEventLoop
def _check_interrupted(self):
# This will be called by BaseEventLoop._run_once() before calling select,
# and before popping any handle from the ready queue
if self._interrupted:
raise KeyboardInterrupt

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Zhibin Dong]

Zhibin Dong  added the comment:

Yes, I think you are right. In the second solution, the callback of SIGINT will 
be append to the end of one task loop. If that code is defined in a task by 
some user, KeyboardInterrupt will not be raised.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Andrew Svetlov]

Andrew Svetlov  added the comment:

The second solution doesn't help with breaking infinite loops like 
while True:
pass

We need a more robust idea.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

After opening the PR, I see that add_signal_handler is not supported on 
non-unix event loop. After looking at the code, it seems there is no problem to 
implement it for other platforms, as it relies on signal.set_wakeup_fd() which 
seems to support all platforms. Should I open an enhancement issue for 
implementing add_signal_handler for all platforms?

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

After digging into asyncio, I stumbled upon this particularly suspicious block 
in BaseEventLoop._run_once: 
https://github.com/python/cpython/blob/v3.9.0a3/Lib/asyncio/base_events.py#L1873

handle = self._ready.popleft()
if handle._cancelled:
continue
if self._debug:
...
handle._run()
...
else:
handle._run()

As you can see, a callback is popped from the dequeue of ready callbacks, and 
only after a couple of lines that callback is called. The question arises, what 
happens if an exception is raised in between? Or more specifically, What 
happens to that callback if a KeyboardInterrupt is raised before it is called?
Well, appparently it dies and becomes one with the universe. The chances of it 
happening are the highest when the ioloop is running very short coroutines 
(like sleep(0)), and are increased when debug is on (because more code is 
executed in between).

This is how the bug we've been experiencing came to life:
When SIGINT is received it raises a KeyboardInterrupt in the running frame. If 
the running frame is a coroutine, it stops, the exception climbs up the stack, 
and the ioloop shuts down. Otherwise, the KeyboardInterrupt is probably raised 
inside asyncio's code, somewhere inside run_forever. In that case, the ioloop 
stops and proceeds to cancel all of the running tasks. After cancelling all the 
tasks, asyncio actually reruns the ioloop so all tasks receive the 
CancelledError and handle it or just die (see 
asyncio.runners._cancel_all_tasks).
Enter our bug; sometimes, randomly, the loop gets stuck waiting for all the 
cancelled tasks to finish. This behavior is caused by the flaw I described 
earlier - if the KeyboardInterrupt was raised after a callback was popped and 
before it was run, the callback is lost and the task that was waiting for it 
will wait forever.
Depending on the running tasks, the event loop might hang on the select call 
(until a interrupted by a signal, like SIGINT). This is what happens in 
SleepTest.py. Another case might be that only a part of the ioloop gets stuck, 
and other parts that are not dependent on the lost call still run correctly 
(and run into a CancelledError). This behavior is demonstrated in the script I 
added to this thread, asyncio_bug_demo.py.

I see two possible solutions:
1. Make all the code inside run_forever signal safe
2. Override the default SIGINT handler in asyncio.run with one more fitting the 
way asyncio works

I find the second solution much easier to implement well, and I think it makes 
more sense. I think python's default SIGINT handler fits normal single-threaded 
applications very well, but not so much an event loop. When using an event loop 
it makes sense to handle a signal as an event an process it along with the 
other running tasks. This is fully supported by the ioloop with the help of 
signal.set_wakeup_fd.
I have implemented the second solution and opened a PR, please review it and 
tell me what you think!

--
Added file: https://bugs.python.org/file48906/asyncio_bug_demo.py

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Change by Maor Kleinberger :


--
keywords: +patch
pull_requests: +17992
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/18628

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Zhibin Dong]

Zhibin Dong  added the comment:

Thank you for your investigation, I will also try to see what went wrong.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

I wrote a small script that checks the behavior of asyncio.sleep when called 
with small amounts of time (starting from 0). For each amount I checked 500 
times whether SleepTest has stopped when interrupted with SIGINT. Below are the 
results:

SLEEP TIME  SIGINT FAILURE PERCENT
0   26.6%
1e-06   9.8%
2e-06   11.8%
3e-06   11.2%
4e-06   9.6%
5e-06   13.8%
6e-06   5.6%
7e-06   2.6%
8e-06   1.4%
9e-06   1.6%
1e-05   2.2%
1.1e-05 2.2%
1.2e-05 2.0%
1.3e-05 1.8%
1.4e-05 0.8%

It is worth mentioning that the failure amount increased when my CPU was 
busier. Maybe it is because of false positives in my script (although I think I 
used relatively generous timeouts when waiting for the ioloop to start running 
and for the process to die after a SIGINT) and maybe it is because of the 
nature of the bug in asyncio. I didn't put a lot of effort into distinguishing 
between the two, as increasing the timeouts also made my CPU less busy.

Anyway, there is definitely a weird behavior here, and I still think there is a 
specific piece of code in asyncio that's causing it. Maybe I'll look at the 
code a bit, or try to think of other ways to approach this.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

This behaved similarly on my machine, also ubuntu. But it also happened (less 
often) with small numbers, like sleep(0.01).

Also, I tried it on my windows 10 and experienced another unexpected behavior - 
only when using sleep(0), Ctrl-C would not work *at all* from time to time. 
Most of the times it would stop the program immediately. This also happened 
when using 0.01 instead of 0.

Unless 0.01 turns into 0 somewhere along the way, I'd 
suspect that there is a piece of code inside asyncio's core that doesn't handle 
interrupting correctly (both on linux and windows), and using sleep with small 
numbers just makes it much more probable for this piece of code to be running 
when the Ctrl-C signal is sent.

I'll try to dig in more.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Zhibin Dong]

Zhibin Dong  added the comment:

Ubuntu 18.04.4 with kernel 5.3.0-28-generic. Additionally Python version is 
3.7.3.

--

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Maor Kleinberger]

Maor Kleinberger  added the comment:

On which OS did that happen?

--
nosy: +kmaork

___
Python tracker 

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

[issue39622] KeyboardInterrupt is ignored when await asyncio.sleep(0)

[Zhibin Dong]

New submission from Zhibin Dong :

As shown in the code, when 0 is passed to asyncio.sleep function, sometimes the 
program does not exit when I press . I must press  again to 
close the program. However, when a number, such as 0.01, which is bigger than 
0, is passed to the sleep function, the program will exit as expected when I 
press  just once.

Is there any bug or just a wrong way to use? Thanks.

--
components: asyncio
files: SleepTest.py
messages: 361939
nosy: Zhibin Dong, asvetlov, yselivanov
priority: normal
severity: normal
status: open
title: KeyboardInterrupt is ignored when await asyncio.sleep(0)
type: behavior
versions: Python 3.7
Added file: https://bugs.python.org/file48893/SleepTest.py

___
Python tracker 

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