Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

Handle BaseException in event loop. #305

Closed
wants to merge 3 commits into from
Closed

Handle BaseException in event loop. #305

wants to merge 3 commits into from

Conversation

mayfield
Copy link

Prior to this patch the event loop was only intended to handle Exception. BaseException exceptions would break out of the event loop (mostly). This patch treats all exceptions equally so application code can throw BaseException and its derivatives.

Created this at Guido's request..

ref: https://groups.google.com/forum/#!topic/python-tulip/QK7oDGJwhXg

First pass at treating all exceptions in an event loop as equal.
BaseExceptions like KeyboardInterrupt are not raised by run_forever.
Grammar fix and removal of outdated comment.
@vstinner
Copy link
Member

vstinner commented Jan 3, 2016 via email

@gvanrossum
Copy link
Member

There was some discussion on the python-tulip list. See also
#276 (which currently has no
motivation either).

IMO the main problem with the existing approach (catching Exception and
subclasses only) is that if you ever catch KeyboardInterrupt and then
attempt to clean up you are likely to find various internal asyncio data
structures corrupted, because of the inconsistent treatment of
BaseException. (It's not that the latter is never caught -- every 'finally'
clause catches it and there are some 'except BaseException' clauses in the
code as well.)

The signal solution is indeed nicer, because it ensures that interrupts are
treated as regular asyncio events, but it means you can't interrupt code
that's stuck in a tight CPU loop (e.g. while True: pass), and it requires
more sophistication from users.

I don't really think that catching BaseException everywhere we currently
catch Exception makes anything worse -- do you?

On Sun, Jan 3, 2016 at 2:37 AM, Victor Stinner notifications@github.com
wrote:

Can you please elaborate the use case? Do you want to catch all base
exceptions? Or only KeyboardInterrupt?

Why not setting a handler for SIGINT signal to catch KeyboardInterrupt in
the regular adyncio control flow?


Reply to this email directly or view it on GitHub
#305 (comment).

--Guido van Rossum (python.org/~guido)

@vstinner
Copy link
Member

vstinner commented Jan 4, 2016

2016-01-03 22:12 GMT+01:00 Guido van Rossum notifications@github.com:

The signal solution is indeed nicer, because it ensures that interrupts are
treated as regular asyncio events, but it means you can't interrupt code
that's stuck in a tight CPU loop (e.g. while True: pass), and it requires
more sophistication from users.

The C signal handler is always called immediatly, but the
KeyboardInterrupt is raised "later". I don't think that expensive
CPU-bound blocking call is interrupted by CTRL+c. And it would be a
really bad idea to call such function in the thread running an event
loop.

Victor

@gvanrossum
Copy link
Member

On Mon, Jan 4, 2016 at 12:45 AM, Victor Stinner notifications@github.com
wrote:

2016-01-03 22:12 GMT+01:00 Guido van Rossum notifications@github.com:

The signal solution is indeed nicer, because it ensures that interrupts
are
treated as regular asyncio events, but it means you can't interrupt code
that's stuck in a tight CPU loop (e.g. while True: pass), and it requires
more sophistication from users.

The C signal handler is always called immediatly, but the
KeyboardInterrupt is raised "later". I don't think that expensive
CPU-bound blocking call is interrupted by CTRL+c. And it would be a
really bad idea to call such function in the thread running an event
loop.

But such things happen all the time, by accident or while people are
learning, and most of the time the loop is written in Python, and the ^C
will interrupt it (though not on Windows IIRC).

--Guido van Rossum (python.org/~guido)

@kxepal
Copy link

kxepal commented Mar 1, 2016

Can you please elaborate the use case?

Another use case was found in Hypothesis testing library: HypothesisWorks/hypothesis#292 (comment)

@1st1
Copy link
Member

1st1 commented Sep 15, 2016

I'm going to close this PR. First it's fairly outdates. Second, we aren't sure that we have a problem with BaseExceptions. Seems that the corner case is actually handling KeyboardInterrupt correctly, but that will require a separate discussion and a different PR.

@1st1 1st1 closed this Sep 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants