From cca8503dfd2bbcd85d54a4e6e089cfa3dbf1e203 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Sat, 22 Jun 2019 11:38:50 -0400 Subject: [PATCH] *: Modernize IO error handling Where possible, replace use of errno with the exception hierarchy available since python 3.3. Remove explicit handling of EINTR which has been automatic since python 3.5 --- tornado/ioloop.py | 4 +-- tornado/iostream.py | 68 +++++++++++------------------------ tornado/netutil.py | 25 ++++--------- tornado/process.py | 14 ++------ tornado/test/iostream_test.py | 9 +---- 5 files changed, 32 insertions(+), 88 deletions(-) diff --git a/tornado/ioloop.py b/tornado/ioloop.py index 163863f27..a0598727a 100644 --- a/tornado/ioloop.py +++ b/tornado/ioloop.py @@ -101,9 +101,7 @@ class IOLoop(Configurable): while True: try: connection, address = sock.accept() - except socket.error as e: - if e.args[0] not in (errno.EWOULDBLOCK, errno.EAGAIN): - raise + except BlockingIOError: return connection.setblocking(0) io_loop = tornado.ioloop.IOLoop.current() diff --git a/tornado/iostream.py b/tornado/iostream.py index c2f2eb774..23ad0da3d 100644 --- a/tornado/iostream.py +++ b/tornado/iostream.py @@ -64,14 +64,6 @@ try: except ImportError: _set_nonblocking = None # type: ignore -# These errnos indicate that a non-blocking operation must be retried -# at a later time. On most platforms they're the same value, but on -# some they differ. -_ERRNO_WOULDBLOCK = (errno.EWOULDBLOCK, errno.EAGAIN) - -if hasattr(errno, "WSAEWOULDBLOCK"): - _ERRNO_WOULDBLOCK += (errno.WSAEWOULDBLOCK,) # type: ignore - # These errnos indicate that a connection has been abruptly terminated. # They should be caught and handled less noisily than other errors. _ERRNO_CONNRESET = (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE, errno.ETIMEDOUT) @@ -91,12 +83,6 @@ if sys.platform == "darwin": # instead of an unexpected error. _ERRNO_CONNRESET += (errno.EPROTOTYPE,) # type: ignore -# More non-portable errnos: -_ERRNO_INPROGRESS = (errno.EINPROGRESS,) - -if hasattr(errno, "WSAEINPROGRESS"): - _ERRNO_INPROGRESS += (errno.WSAEINPROGRESS,) # type: ignore - _WINDOWS = sys.platform.startswith("win") @@ -859,8 +845,6 @@ class BaseIOStream(object): buf = bytearray(self.read_chunk_size) bytes_read = self.read_from_fd(buf) except (socket.error, IOError, OSError) as e: - if errno_from_exception(e) == errno.EINTR: - continue # ssl.SSLError is a subclass of socket.error if self._is_connreset(e): # Treat ECONNRESET as a connection close rather than @@ -968,17 +952,16 @@ class BaseIOStream(object): break self._write_buffer.advance(num_bytes) self._total_write_done_index += num_bytes + except BlockingIOError: + break except (socket.error, IOError, OSError) as e: - if e.args[0] in _ERRNO_WOULDBLOCK: - break - else: - if not self._is_connreset(e): - # Broken pipe errors are usually caused by connection - # reset, and its better to not log EPIPE errors to - # minimize log spam - gen_log.warning("Write error on %s: %s", self.fileno(), e) - self.close(exc_info=e) - return + if not self._is_connreset(e): + # Broken pipe errors are usually caused by connection + # reset, and its better to not log EPIPE errors to + # minimize log spam + gen_log.warning("Write error on %s: %s", self.fileno(), e) + self.close(exc_info=e) + return while self._write_futures: index, future = self._write_futures[0] @@ -1134,11 +1117,8 @@ class IOStream(BaseIOStream): def read_from_fd(self, buf: Union[bytearray, memoryview]) -> Optional[int]: try: return self.socket.recv_into(buf, len(buf)) - except socket.error as e: - if e.args[0] in _ERRNO_WOULDBLOCK: - return None - else: - raise + except BlockingIOError: + return None finally: del buf @@ -1202,24 +1182,19 @@ class IOStream(BaseIOStream): self._connect_future = typing.cast("Future[IOStream]", future) try: self.socket.connect(address) - except socket.error as e: + except BlockingIOError: # In non-blocking mode we expect connect() to raise an # exception with EINPROGRESS or EWOULDBLOCK. - # + pass + except socket.error as e: # On freebsd, other errors such as ECONNREFUSED may be # returned immediately when attempting to connect to # localhost, so handle them the same way as an error # reported later in _handle_connect. - if ( - errno_from_exception(e) not in _ERRNO_INPROGRESS - and errno_from_exception(e) not in _ERRNO_WOULDBLOCK - ): - if future is None: - gen_log.warning( - "Connect error on fd %s: %s", self.socket.fileno(), e - ) - self.close(exc_info=e) - return future + if future is None: + gen_log.warning("Connect error on fd %s: %s", self.socket.fileno(), e) + self.close(exc_info=e) + return future self._add_io_state(self.io_loop.WRITE) return future @@ -1595,11 +1570,8 @@ class SSLIOStream(IOStream): return None else: raise - except socket.error as e: - if e.args[0] in _ERRNO_WOULDBLOCK: - return None - else: - raise + except BlockingIOError: + return None finally: del buf diff --git a/tornado/netutil.py b/tornado/netutil.py index 54217df1c..293f3aab5 100644 --- a/tornado/netutil.py +++ b/tornado/netutil.py @@ -49,14 +49,6 @@ u"foo".encode("idna") # For undiagnosed reasons, 'latin1' codec may also need to be preloaded. u"foo".encode("latin1") -# These errnos indicate that a non-blocking operation must be retried -# at a later time. On most platforms they're the same value, but on -# some they differ. -_ERRNO_WOULDBLOCK = (errno.EWOULDBLOCK, errno.EAGAIN) - -if hasattr(errno, "WSAEWOULDBLOCK"): - _ERRNO_WOULDBLOCK += (errno.WSAEWOULDBLOCK,) # type: ignore - # Default backlog used when calling sock.listen() _DEFAULT_BACKLOG = 128 @@ -199,9 +191,8 @@ if hasattr(socket, "AF_UNIX"): sock.setblocking(False) try: st = os.stat(file) - except OSError as err: - if errno_from_exception(err) != errno.ENOENT: - raise + except FileNotFoundError: + pass else: if stat.S_ISSOCK(st.st_mode): os.remove(file) @@ -254,17 +245,15 @@ def add_accept_handler( return try: connection, address = sock.accept() - except socket.error as e: - # _ERRNO_WOULDBLOCK indicate we have accepted every + except BlockingIOError: + # EWOULDBLOCK indicates we have accepted every # connection that is available. - if errno_from_exception(e) in _ERRNO_WOULDBLOCK: - return + return + except ConnectionAbortedError: # ECONNABORTED indicates that there was a connection # but it was closed while still in the accept queue. # (observed on FreeBSD). - if errno_from_exception(e) == errno.ECONNABORTED: - continue - raise + continue set_close_exec(connection.fileno()) callback(connection, address) diff --git a/tornado/process.py b/tornado/process.py index ed3e15115..9e4c194af 100644 --- a/tornado/process.py +++ b/tornado/process.py @@ -17,7 +17,6 @@ the server into multiple processes and managing subprocesses. """ -import errno import os import multiprocessing import signal @@ -36,7 +35,6 @@ from tornado import ioloop from tornado.iostream import PipeIOStream from tornado.log import gen_log from tornado.platform.auto import set_close_exec -from tornado.util import errno_from_exception import typing from typing import Tuple, Optional, Any, Callable @@ -148,12 +146,7 @@ def fork_processes( return id num_restarts = 0 while children: - try: - pid, status = os.wait() - except OSError as e: - if errno_from_exception(e) == errno.EINTR: - continue - raise + pid, status = os.wait() if pid not in children: continue id = children.pop(pid) @@ -358,9 +351,8 @@ class Subprocess(object): def _try_cleanup_process(cls, pid: int) -> None: try: ret_pid, status = os.waitpid(pid, os.WNOHANG) - except OSError as e: - if errno_from_exception(e) == errno.ECHILD: - return + except ChildProcessError: + return if ret_pid == 0: return assert ret_pid == pid diff --git a/tornado/test/iostream_test.py b/tornado/test/iostream_test.py index 295cdd58d..304146ce9 100644 --- a/tornado/test/iostream_test.py +++ b/tornado/test/iostream_test.py @@ -30,7 +30,6 @@ import platform import random import socket import ssl -import sys from unittest import mock import unittest @@ -694,13 +693,7 @@ class TestIOStreamMixin(TestReadWriteMixin): with self.assertRaises(StreamClosedError): yield stream.connect(("127.0.0.1", port)) - self.assertTrue(isinstance(stream.error, socket.error), stream.error) - if sys.platform != "cygwin": - _ERRNO_CONNREFUSED = [errno.ECONNREFUSED] - if hasattr(errno, "WSAECONNREFUSED"): - _ERRNO_CONNREFUSED.append(errno.WSAECONNREFUSED) # type: ignore - # cygwin's errnos don't match those used on native windows python - self.assertTrue(stream.error.args[0] in _ERRNO_CONNREFUSED) # type: ignore + self.assertTrue(isinstance(stream.error, ConnectionRefusedError), stream.error) @gen_test def test_gaierror(self): -- 2.47.2