Heh, yeah, I may not have read your initial post in this thread too closely... :-(
I still think it should be possible to be more principled about how to handle BaseException -- it seems about 50/50 whether it is treated the same or differently. Regarding call_exception_handler(), I think it's intentional that it doesn't let exceptions in the handler bubble out; in many call sites there either is nothing else to be done anyway (e.g. it's called from various __del__ methods) or the call site has an explicit assumption that it can do more cleanup after it returns. This does remind me of an unfortunate thing: if KeyboardInterrupt is caught while a __del__ method is running, *nothing* the code can do can cause the KeyboardInterrupt to properly bubble up -- it is logged when __del__ exits. But this is a Python problem, not an asyncio problem, and what we decide to do in call_exception_handler() doesn't change this. Basically the handling of normal exceptions, when they arrive during execution of a coroutine, is to transfer them to the Task wrapping the coroutine (there always is one, possibly further up the stack). I think it would be reasonable to try doing the same thing with BaseExceptions; or at least to experiment and see what happens if we do that. Hopefully you're still game for this experiment! On Sat, Dec 5, 2015 at 6:45 PM, Justin Mayfield <too...@gmail.com> wrote: > Your reply makes me think I have muddled two issues together. However, > they may be linked so I'm moving forward with an experiment to unify > exception handling such that BaseException handling is closer to > Exception's handling. The consequences of such a change are going to be > big from my initial investigation. > > The initial reason for my post was intended to help me understand behavior > that felt erroneous. Namely a few cases where exception handlers ARE > catching BaseException via naked except clauses and then making assumptions > about the event loop state. I suppose I was trying to articulate that an > argument could be made for adjusting these cases to be more like the > current asyncio exception handling strategy (ie. don't catch BaseException). > > That being said, I tend to agree with your thinking of making asyncio > robust enough to handle BaseException so that a user's code is not limited > in their exception handling scope. Since these changes will likely affect > and change the nature of the the corner cases in question, it's probably > best to just focus BaseException handling first. > > One minor note about this; I notice that call_exception_handler() has a > peculiar model where a user defined exception handler is guarded from > raising its own exception; It calls the default exception handler instead, > as described by this thread, > https://groups.google.com/d/topic/python-tulip/XrcqtuXwMVc/discussion . > I wonder if this deserves a second look in light of this discussion. My > personal take is that exceptions from call_exception_handler are better > served if they can blow through the stack. This could be their only > opportunity to dirty-stop the event loop if BaseException is no longer > special. It also seems to more obvious and more compatible with common > save/restore patterns. > > JM > > On Friday, December 4, 2015 at 9:51:07 PM UTC-7, Guido van Rossum wrote: >> >> On Fri, Dec 4, 2015 at 11:39 AM, Justin Mayfield <too...@gmail.com> >> wrote: >> >>> Hi Guido, >>> >>> Yes, I'd be interested in that. >>> >> >> This would be a great way to get your hands dirty! >> >> >>> There are two things off the top of my head that I don't have a solid >>> grasp on. >>> >>> 1. The effect of catching BaseException unilaterally and its affect on >>> generator protocol. I'm not fluent enough in the Task mech to understand >>> if this would break things. >>> >> >> Hm, I don't think that BaseException is treated specially by the >> generator protocol. Now, there might be some BaseExceptions where catching >> them isn't going to help much. MemoryError comes to mind, but catching it >> is unlikely to make things worse -- they are already pretty bad at that >> point, and if it gets re-raised it'll likely eventually just bubble up the >> stack until the program exits -- just as if you don't catch it. >> >> The bottom line is that catching exceptions isn't going to mess with the >> interpreter's integrity, so whatever you do shouldn't overly worry you. >> >> >>> 2. Having seen the issue in asyncio and also aiohttp I assume there will >>> be a never ending list of libraries that sometimes catch BaseException and >>> some that do not. This may be unsolvable by trying to achieve consistency >>> everywhere unless we can somehow convince an entire community that there is >>> one, and only one, way to do exception handling in async code. That just >>> seems like an impossible task to me but your influence may tell another >>> story. >>> >> >> I can't force people to change their code. And I don't think we should >> worry about this -- if a library doesn't catch BaseException, it'll be >> dealt with by asyncio itself, and that should be sufficient. If for a >> particular use case it isn't, that is between the user and the library >> author. Dealing with BaseException properly in asyncio should give both >> users and library authors the confidence that this *can* be dealt with (as >> opposed to the current situation where asyncio itself doesn't really try, >> so why would other libraries bother). >> >> >>> I'll do some experiments this weekend and report back. Please advise if >>> you want an issue filed and I'll spend some time writing a test that fails. >>> >> >> It's always better to track a specific issue in a tracker than in a >> general mailing list. In this case I recommend using asyncio's own tracker ( >> https://github.com/python/asyncio) so we can refine the approach without >> bothering the other CPython developers. >> >> Thanks again for volunteering to look into this! >> >> --Guido >> >> >>> JM >>> >>> On Fri, Dec 4, 2015 at 12:08 PM Guido van Rossum <gu...@python.org> >>> wrote: >>> >>>> Hi Justin, >>>> >>>> I think in the long run the event loop (and everything in asyncio) >>>> needs to be much more careful with BaseExceptions. Their treatment is >>>> uneven -- if we're writing "except Exception" they are not caught, but when >>>> we're writing "finally" or "except: ...; raise" they are caught. I think >>>> the right thing to do is to tread BaseException the same way we're treating >>>> Exception, *except* that in cases where we catch it and continue running >>>> the event loop we should probably still break out of the event loop right >>>> away. So a KeyboardError would still break out of the event loop (without >>>> executing the remaining scheduled callbacks; or perhaps after, similar to >>>> the way the new stop() behaves), but the event loop would be in a >>>> consistent state -- if you were to restart the loop the KeyboardError would >>>> be delivered to the callback to which an Exception class would be >>>> delivered. >>>> >>>> Hope this description makes sense. >>>> >>>> Maybe you're interested in improving asyncio by submitting a patch, >>>> given that you're running into this problem and so have a motivation as >>>> well as a use case that can drive the improvements? >>>> >>>> On Fri, Dec 4, 2015 at 12:54 AM, Justin Mayfield <too...@gmail.com> >>>> wrote: >>>> >>>>> I have an experimental library where I manage the lifecycle of event >>>>> loops for purposes of allowing async routines to operate inside a >>>>> classical >>>>> generator (among other things). I create a new event loop for each use of >>>>> my routine and upon completion or GC this event loop is closed. >>>>> >>>>> My recent testing has left me a bit perplexed as to the right way to >>>>> deal with KeyboardInterrupt and possibly other BaseException exceptions >>>>> that halt execution of an event loop. I've read a few posts on this list >>>>> suggesting that the basic rule of thumb employed within asyncio itself, is >>>>> to only catch Exception because BaseException is generally considered >>>>> unrecoverable (or coro protocol as in GeneratorExit). However I seem to >>>>> have found several instances where this rule is not followed and in the >>>>> except clause, further calls are made that expect the event loop to be in >>>>> good standing. >>>>> >>>>> Because I close the event loop when my routine is done/interrupted the >>>>> eventual GeneratorExit exception thrown by the garbage collector triggers >>>>> some cleanup/close routines in asyncio itself (as well as aiohttp) that >>>>> eventually find themselves staring at RuntimeError('Event loop is >>>>> closed'). >>>>> >>>>> My question to the community is if I'm witnessing bugs in these places >>>>> where naked except clauses are used to perform cleanup/closing actions on >>>>> an event loop or if I'm fundamentally doing something wrong. I hope it's >>>>> not the latter as I've been pretty happy with the results of my experiment >>>>> baring these corner cases. >>>>> >>>>> [Code in question is here: >>>>> https://github.com/mayfield/cellulario/blob/master/cellulario/iocell.py >>>>> ] >>>>> >>>>> Example traceback.. >>>>> >>>>> Traceback (most recent call last): >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", >>>>> line 692, in _create_connection_transport >>>>> >>>>> yield from waiter >>>>> >>>>> GeneratorExit >>>>> >>>>> >>>>> During handling of the above exception, another exception occurred: >>>>> >>>>> >>>>> Traceback (most recent call last): >>>>> >>>>> File >>>>> "/Users/mayfield/project/syndicate/syndicate/adapters/async.py", line 74, >>>>> in request >>>>> >>>>> result = yield from asyncio.wait_for(r, timeout, loop=self.loop) >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/tasks.py", >>>>> line 359, in wait_for >>>>> >>>>> return (yield from fut) >>>>> >>>>> File >>>>> "/usr/local/lib/python3.5/site-packages/aiohttp-0.18.3-py3.5-macosx-10.10-x86_64.egg/aiohttp/client.py", >>>>> line 456, in __iter__ >>>>> >>>>> resp = yield from self._coro >>>>> >>>>> File >>>>> "/usr/local/lib/python3.5/site-packages/aiohttp-0.18.3-py3.5-macosx-10.10-x86_64.egg/aiohttp/client.py", >>>>> line 173, in _request >>>>> >>>>> conn = yield from self._connector.connect(req) >>>>> >>>>> File >>>>> "/usr/local/lib/python3.5/site-packages/aiohttp-0.18.3-py3.5-macosx-10.10-x86_64.egg/aiohttp/connector.py", >>>>> line 289, in connect >>>>> >>>>> transport, proto = yield from self._create_connection(req) >>>>> >>>>> File >>>>> "/usr/local/lib/python3.5/site-packages/aiohttp-0.18.3-py3.5-macosx-10.10-x86_64.egg/aiohttp/connector.py", >>>>> line 557, in _create_connection >>>>> >>>>> server_hostname=hinfo['hostname'] if sslcontext else None) >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", >>>>> line 669, in create_connection >>>>> >>>>> sock, protocol_factory, ssl, server_hostname) >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", >>>>> line 694, in _create_connection_transport >>>>> >>>>> transport.close() >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/selector_events.py", >>>>> line 566, in close >>>>> >>>>> self._loop.call_soon(self._call_connection_lost, None) >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", >>>>> line 453, in call_soon >>>>> >>>>> handle = self._call_soon(callback, args) >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", >>>>> line 462, in _call_soon >>>>> >>>>> self._check_closed() >>>>> >>>>> File >>>>> "/usr/local/Cellar/python3/3.5.0/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", >>>>> line 289, in _check_closed >>>>> >>>>> raise RuntimeError('Event loop is closed') >>>>> >>>>> >>>> >>>> >>>> -- >>>> --Guido van Rossum (python.org/~guido) >>>> >>> >> >> >> -- >> --Guido van Rossum (python.org/~guido) >> > -- --Guido van Rossum (python.org/~guido)
