Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The C implementation of asyncio.Task currently fails to perform the
cancellation cleanup correctly in the following scenario.
async def task1():
async def task2():
await task3 # task3 is never cancelled
asyncio.current_task().cancel()
await asyncio.create_task(task2())
The actuall error is a hardcoded call to `future_cancel()` instead of
calling the `cancel()` method of a future-like object.
Thanks to Vladimir Matveev for noticing the code discrepancy and to
Yury Selivanov for coming up with a pathological scenario.
| @@ -2713,14 +2713,19 @@ task_step_impl(TaskObj *task, PyObject *exc) | |||
|
|
|||
| if (task->task_must_cancel) { | |||
| PyObject *r; | |||
| r = future_cancel(fut); | |||
| int is_true; | |||
| r = _PyObject_CallMethodId(fut, &PyId_cancel, NULL); | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The fix is correct.
But I'd like to go one step forward: use Future_CheckExact and Task_CheckExact to call future_cancel() and _asyncio_Task_cancel_impl() as fast paths.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Id keep it as simple as possible as it needs to be backported to 3.6 & 3.7. A proper refactoring for C code is needed anyways, and I'll add this optimization.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ok
|
Sorry, @elprans and @1st1, I could not cleanly backport this to |
|
Sorry, @elprans and @1st1, I could not cleanly backport this to |
|
@elprans please submit two PRs to backport the change to 3.7 and 3.6. The automatic cherry-picking has failed :( |
|
GH-9690 is a backport of this pull request to the 3.6 branch. |
….Task (pythonGH-9679) The C implementation of asyncio.Task currently fails to perform the cancellation cleanup correctly in the following scenario. async def task1(): async def task2(): await task3 # task3 is never cancelled asyncio.current_task().cancel() await asyncio.create_task(task2()) The actuall error is a hardcoded call to `future_cancel()` instead of calling the `cancel()` method of a future-like object. Thanks to Vladimir Matveev for noticing the code discrepancy and to Yury Selivanov for coming up with a pathological scenario. (cherry picked from commit 548ce9d)
|
GH-9691 is a backport of this pull request to the 3.7 branch. |
….Task (pythonGH-9679) The C implementation of asyncio.Task currently fails to perform the cancellation cleanup correctly in the following scenario. async def task1(): async def task2(): await task3 # task3 is never cancelled asyncio.current_task().cancel() await asyncio.create_task(task2()) The actuall error is a hardcoded call to `future_cancel()` instead of calling the `cancel()` method of a future-like object. Thanks to Vladimir Matveev for noticing the code discrepancy and to Yury Selivanov for coming up with a pathological scenario.. (cherry picked from commit 548ce9d) Co-authored-by: Elvis Pranskevichus <elvis@magic.io>
….Task (GH-9679) (GH-9690) The C implementation of asyncio.Task currently fails to perform the cancellation cleanup correctly in the following scenario. async def task1(): async def task2(): await task3 # task3 is never cancelled asyncio.current_task().cancel() await asyncio.create_task(task2()) The actuall error is a hardcoded call to `future_cancel()` instead of calling the `cancel()` method of a future-like object. Thanks to Vladimir Matveev for noticing the code discrepancy and to Yury Selivanov for coming up with a pathological scenario. (cherry picked from commit 548ce9d) https://bugs.python.org/issue34872
….Task (GH-9679) (GH-9691) The C implementation of asyncio.Task currently fails to perform the cancellation cleanup correctly in the following scenario. async def task1(): async def task2(): await task3 # task3 is never cancelled asyncio.current_task().cancel() await asyncio.create_task(task2()) The actuall error is a hardcoded call to `future_cancel()` instead of calling the `cancel()` method of a future-like object. Thanks to Vladimir Matveev for noticing the code discrepancy and to Yury Selivanov for coming up with a pathological scenario.. (cherry picked from commit 548ce9d) Co-authored-by: Elvis Pranskevichus <elvis@magic.io> https://bugs.python.org/issue34872
…ythonGH-9679) The C implementation of asyncio.Task currently fails to perform the cancellation cleanup correctly in the following scenario. async def task1(): async def task2(): await task3 # task3 is never cancelled asyncio.current_task().cancel() await asyncio.create_task(task2()) The actuall error is a hardcoded call to `future_cancel()` instead of calling the `cancel()` method of a future-like object. Thanks to Vladimir Matveev for noticing the code discrepancy and to Yury Selivanov for coming up with a pathological scenario.
The C implementation of asyncio.Task currently fails to perform the
cancellation cleanup correctly in the following scenario.
The actuall error is a hardcoded call to
future_cancel()instead ofcalling the
cancel()method of a future-like object.Thanks to Vladimir Matveev for noticing the code discrepancy and to
Yury Selivanov for coming up with a pathological scenario.
https://bugs.python.org/issue34872