bpo-30679: __aexit__ is not called on KeyboardInterrupt

[arthurdarcet]

Hi,

Here is the example code I am running:

import asyncio

class it:
	async def __aenter__(self):
		return self
	async def __aexit__(self, *_):
		print('EXIT')

async def main():
	async with it():
		await asyncio.sleep(100)

asyncio.get_event_loop().run_until_complete(main())

When this gets interrupted by a SIGINT,I would expect this code to display EXIT before the KeyboardInterrupt stacktrace. But instead the __aexit__ function is simply not called.

Here is how I understand things:

A KeyboardInterrupt gets raised in loop.run_forever, which is caught by the except clause in run_until_complete. But this clause is supposed to handle errors that occurs in the coroutine, and not outside of it. And so, when this KeyboardInterrupt is raised again at the end of the except, the underlying coroutine is not cancelled and simply discarded (and the associated warning is hidden because it is explicitly disabled here.

The proposed change cancels the underlying coroutine when run_forever is interrupted from outside, and then relaunch run_forever so that any remaining event handler can finish. It will be stopped right after because of the done_callback that is still present.

I have absolutely no idea if this proposed solution is anywhere near the right approach. If it is, I will add some tests to this PR.

Thank you, and sorry for the noise if this is "expected behaviour"

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

[1st1]

What's a 'signal'? It could be a simple unhandled exception. Right now, the 'except' clause is too broad to do what you are doing here.

[arthurdarcet]

If we are running a coroutine (ie new_task is True), and future.done() is false, then the exception is not coming from inside the coroutine and has to be some kind of external signal.

The raw except: clause is very broad indeed, but the exception can actually be any kind of user defined exception here, so it makes sense to catch them all. For instance, the signal example from the docs would raise an OSError:

import asyncio, signal

def handler(signum, frame):
    raise OSError("Couldn't open device!")
signal.signal(signal.SIGALRM, handler)
signal.alarm(1)

loop = asyncio.get_event_loop()
loop.run_until_complete(asyncio.sleep(10))

And in this example, the sleep(10) coroutine is never cancelled. This isn't an issue here, but is an issue in the example I provided with the PR: if you mix async contextes into this, the __aexit__ are not called

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[arthurdarcet]

This is still an existing problem with the latest version of Python, and as far as I can see, aexit is pretty broken because of this. @1st1 I replied to your comment back in 2017 - can you let me know how you think we should fix this if this PR isn't ok?

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[kumaraditya303]

This is already fixed as commented on the issue #74864.