asyncio: _accept_connection keeps looping after a resource error, causing duplicate _start_serving reschedules

[deadlovelll]

Bug report

Bug description:

In BaseSelectorEventLoop._accept_connection when sock.accept() fails when system is out of file descriptors (EMFILE/ENFILE/ENOBUFS/ENOMEM) code removes the reader and schedules a retry, but does not make return, which generates a lot of unnecessary calls.

# Lib/asyncio/selector_events.py:168
def _accept_connection(
        self, protocol_factory, sock,
        sslcontext=None, server=None, backlog=100,
        ssl_handshake_timeout=constants.SSL_HANDSHAKE_TIMEOUT,
        ssl_shutdown_timeout=constants.SSL_SHUTDOWN_TIMEOUT, context=None):
    for _ in range(backlog + 1):
        try:
            conn, addr = sock.accept()
            if self._debug:
                logger.debug("%r got a new connection from %r: %r",
                                server, addr, conn)
            conn.setblocking(False)
        except ConnectionAbortedError:
            continue
        except (BlockingIOError, InterruptedError):
            return
        except OSError as exc:
            if exc.errno in (errno.EMFILE, errno.ENFILE,
                                errno.ENOBUFS, errno.ENOMEM):
                self.call_exception_handler({
                    'message': 'socket.accept() out of system resource',
                    'exception': exc,
                    'socket': trsock.TransportSocket(sock),
                })
                self._remove_reader(sock.fileno())
                self.call_later(constants.ACCEPT_RETRY_DELAY,
                                self._start_serving,
                                protocol_factory, sock, sslcontext, server,
                                backlog, ssl_handshake_timeout,
                                ssl_shutdown_timeout, context)
            else:
                raise

The branch except (BlockingIOError, InterruptedError) already do early return, so this branch should too

Reproducer:

import errno, asyncio
from unittest import mock

loop = asyncio.new_event_loop()
sock = mock.Mock()
sock.accept.side_effect = OSError(errno.EMFILE, "too many open files")
loop.call_exception_handler = mock.Mock()
loop._remove_reader = mock.Mock()
loop.call_later = mock.Mock()

loop._accept_connection(mock.Mock(), sock, backlog=100)

print("accept calls   :", sock.accept.call_count)
print("retry schedules:", loop.call_later.call_count)

Expected:

accept calls   : 1
retry schedules: 1

Actual:

accept calls   : 101
retry schedules: 101

Proposed fix is to add the return statement :

except OSError as exc:
    if exc.errno in (errno.EMFILE, errno.ENFILE,
                        errno.ENOBUFS, errno.ENOMEM):
        self.call_exception_handler({
            'message': 'socket.accept() out of system resource',
            'exception': exc,
            'socket': trsock.TransportSocket(sock),
        })
        self._remove_reader(sock.fileno())
        self.call_later(constants.ACCEPT_RETRY_DELAY,
                        self._start_serving,
                        protocol_factory, sock, sslcontext, server,
                        backlog, ssl_handshake_timeout,
                        ssl_shutdown_timeout, context)
        return
    else:
        raise 

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

• gh-151615: Avoid repeated reschedule in asyncio _accept_connection #151616