From fb9cb76aacc93800bc31d578f04596bd97e238c6 Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Fri, 2 Oct 2026 10:07:41 +0530 Subject: [PATCH] gh-156321: Don't eagerly log exceptions from a cancelled asyncio.shield() When the outer future returned by shield() was cancelled, a callback was added to the inner future that reported its exception through the loop exception handler as soon as it completed, even if the exception was retrieved afterwards by awaiting the inner future or calling `exception()`. Remove the callback; an exception nobody retrieves is still reported by the inner future itself when it is garbage collected, like any other future. --- Lib/asyncio/tasks.py | 22 ------------ Lib/test/test_asyncio/test_tasks.py | 34 ++++++++++++++++--- ...-10-01-16-45-00.gh-issue-156321.Kq3xVt.rst | 5 +++ 3 files changed, 35 insertions(+), 26 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-10-01-16-45-00.gh-issue-156321.Kq3xVt.rst diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index cf4787db1730597..979f8d2f83eb1eb 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -938,25 +938,6 @@ def _done_callback(fut, cur_task=cur_task): return outer -def _log_on_exception(fut): - if fut.cancelled(): - return - - exc = fut.exception() - if exc is None: - return - - context = { - 'message': - f'{exc.__class__.__name__} exception in shielded future', - 'exception': exc, - 'future': fut, - } - if fut._source_traceback: - context['source_traceback'] = fut._source_traceback - fut._loop.call_exception_handler(context) - - def shield(arg): """Wait for a future, shielding it from cancellation. @@ -1021,9 +1002,6 @@ def _inner_done_callback(inner): def _outer_done_callback(outer): if not inner.done(): inner.remove_done_callback(_inner_done_callback) - # Keep only one callback to log on cancel - inner.remove_done_callback(_log_on_exception) - inner.add_done_callback(_log_on_exception) if cur_task is not None: inner.remove_done_callback(_clear_awaited_by_callback) futures.future_discard_from_awaited_by(inner, cur_task) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 570810a231b48d2..c380dcfec213951 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -2153,7 +2153,7 @@ def test_shield_cancel_outer(self): self.assertTrue(outer.cancelled()) self.assertEqual(0, 0 if outer._callbacks is None else len(outer._callbacks)) self.assertFalse(inner._asyncio_awaited_by) - self.assertTrue({f for f, _ctx in inner._callbacks or []} <= {asyncio.tasks._log_on_exception}) + self.assertFalse(inner._callbacks) def test_shield_cancel_outer_result(self): mock_handler = mock.Mock() @@ -2168,6 +2168,8 @@ def test_shield_cancel_outer_result(self): mock_handler.assert_not_called() def test_shield_cancel_outer_exception(self): + # gh-156321: an exception in the inner future must not be reported + # eagerly, as it may still be retrieved later. mock_handler = mock.Mock() self.loop.set_exception_handler(mock_handler) inner = self.new_future(self.loop) @@ -2177,7 +2179,29 @@ def test_shield_cancel_outer_exception(self): test_utils.run_briefly(self.loop) inner.set_exception(Exception('foo')) test_utils.run_briefly(self.loop) + mock_handler.assert_not_called() + self.assertIsInstance(inner.exception(), Exception) + + def test_shield_cancel_outer_exception_never_retrieved(self): + # gh-156321: an exception nobody retrieves is reported by the inner + # future itself when it is garbage collected, like any other future. + mock_handler = mock.Mock() + self.loop.set_exception_handler(mock_handler) + inner = self.new_future(self.loop) + outer = asyncio.shield(inner) + test_utils.run_briefly(self.loop) + outer.cancel() + test_utils.run_briefly(self.loop) + inner.set_exception(Exception('foo')) + test_utils.run_briefly(self.loop) + mock_handler.assert_not_called() + inner = None + outer = None + support.gc_collect() mock_handler.assert_called_once() + context = mock_handler.call_args[0][1] + self.assertEndsWith(context['message'], 'exception was never retrieved') + self.assertIsInstance(context['exception'], Exception) def test_shield_cancel_outer_in_task(self): inner = self.new_future(self.loop) @@ -2192,9 +2216,9 @@ async def coro(): task = self.new_task(self.loop, coro()) self.loop.run_until_complete(task) self.assertFalse(inner._asyncio_awaited_by) - self.assertTrue({f for f, _ctx in inner._callbacks or []} <= {asyncio.tasks._log_on_exception}) + self.assertFalse(inner._callbacks) - def test_shield_duplicate_log_once(self): + def test_shield_cancel_outer_twice_exception(self): mock_handler = mock.Mock() self.loop.set_exception_handler(mock_handler) inner = self.new_future(self.loop) @@ -2206,9 +2230,11 @@ def test_shield_duplicate_log_once(self): test_utils.run_briefly(self.loop) outer.cancel() test_utils.run_briefly(self.loop) + self.assertFalse(inner._callbacks) inner.set_exception(Exception('foo')) test_utils.run_briefly(self.loop) - mock_handler.assert_called_once() + mock_handler.assert_not_called() + self.assertIsInstance(inner.exception(), Exception) def test_shield_shortcut(self): fut = self.new_future(self.loop) diff --git a/Misc/NEWS.d/next/Library/2026-10-01-16-45-00.gh-issue-156321.Kq3xVt.rst b/Misc/NEWS.d/next/Library/2026-10-01-16-45-00.gh-issue-156321.Kq3xVt.rst new file mode 100644 index 000000000000000..2101e9a899e5073 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-01-16-45-00.gh-issue-156321.Kq3xVt.rst @@ -0,0 +1,5 @@ +Fix :func:`asyncio.shield` reporting an exception from the inner future +through the loop exception handler as soon as it completes after the shield +was cancelled, even when the exception is retrieved afterwards. The +exception is now reported only if it is never retrieved, as with any other +future.