Issue 34872: investigate task/future cancellation in asynciomodule.c

Vladimir Matveev has discovered that C and Python implementation of asyncio.Task diverge:

* asynciomodule.c:
https://github.com/python/cpython/blob/9012a0fb4c4ec1afef9efb9fdb0964554ea17983/Modules/_asynciomodule.c#L2716

* tasks.py:
https://github.com/python/cpython/blob/9012a0fb4c4ec1afef9efb9fdb0964554ea17983/Lib/asyncio/tasks.py#L286

In tasks.py we call "fut_obj.cancel()", so either "Task.cancel()" or "Future.cancel()" can be called depending on what "fut_obj" is.

In asynciomodule.c we essentially always call "Future.cancel(fut_obj)".  This probably leads to unexpected behaviour if "fut_obj" is either a Task or a Task subclass with an overloaded "cancel()" method.

We need to first write a set of tests to understand the scope of this; then we'll need to patch asynciomodule.c and backport this to 3.7 & 3.6.

Elvis, please take a look at this.

Ned, will we have 3.7.1rc2?  If so, would it be possible to include the fix for this one?

New changeset 0c797a6aca1c293e530e18c5e9fa02c670a9a4ed by Yury Selivanov (Elvis Pranskevichus) in branch 'master':
bpo-34872: Fix self-cancellation in C implementation of asyncio.Task (GH-9679)
https://github.com/python/cpython/commit/0c797a6aca1c293e530e18c5e9fa02c670a9a4ed

New changeset 166773df0ce6c852130f524029fa2e62b37b89cb by Miss Islington (bot) (Elvis Pranskevichus) in branch '3.6':
[3.6] bpo-34872: Fix self-cancellation in C implementation of asyncio.Task (GH-9679) (GH-9690)
https://github.com/python/cpython/commit/166773df0ce6c852130f524029fa2e62b37b89cb

New changeset a67bd53d3f80ac9c518b5426aa35459bd373b2b3 by Miss Islington (bot) (Elvis Pranskevichus) in branch '3.7':
[3.7] bpo-34872: Fix self-cancellation in C implementation of asyncio.Task (GH-9679) (GH-9691)
https://github.com/python/cpython/commit/a67bd53d3f80ac9c518b5426aa35459bd373b2b3

Ned, elevating this to "release blocker", see https://bugs.python.org/msg326928

Feel free to close this issue.

Prior to this, there were no plans to have a 3.7.1rc2.  We have three options:

1. do a 3.7.1rc2 with this and possibly a few other cherry-picked changes
2. just cherry-pick this fix into 3.7.1 final
3. wait for 3.7.2 (in one to three months)

What's your assessment of the risk of option 2?

> What's your assessment of the risk of option 2?

Let's not rush it in without testing, for sure.

Having 3.7.1rc2 would be great, but really up to you.

Seems this change introduced a compiler warning.

/home/serhiy/py/cpython3.7/Modules/_asynciomodule.c: In function ‘task_step_impl’:
/home/serhiy/py/cpython3.7/Modules/_asynciomodule.c:2666:44: warning: passing argument 1 of ‘_PyObject_CallMethodId’ from incompatible pointer type [-Wincompatible-pointer-types]
                 r = _PyObject_CallMethodId(fut, &PyId_cancel, NULL);
                                            ^~~
In file included from ./Include/Python.h:128:0,
                 from /home/serhiy/py/cpython3.7/Modules/_asynciomodule.c:1:
./Include/abstract.h:315:24: note: expected ‘PyObject * {aka struct _object *}’ but argument is of type ‘FutureObj * {aka struct <anonymous> *}’
 PyAPI_FUNC(PyObject *) _PyObject_CallMethodId(PyObject *obj,
                        ^~~~~~~~~~~~~~~~~~~~~~

New changeset addf8afb43af58b9bf56a0ecfd0f316dd60ac0c3 by Serhiy Storchaka in branch 'master':
Fix a compiler warning added in bpo-34872. (GH-9722)
https://github.com/python/cpython/commit/addf8afb43af58b9bf56a0ecfd0f316dd60ac0c3

New changeset d9212200fe8ddb55d73b8231869cfbb32635ba92 by Serhiy Storchaka in branch '3.7':
[3.7] Fix a compiler warning added in bpo-34872. (GH-9722). (GH-9726)
https://github.com/python/cpython/commit/d9212200fe8ddb55d73b8231869cfbb32635ba92

New changeset dd0670f12b159eff5336d6011f046e1ccac495e1 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
Fix a compiler warning added in bpo-34872. (GH-9722). (GH-9726) (GH-9728)
https://github.com/python/cpython/commit/dd0670f12b159eff5336d6011f046e1ccac495e1

Is there any reason this issue needs to remain open?  Or can we at least remove the "release blocker" priority?

Closing it now, Ned, thanks!  I assume it will make it into 3.7.1rc2, right?

Both 3.7.1rc2 and 3.6.7rc2

resolution: fixed
stage: patch review -> resolved

messages: + msg326985