Skip to content

bpo-32528: Make asyncio.CancelledError a BaseException. #13528

New issue

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

Merged
merged 2 commits into from
May 27, 2019
Merged

Conversation

1st1
Copy link
Member

@1st1 1st1 commented May 23, 2019

This will address the common mistake many asyncio users make:
an "except Exception" clause breaking Tasks cancellation.

In addition to this change, we stop inheriting asyncio.TimeoutError
and asyncio.InvalidStateError from their concurrent.futures.*
counterparts. There's no point for these exceptions to share the
inheritance chain.

https://bugs.python.org/issue32528

@1st1 1st1 requested a review from asvetlov May 23, 2019 21:38
@1st1 1st1 requested a review from gpshead as a code owner May 23, 2019 21:38
@1st1 1st1 removed the request for review from gpshead May 23, 2019 21:38
@asvetlov
Copy link
Contributor

I see a small problem with the PR in test_base_events.py:

    def test_run_until_complete_loop_orphan_future_close_loop(self):
        class ShowStopper(BaseException):
            pass

        async def foo(delay):
            await asyncio.sleep(delay)

        def throw():
            raise ShowStopper

        self.loop._process_events = mock.Mock()
        self.loop.call_soon(throw)
        try:
            self.loop.run_until_complete(foo(0.1))
        except ShowStopper:
            pass

        # This call fails if run_until_complete does not clean up
        # done-callback for the previous future.
        self.loop.run_until_complete(foo(0.2))

The test passes but a message is printed:

test_run_until_complete_loop_orphan_future_close_loop (test.test_asyncio.test_base_events.BaseEventLoopTests) ... Exception in callback BaseEventLoopTests.test_run_until_complete_loop_orphan_future_close_loop.<locals>.throw() at /home/andrew/projects/cpython/Lib/test/test_asyncio/test_base_events.py:485
handle: <Handle BaseEventLoopTests.test_run_until_complete_loop_orphan_future_close_loop.<locals>.throw() at /home/andrew/projects/cpython/Lib/test/test_asyncio/test_base_events.py:485>
Traceback (most recent call last):
  File "/home/andrew/projects/cpython/Lib/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "/home/andrew/projects/cpython/Lib/test/test_asyncio/test_base_events.py", line 486, in throw
    raise ShowStopper
test.test_asyncio.test_base_events.BaseEventLoopTests.test_run_until_complete_loop_orphan_future_close_loop.<locals>.ShowStopper
ok

I suggest a little different implementation.
On catching exceptions let's don't change except Exception to except BaseException but replace it with except (Exception, asyncio.CancelledError).

By this, we keep exception handling logic in asyncio unchanged but put CacelledError at the proper place in exceptions hierarchy.
I don't know if users use custom BaseException derived classes in their code. This PR has a potential regression risk, while my proposal eliminates it (I hope).

Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After offline talk @1st1 convinced me that we should process all exceptions as proposed by the PR.
Eventually in Python 3.9 we will get rid of custom handling of SystemExit and KeyboardInterrupt.

1st1 added 2 commits May 27, 2019 14:23
This will address the common mistake many asyncio users make:
an "except Exception" clause breaking Tasks cancellation.

In addition to this change, we stop inheriting asyncio.TimeoutError
and asyncio.InvalidStateError from their concurrent.futures.*
counterparts.  There's no point for these exceptions to share the
inheritance chain.
@1st1 1st1 merged commit 431b540 into python:master May 27, 2019
@1st1 1st1 deleted the exc branch May 27, 2019 12:45
bdarnell added a commit to bdarnell/tornado that referenced this pull request Jun 22, 2019
python/cpython#13528 broke us in two ways: asyncio.CancelledError is
no longer an alias for concurrent.futures.CancelledError and it's now
a BaseException.

Fixes tornadoweb#2677
Closes tornadoweb#2681
achimnol added a commit to achimnol/aiotools that referenced this pull request Aug 28, 2019
* See also: https://bugs.python.org/issue32528

* This change makes actxmgr cancellation behaviors in Python 3.6/3.7/3.8 consistent.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
This will address the common mistake many asyncio users make:
an "except Exception" clause breaking Tasks cancellation.

In addition to this change, we stop inheriting asyncio.TimeoutError
and asyncio.InvalidStateError from their concurrent.futures.*
counterparts.  There's no point for these exceptions to share the
inheritance chain.

In 3.9 we'll focus on implementing supervisors and cancel scopes,
which should allow better handling of all exceptions, including
SystemExit and KeyboardInterrupt
miss-islington pushed a commit that referenced this pull request Jul 14, 2020
…or (GH-21474)

#msg373510

[bpo-32528]()/#13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 14, 2020
…or (pythonGH-21474)

GH-msg373510

[bpo-32528]()/pythonGH-13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
(cherry picked from commit 2a51818)

Co-authored-by: JustAnotherArchivist <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 14, 2020
…or (pythonGH-21474)

GH-msg373510

[bpo-32528]()/pythonGH-13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
(cherry picked from commit 2a51818)

Co-authored-by: JustAnotherArchivist <[email protected]>
miss-islington added a commit that referenced this pull request Jul 19, 2020
…or (GH-21474)

GH-msg373510

[bpo-32528]()/GH-13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
(cherry picked from commit 2a51818)

Co-authored-by: JustAnotherArchivist <[email protected]>
arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
…or (pythonGH-21474)

#msg373510

[bpo-32528]()/python#13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 24, 2020
…or (pythonGH-21474)

GH-msg373510

[bpo-32528]()/pythonGH-13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
(cherry picked from commit 2a51818)

Co-authored-by: JustAnotherArchivist <[email protected]>
miss-islington added a commit that referenced this pull request Jul 24, 2020
…or (GH-21474)

GH-msg373510

[bpo-32528]()/GH-13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
(cherry picked from commit 2a51818)

Co-authored-by: JustAnotherArchivist <[email protected]>
hoffmang9 pushed a commit to Chia-Network/chia-blockchain that referenced this pull request Jul 27, 2020
* fix for 3.8 python pull 13528 - python/cpython#13528
* changes
* flake8
hoffmang9 pushed a commit to Chia-Network/chia-blockchain that referenced this pull request Jul 27, 2020
* fix for 3.8 python pull 13528 - python/cpython#13528
* changes
* flake8
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Aug 4, 2020
…or (pythonGH-21474)

#msg373510

[bpo-32528]()/python#13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Aug 20, 2020
…or (pythonGH-21474)

#msg373510

[bpo-32528]()/python#13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
…or (pythonGH-21474)

#msg373510

[bpo-32528]()/python#13528 changed `asyncio.CancelledError` such that it no longer inherits from `concurrent.futures.CancelledError`. As this affects existing code, specifically when catching the latter instead of the former in exception handling, it should be documented in the "What's new in 3.8?" document.

Automerge-Triggered-By: @1st1
vxgmichel referenced this pull request in vxgmichel/aioconsole Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants