bpo-32528: Make asyncio.CancelledError a BaseException. (GH-13528)

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
This commit is contained in:
Yury Selivanov 2019-05-27 14:45:12 +02:00 committed by GitHub
parent 16cefb0bc7
commit 431b540bf7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 147 additions and 67 deletions

View File

@ -186,7 +186,7 @@ def _interleave_addrinfos(addrinfos, first_address_family_count=1):
def _run_until_complete_cb(fut):
if not fut.cancelled():
exc = fut.exception()
if isinstance(exc, BaseException) and not isinstance(exc, Exception):
if isinstance(exc, (SystemExit, KeyboardInterrupt)):
# Issue #22429: run_forever() already finished, no need to
# stop it.
return
@ -1196,7 +1196,7 @@ class BaseEventLoop(events.AbstractEventLoop):
try:
await waiter
except Exception:
except BaseException:
transport.close()
conmade_cb.cancel()
resume_cb.cancel()
@ -1710,7 +1710,9 @@ class BaseEventLoop(events.AbstractEventLoop):
if self._exception_handler is None:
try:
self.default_exception_handler(context)
except Exception:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException:
# Second protection layer for unexpected errors
# in the default implementation, as well as for subclassed
# event loops with overloaded "default_exception_handler".
@ -1719,7 +1721,9 @@ class BaseEventLoop(events.AbstractEventLoop):
else:
try:
self._exception_handler(self, context)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
# Exception in the user set custom exception handler.
try:
# Let's try default handler.
@ -1728,7 +1732,9 @@ class BaseEventLoop(events.AbstractEventLoop):
'exception': exc,
'context': context,
})
except Exception:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException:
# Guard 'default_exception_handler' in case it is
# overloaded.
logger.error('Exception in default exception handler '

View File

@ -182,7 +182,9 @@ class BaseSubprocessTransport(transports.SubprocessTransport):
for callback, data in self._pending_calls:
loop.call_soon(callback, *data)
self._pending_calls = None
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
if waiter is not None and not waiter.cancelled():
waiter.set_exception(exc)
else:

View File

@ -79,7 +79,9 @@ class Handle:
def _run(self):
try:
self._context.run(self._callback, *self._args)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
cb = format_helpers._format_callback_source(
self._callback, self._args)
msg = f'Exception in callback {cb}'

View File

@ -5,19 +5,16 @@ __all__ = ('CancelledError', 'InvalidStateError', 'TimeoutError',
'IncompleteReadError', 'LimitOverrunError',
'SendfileNotAvailableError')
import concurrent.futures
from . import base_futures
class CancelledError(concurrent.futures.CancelledError):
class CancelledError(BaseException):
"""The Future or Task was cancelled."""
class TimeoutError(concurrent.futures.TimeoutError):
class TimeoutError(Exception):
"""The operation exceeded the given deadline."""
class InvalidStateError(concurrent.futures.InvalidStateError):
class InvalidStateError(Exception):
"""The operation is not allowed in this state."""

View File

@ -212,7 +212,9 @@ class _ProactorReadPipeTransport(_ProactorBasePipeTransport,
try:
keep_open = self._protocol.eof_received()
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal error: protocol.eof_received() call failed.')
return
@ -235,7 +237,9 @@ class _ProactorReadPipeTransport(_ProactorBasePipeTransport,
if isinstance(self._protocol, protocols.BufferedProtocol):
try:
protocols._feed_data_to_buffered_proto(self._protocol, data)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(exc,
'Fatal error: protocol.buffer_updated() '
'call failed.')
@ -625,7 +629,9 @@ class BaseProactorEventLoop(base_events.BaseEventLoop):
except exceptions.CancelledError:
# _close_self_pipe() has been called, stop waiting for data
return
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self.call_exception_handler({
'message': 'Error on reading from the event loop self pipe',
'exception': exc,

View File

@ -208,12 +208,14 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
try:
await waiter
except:
except BaseException:
transport.close()
raise
# It's now up to the protocol to handle the connection.
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
if self._debug:
context = {
'message':
@ -370,7 +372,9 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
data = sock.recv(n)
except (BlockingIOError, InterruptedError):
return # try again next time
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
fut.set_exception(exc)
else:
fut.set_result(data)
@ -404,7 +408,9 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
nbytes = sock.recv_into(buf)
except (BlockingIOError, InterruptedError):
return # try again next time
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
fut.set_exception(exc)
else:
fut.set_result(nbytes)
@ -447,7 +453,9 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
n = sock.send(view[start:])
except (BlockingIOError, InterruptedError):
return
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
fut.set_exception(exc)
return
@ -487,7 +495,9 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
fut.add_done_callback(
functools.partial(self._sock_write_done, fd))
self.add_writer(fd, self._sock_connect_cb, fut, sock, address)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
fut.set_exception(exc)
else:
fut.set_result(None)
@ -507,7 +517,9 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
except (BlockingIOError, InterruptedError):
# socket is still registered, the callback will be retried later
pass
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
fut.set_exception(exc)
else:
fut.set_result(None)
@ -537,7 +549,9 @@ class BaseSelectorEventLoop(base_events.BaseEventLoop):
conn.setblocking(False)
except (BlockingIOError, InterruptedError):
self.add_reader(fd, self._sock_accept, fut, True, sock)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
fut.set_exception(exc)
else:
fut.set_result((conn, address))
@ -785,7 +799,9 @@ class _SelectorSocketTransport(_SelectorTransport):
buf = self._protocol.get_buffer(-1)
if not len(buf):
raise RuntimeError('get_buffer() returned an empty buffer')
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal error: protocol.get_buffer() call failed.')
return
@ -794,7 +810,9 @@ class _SelectorSocketTransport(_SelectorTransport):
nbytes = self._sock.recv_into(buf)
except (BlockingIOError, InterruptedError):
return
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(exc, 'Fatal read error on socket transport')
return
@ -804,7 +822,9 @@ class _SelectorSocketTransport(_SelectorTransport):
try:
self._protocol.buffer_updated(nbytes)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal error: protocol.buffer_updated() call failed.')
@ -815,7 +835,9 @@ class _SelectorSocketTransport(_SelectorTransport):
data = self._sock.recv(self.max_size)
except (BlockingIOError, InterruptedError):
return
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(exc, 'Fatal read error on socket transport')
return
@ -825,7 +847,9 @@ class _SelectorSocketTransport(_SelectorTransport):
try:
self._protocol.data_received(data)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal error: protocol.data_received() call failed.')
@ -835,7 +859,9 @@ class _SelectorSocketTransport(_SelectorTransport):
try:
keep_open = self._protocol.eof_received()
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal error: protocol.eof_received() call failed.')
return
@ -871,7 +897,9 @@ class _SelectorSocketTransport(_SelectorTransport):
n = self._sock.send(data)
except (BlockingIOError, InterruptedError):
pass
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(exc, 'Fatal write error on socket transport')
return
else:
@ -894,7 +922,9 @@ class _SelectorSocketTransport(_SelectorTransport):
n = self._sock.send(self._buffer)
except (BlockingIOError, InterruptedError):
pass
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._loop._remove_writer(self._sock_fd)
self._buffer.clear()
self._fatal_error(exc, 'Fatal write error on socket transport')
@ -970,7 +1000,9 @@ class _SelectorDatagramTransport(_SelectorTransport):
pass
except OSError as exc:
self._protocol.error_received(exc)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(exc, 'Fatal read error on datagram transport')
else:
self._protocol.datagram_received(data, addr)
@ -1007,7 +1039,9 @@ class _SelectorDatagramTransport(_SelectorTransport):
except OSError as exc:
self._protocol.error_received(exc)
return
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal write error on datagram transport')
return
@ -1030,7 +1064,9 @@ class _SelectorDatagramTransport(_SelectorTransport):
except OSError as exc:
self._protocol.error_received(exc)
return
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._fatal_error(
exc, 'Fatal write error on datagram transport')
return

View File

@ -527,7 +527,9 @@ class SSLProtocol(protocols.Protocol):
try:
ssldata, appdata = self._sslpipe.feed_ssldata(data)
except Exception as e:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as e:
self._fatal_error(e, 'SSL error in data received')
return
@ -542,7 +544,9 @@ class SSLProtocol(protocols.Protocol):
self._app_protocol, chunk)
else:
self._app_protocol.data_received(chunk)
except Exception as ex:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as ex:
self._fatal_error(
ex, 'application protocol failed to receive SSL data')
return
@ -628,7 +632,9 @@ class SSLProtocol(protocols.Protocol):
raise handshake_exc
peercert = sslobj.getpeercert()
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
if isinstance(exc, ssl.CertificateError):
msg = 'SSL handshake failed on verifying the certificate'
else:
@ -691,7 +697,9 @@ class SSLProtocol(protocols.Protocol):
# delete it and reduce the outstanding buffer size.
del self._write_backlog[0]
self._write_buffer_size -= len(data)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
if self._in_handshake:
# Exceptions will be re-raised in _on_handshake_complete.
self._on_handshake_complete(exc)

View File

@ -105,7 +105,9 @@ async def staggered_race(
try:
result = await coro_fn()
except Exception as e:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as e:
exceptions[this_index] = e
this_failed.set() # Kickstart the next coroutine
else:

View File

@ -260,11 +260,11 @@ class Task(futures._PyFuture): # Inherit Python Task implementation
super().set_result(exc.value)
except exceptions.CancelledError:
super().cancel() # I.e., Future.cancel(self).
except Exception as exc:
super().set_exception(exc)
except BaseException as exc:
except (KeyboardInterrupt, SystemExit) as exc:
super().set_exception(exc)
raise
except BaseException as exc:
super().set_exception(exc)
else:
blocking = getattr(result, '_asyncio_future_blocking', None)
if blocking is not None:
@ -318,7 +318,7 @@ class Task(futures._PyFuture): # Inherit Python Task implementation
def __wakeup(self, future):
try:
future.result()
except Exception as exc:
except BaseException as exc:
# This may also be a cancellation.
self.__step(exc)
else:
@ -858,7 +858,9 @@ def run_coroutine_threadsafe(coro, loop):
def callback():
try:
futures._chain_future(ensure_future(coro, loop=loop), future)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
if future.set_running_or_notify_cancel():
future.set_exception(exc)
raise

View File

@ -262,7 +262,9 @@ class _FlowControlMixin(Transport):
self._protocol_paused = True
try:
self._protocol.pause_writing()
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._loop.call_exception_handler({
'message': 'protocol.pause_writing() failed',
'exception': exc,
@ -276,7 +278,9 @@ class _FlowControlMixin(Transport):
self._protocol_paused = False
try:
self._protocol.resume_writing()
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._loop.call_exception_handler({
'message': 'protocol.resume_writing() failed',
'exception': exc,

View File

@ -194,7 +194,9 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop):
self._child_watcher_callback, transp)
try:
await waiter
except Exception:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException:
transp.close()
await transp._wait()
raise
@ -390,7 +392,9 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop):
else:
self._sock_sendfile_update_filepos(fileno, offset, total_sent)
fut.set_exception(exc)
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._sock_sendfile_update_filepos(fileno, offset, total_sent)
fut.set_exception(exc)
else:
@ -641,7 +645,9 @@ class _UnixWritePipeTransport(transports._FlowControlMixin,
n = os.write(self._fileno, data)
except (BlockingIOError, InterruptedError):
n = 0
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._conn_lost += 1
self._fatal_error(exc, 'Fatal write error on pipe transport')
return
@ -661,7 +667,9 @@ class _UnixWritePipeTransport(transports._FlowControlMixin,
n = os.write(self._fileno, self._buffer)
except (BlockingIOError, InterruptedError):
pass
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
self._buffer.clear()
self._conn_lost += 1
# Remove writer here, _fatal_error() doesn't it
@ -879,7 +887,9 @@ class BaseChildWatcher(AbstractChildWatcher):
def _sig_chld(self):
try:
self._do_waitpid_all()
except Exception as exc:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException as exc:
# self._loop should always be available here
# as '_sig_chld' is added as a signal handler
# in 'attach_loop'

View File

@ -388,7 +388,9 @@ class ProactorEventLoop(proactor_events.BaseProactorEventLoop):
**kwargs)
try:
await waiter
except Exception:
except (SystemExit, KeyboardInterrupt):
raise
except BaseException:
transp.close()
await transp._wait()
raise

View File

@ -476,7 +476,7 @@ class BaseEventLoopTests(test_utils.TestCase):
other_loop.run_until_complete, task)
def test_run_until_complete_loop_orphan_future_close_loop(self):
class ShowStopper(BaseException):
class ShowStopper(SystemExit):
pass
async def foo(delay):
@ -487,10 +487,8 @@ class BaseEventLoopTests(test_utils.TestCase):
self.loop._process_events = mock.Mock()
self.loop.call_soon(throw)
try:
with self.assertRaises(ShowStopper):
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.

View File

@ -1527,7 +1527,7 @@ class BaseTaskTests:
async def sleeper():
await asyncio.sleep(10)
base_exc = BaseException()
base_exc = SystemExit()
async def notmutch():
try:
@ -1541,7 +1541,7 @@ class BaseTaskTests:
task.cancel()
self.assertFalse(task.done())
self.assertRaises(BaseException, test_utils.run_briefly, loop)
self.assertRaises(SystemExit, test_utils.run_briefly, loop)
self.assertTrue(task.done())
self.assertFalse(task.cancelled())

View File

@ -0,0 +1,8 @@
Make asyncio.CancelledError a BaseException.
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.

View File

@ -2672,8 +2672,10 @@ set_exception:
assert(o == Py_None);
Py_DECREF(o);
if (!PyErr_GivenExceptionMatches(et, PyExc_Exception)) {
/* We've got a BaseException; re-raise it */
if (PyErr_GivenExceptionMatches(et, PyExc_KeyboardInterrupt) ||
PyErr_GivenExceptionMatches(et, PyExc_SystemExit))
{
/* We've got a KeyboardInterrupt or a SystemError; re-raise it */
PyErr_Restore(et, ev, tb);
goto fail;
}
@ -2950,11 +2952,6 @@ task_wakeup(TaskObj *task, PyObject *o)
}
PyErr_Fetch(&et, &ev, &tb);
if (!PyErr_GivenExceptionMatches(et, PyExc_Exception)) {
/* We've got a BaseException; re-raise it */
PyErr_Restore(et, ev, tb);
return NULL;
}
if (!ev || !PyObject_TypeCheck(ev, (PyTypeObject *) et)) {
PyErr_NormalizeException(&et, &ev, &tb);
}