Stowage/cpython
2
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-10-31 05:31:20 +00:00
Code Issues Projects Releases Packages Wiki Activity

bpo-46771: Remove two controversial lines from Task.cancel() (GH-31623)

Browse source 
Also from the _asyncio C accelerator module,
and adjust one test that the change caused to fail.

For more discussion see the discussion starting here:
https://github.com/python/cpython/pull/31394#issuecomment-1053545331

(Basically, @asvetlov proposed to return False from cancel()
when there is already a pending cancellation, and I went along,
even though it wasn't necessary for the task group implementation,
and @agronholm has come up with a counterexample that fails
because of this change.  So now I'm changing it back to the old
semantics (but still bumping the counter) until we can have a
proper discussion about this.)
This commit is contained in:
Guido van Rossum 2022-02-28 15:15:56 -08:00 • committed by GitHub
parent 08deed1af5
commit 7d611b4cab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 17 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

7
Lib/asyncio/tasks.py
View file

@ -205,8 +205,11 @@ def cancel(self, msg=None):
if self.done(): if self.done():
return False return False
self._num_cancels_requested += 1 self._num_cancels_requested += 1
if self._num_cancels_requested > 1: # These two lines are controversial. See discussion starting at
return False # https://github.com/python/cpython/pull/31394#issuecomment-1053545331
# Also remember that this is duplicated in _asynciomodule.c.
# if self._num_cancels_requested > 1:
# return False
if self._fut_waiter is not None: if self._fut_waiter is not None:
if self._fut_waiter.cancel(msg=msg): if self._fut_waiter.cancel(msg=msg):
# Leave self._fut_waiter; it may be a Task that # Leave self._fut_waiter; it may be a Task that

6
Lib/test/test_asyncio/test_tasks.py
View file

@ -514,7 +514,11 @@ async def task():
self.assertTrue(t.cancel()) self.assertTrue(t.cancel())
self.assertTrue(t.cancelling()) self.assertTrue(t.cancelling())
self.assertIn(" cancelling ", repr(t)) self.assertIn(" cancelling ", repr(t))
self.assertFalse(t.cancel())
# Since we commented out two lines from Task.cancel(),
# this t.cancel() call now returns True.
# self.assertFalse(t.cancel())
self.assertTrue(t.cancel())
with self.assertRaises(asyncio.CancelledError): with self.assertRaises(asyncio.CancelledError):
loop.run_until_complete(t) loop.run_until_complete(t)

10
Modules/_asynciomodule.c
View file

@ -2206,9 +2206,13 @@ _asyncio_Task_cancel_impl(TaskObj *self, PyObject *msg)
} }
self->task_num_cancels_requested += 1; self->task_num_cancels_requested += 1;
if (self->task_num_cancels_requested > 1) {
Py_RETURN_FALSE; // These three lines are controversial. See discussion starting at
} // https://github.com/python/cpython/pull/31394#issuecomment-1053545331
// and corresponding code in tasks.py.
// if (self->task_num_cancels_requested > 1) {
// Py_RETURN_FALSE;
// }
if (self->task_fut_waiter) { if (self->task_fut_waiter) {
PyObject *res; PyObject *res;