[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[Yury Selivanov]

Change by Yury Selivanov :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[STINNER Victor]

STINNER Victor  added the comment:

Can this issue be closed now?

--

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[STINNER Victor]

STINNER Victor  added the comment:


New changeset bfe1f74e39d0049a829962050e86a6a2d2a2781e by Victor Stinner in 
branch '3.8':
[3.8] bpo-3832: Fix compiler warnings (GH-16518)
https://github.com/python/cpython/commit/bfe1f74e39d0049a829962050e86a6a2d2a2781e


--

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[STINNER Victor]

STINNER Victor  added the comment:


New changeset efe74b6369a8d08f27c69703fcc1686966e51068 by Victor Stinner in 
branch 'master':
bpo-38321: Fix _asynciomodule.c compiler warning (GH-16493)
https://github.com/python/cpython/commit/efe74b6369a8d08f27c69703fcc1686966e51068


--
nosy: +vstinner

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +16081
pull_request: https://github.com/python/cpython/pull/16493

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[Carol Willing]

Carol Willing  added the comment:


New changeset 16cec136b75daf438080a5b6685d2679dfa406af by Carol Willing (Miss 
Islington (bot)) in branch '3.8':
bpo-38248: Fix inconsistent immediate asyncio.Task cancellation (GH-16330) 
(GH-16383)
https://github.com/python/cpython/commit/16cec136b75daf438080a5b6685d2679dfa406af


--

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[miss-islington]

Change by miss-islington :


--
pull_requests: +15963
pull_request: https://github.com/python/cpython/pull/16383

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[miss-islington]

Change by miss-islington :


--
pull_requests: +15963, 15964, 15965
pull_request: https://github.com/python/cpython/pull/16383

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[miss-islington]

Change by miss-islington :


--
pull_requests: +15963, 15964
pull_request: https://github.com/python/cpython/pull/16383

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[Carol Willing]

Carol Willing  added the comment:


New changeset edad4d89e357c92f70c0324b937845d652b20afd by Carol Willing (Yury 
Selivanov) in branch 'master':
bpo-38248: Fix inconsistent immediate asyncio.Task cancellation (GH-16330)
https://github.com/python/cpython/commit/edad4d89e357c92f70c0324b937845d652b20afd


--
nosy: +willingc

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[Yury Selivanov]

Change by Yury Selivanov :


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

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[Yury Selivanov]

Yury Selivanov  added the comment:

> The obvious fix would be to modify the 'except StopIteration' branch to 
> handle this case by calling super().cancel() instead of 
> super().set_exception(...).

Yeah, I think this is the solution we should do in 3.8.

--

___
Python tracker 

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

[issue38248] inconsistency in asyncio.Task between cancellation while running vs. cancellation immediately after it finishes

[Nathaniel Smith]

New submission from Nathaniel Smith :

Just noticed this while looking at the code to asyncio.Task.__step

asyncio Futures have two different error states: they can have an arbitrary 
exception set, or they can be marked as cancelled.

asyncio Tasks handle this by detecting what exception was raised by the task 
code: if it's a CancelledError, then the mark the Future as cancelled, and if 
it's anything else, they set that as the Future's exception.

There is also a special case handled inside the 'except StopIteration' clause 
in Task.__step. If we request that a Task be cancelled, but then the task exits 
normally before we have a chance to throw in a CancelledError, then we also 
want mark the Future as cancelled. BUT, this special case is handled 
incorrectly: it does Future.set_exception(CancelledError()), instead of 
Future.cancel(). Normally it's impossible for a task to end up with its 
exception set to CancelledError, but it does happen in this one weird edge 
case, basically as a race condition.

Here's some sample code to illustrate the problem (tested on 3.7):

--

import asyncio

# This gets cancelled normally
async def cancel_early():
asyncio.current_task().cancel()
await asyncio.sleep(1)

async def cancel_late():
asyncio.current_task().cancel()
# No sleep, so CancelledError doesn't get delivered until after the
# task exits

async def main():
t_early = asyncio.create_task(cancel_early())
await asyncio.sleep(0.1)
print(f"t_early.cancelled(): {t_early.cancelled()!r}")

t_late = asyncio.create_task(cancel_late())
await asyncio.sleep(0.1)
print(f"t_late.cancelled(): {t_late.cancelled()!r}")
print(f"t_late.exception(): {t_late.exception()!r}")

asyncio.run(main())


--

Output:

t_early.cancelled(): True
t_late.cancelled(): False
t_late.exception(): CancelledError()

The obvious fix would be to modify the 'except StopIteration' branch to handle 
this case by calling super().cancel() instead of super().set_exception(...).

Alternatively, I could see an argument that asyncio.Task should always preserve 
the CancelledError, so that e.g. you can get a proper traceback. In that case 
we'd need to change the 'except CancelledError' branch to call 
super().set_exception(...) instead of super().cancel(). This would also need 
some more changes, like overriding .cancelled() to check for a stored exception 
and then return isinstance(stored_exc, CancelledError), and maybe others... I'm 
not sure of the full consequences.

But handling these two cases differently is definitely wrong, that part I'm 
sure of :-)

--
messages: 352964
nosy: asvetlov, njs, yselivanov
priority: normal
severity: normal
status: open
title: inconsistency in asyncio.Task between cancellation while running vs. 
cancellation immediately after it finishes

___
Python tracker 

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