summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGuido van Rossum <guido@python.org>2022-08-17 01:23:06 (GMT)
committerGitHub <noreply@github.com>2022-08-17 01:23:06 (GMT)
commitf51f54f39d384da63be622bcdc9cf4cfb43bad3d (patch)
treebdedc65a5f961e9aa330f7d8b929d8df0c5a4bb6
parent9b30b965f0c1da216397b495faef4d93ff7a5328 (diff)
downloadcpython-f51f54f39d384da63be622bcdc9cf4cfb43bad3d.zip
cpython-f51f54f39d384da63be622bcdc9cf4cfb43bad3d.tar.gz
cpython-f51f54f39d384da63be622bcdc9cf4cfb43bad3d.tar.bz2
GH-95704: Don't suppress errors from tasks when TG is cancelled (#95761)
When a task catches CancelledError and raises some other error, the other error should not silently be suppressed. Any scenario where a task crashes in cleanup upon cancellation will now result in an ExceptionGroup wrapping the crash(es) instead of propagating CancelledError and ignoring the side errors. NOTE: This represents a change in behavior (hence the need to change several tests). But it is only an edge case. Co-authored-by: Thomas Grainger <tagrain@gmail.com>
-rw-r--r--Lib/asyncio/taskgroups.py7
-rw-r--r--Lib/test/test_asyncio/test_taskgroups.py54
-rw-r--r--Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst2
3 files changed, 35 insertions, 28 deletions
diff --git a/Lib/asyncio/taskgroups.py b/Lib/asyncio/taskgroups.py
index 9be4838..5d5e2a8 100644
--- a/Lib/asyncio/taskgroups.py
+++ b/Lib/asyncio/taskgroups.py
@@ -116,10 +116,9 @@ class TaskGroup:
if self._base_error is not None:
raise self._base_error
- if propagate_cancellation_error is not None:
- # The wrapping task was cancelled; since we're done with
- # closing all child tasks, just propagate the cancellation
- # request now.
+ # Propagate CancelledError if there is one, except if there
+ # are other errors -- those have priority.
+ if propagate_cancellation_error and not self._errors:
raise propagate_cancellation_error
if et is not None and et is not exceptions.CancelledError:
diff --git a/Lib/test/test_asyncio/test_taskgroups.py b/Lib/test/test_asyncio/test_taskgroups.py
index 74bae06..6a0231f 100644
--- a/Lib/test/test_asyncio/test_taskgroups.py
+++ b/Lib/test/test_asyncio/test_taskgroups.py
@@ -230,29 +230,29 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
self.assertEqual(NUM, 15)
- async def test_cancellation_in_body(self):
+ async def test_taskgroup_08(self):
async def foo():
- await asyncio.sleep(0.1)
- 1 / 0
+ try:
+ await asyncio.sleep(10)
+ finally:
+ 1 / 0
async def runner():
async with taskgroups.TaskGroup() as g:
for _ in range(5):
g.create_task(foo())
- try:
- await asyncio.sleep(10)
- except asyncio.CancelledError:
- raise
+ await asyncio.sleep(10)
r = asyncio.create_task(runner())
await asyncio.sleep(0.1)
self.assertFalse(r.done())
r.cancel()
- with self.assertRaises(asyncio.CancelledError) as cm:
+ with self.assertRaises(ExceptionGroup) as cm:
await r
+ self.assertEqual(get_error_types(cm.exception), {ZeroDivisionError})
async def test_taskgroup_09(self):
@@ -316,8 +316,10 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
async def test_taskgroup_11(self):
async def foo():
- await asyncio.sleep(0.1)
- 1 / 0
+ try:
+ await asyncio.sleep(10)
+ finally:
+ 1 / 0
async def runner():
async with taskgroups.TaskGroup():
@@ -325,24 +327,26 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
for _ in range(5):
g2.create_task(foo())
- try:
- await asyncio.sleep(10)
- except asyncio.CancelledError:
- raise
+ await asyncio.sleep(10)
r = asyncio.create_task(runner())
await asyncio.sleep(0.1)
self.assertFalse(r.done())
r.cancel()
- with self.assertRaises(asyncio.CancelledError):
+ with self.assertRaises(ExceptionGroup) as cm:
await r
+ self.assertEqual(get_error_types(cm.exception), {ExceptionGroup})
+ self.assertEqual(get_error_types(cm.exception.exceptions[0]), {ZeroDivisionError})
+
async def test_taskgroup_12(self):
async def foo():
- await asyncio.sleep(0.1)
- 1 / 0
+ try:
+ await asyncio.sleep(10)
+ finally:
+ 1 / 0
async def runner():
async with taskgroups.TaskGroup() as g1:
@@ -352,19 +356,19 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
for _ in range(5):
g2.create_task(foo())
- try:
- await asyncio.sleep(10)
- except asyncio.CancelledError:
- raise
+ await asyncio.sleep(10)
r = asyncio.create_task(runner())
await asyncio.sleep(0.1)
self.assertFalse(r.done())
r.cancel()
- with self.assertRaises(asyncio.CancelledError):
+ with self.assertRaises(ExceptionGroup) as cm:
await r
+ self.assertEqual(get_error_types(cm.exception), {ExceptionGroup})
+ self.assertEqual(get_error_types(cm.exception.exceptions[0]), {ZeroDivisionError})
+
async def test_taskgroup_13(self):
async def crash_after(t):
@@ -424,8 +428,9 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
self.assertFalse(r.done())
r.cancel()
- with self.assertRaises(asyncio.CancelledError):
+ with self.assertRaises(ExceptionGroup) as cm:
await r
+ self.assertEqual(get_error_types(cm.exception), {ZeroDivisionError})
async def test_taskgroup_16(self):
@@ -451,8 +456,9 @@ class TestTaskGroup(unittest.IsolatedAsyncioTestCase):
self.assertFalse(r.done())
r.cancel()
- with self.assertRaises(asyncio.CancelledError):
+ with self.assertRaises(ExceptionGroup) as cm:
await r
+ self.assertEqual(get_error_types(cm.exception), {ZeroDivisionError})
async def test_taskgroup_17(self):
NUM = 0
diff --git a/Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst b/Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst
new file mode 100644
index 0000000..31f9fc6
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-08-08-01-42-11.gh-issue-95704.MOPFfX.rst
@@ -0,0 +1,2 @@
+When a task catches :exc:`asyncio.CancelledError` and raises some other error,
+the other error should generally not silently be suppressed.