Skip to content

Commit 6bf8315

Browse files
committed
gh-156365: Fix socket leak and stalled accept in the asyncio proactor server
1 parent 223ee6d commit 6bf8315

3 files changed

Lines changed: 73 additions & 11 deletions

File tree

Lib/asyncio/proactor_events.py

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -841,17 +841,31 @@ def loop(f=None):
841841
if self._debug:
842842
logger.debug("%r got a new connection from %r: %r",
843843
server, addr, conn)
844-
protocol = protocol_factory()
845-
if sslcontext is not None:
846-
self._make_ssl_transport(
847-
conn, protocol, sslcontext, server_side=True,
848-
extra={'peername': addr}, server=server,
849-
ssl_handshake_timeout=ssl_handshake_timeout,
850-
ssl_shutdown_timeout=ssl_shutdown_timeout)
851-
else:
852-
self._make_socket_transport(
853-
conn, protocol,
854-
extra={'peername': addr}, server=server)
844+
protocol = None
845+
try:
846+
protocol = protocol_factory()
847+
if sslcontext is not None:
848+
self._make_ssl_transport(
849+
conn, protocol, sslcontext, server_side=True,
850+
extra={'peername': addr}, server=server,
851+
ssl_handshake_timeout=ssl_handshake_timeout,
852+
ssl_shutdown_timeout=ssl_shutdown_timeout)
853+
else:
854+
self._make_socket_transport(
855+
conn, protocol,
856+
extra={'peername': addr}, server=server)
857+
except (SystemExit, KeyboardInterrupt):
858+
raise
859+
except BaseException as exc:
860+
conn.close()
861+
context = {
862+
'message': 'Error on transport creation '
863+
'for incoming connection',
864+
'exception': exc,
865+
}
866+
if protocol is not None:
867+
context['protocol'] = protocol
868+
self.call_exception_handler(context)
855869
if self.is_closed():
856870
return
857871
f = self._proactor.accept(sock)

Lib/test/test_asyncio/test_proactor_events.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,50 @@ def test_create_server(self, m_log):
866866
self.assertTrue(self.sock.close.called)
867867
self.assertTrue(m_log.error.called)
868868

869+
def test_create_server_transport_creation_error(self):
870+
# gh-156365: a protocol_factory() failure closes the accepted socket
871+
# and keeps the server accepting; the listening socket stays open.
872+
pf = mock.Mock(side_effect=RuntimeError)
873+
call_soon = self.loop.call_soon = mock.Mock()
874+
self.loop.call_exception_handler = mock.Mock()
875+
876+
self.loop._start_serving(pf, self.sock)
877+
loop = call_soon.call_args[0][0]
878+
loop()
879+
self.proactor.accept.reset_mock()
880+
881+
conn = mock.Mock()
882+
fut = mock.Mock()
883+
fut.result.return_value = (conn, ('127.0.0.1', 1234))
884+
loop(fut)
885+
886+
self.assertTrue(conn.close.called)
887+
self.assertFalse(self.sock.close.called)
888+
self.assertTrue(self.proactor.accept.called)
889+
self.loop.call_exception_handler.assert_called_once()
890+
891+
def test_create_server_transport_oserror_keeps_listening(self):
892+
# gh-156365: an OSError from transport creation must not close the
893+
# listening socket (mistaken for an accept failure).
894+
pf = mock.Mock()
895+
call_soon = self.loop.call_soon = mock.Mock()
896+
self.loop.call_exception_handler = mock.Mock()
897+
self.loop._make_socket_transport = mock.Mock(side_effect=OSError)
898+
899+
self.loop._start_serving(pf, self.sock)
900+
loop = call_soon.call_args[0][0]
901+
loop()
902+
self.proactor.accept.reset_mock()
903+
904+
conn = mock.Mock()
905+
fut = mock.Mock()
906+
fut.result.return_value = (conn, ('127.0.0.1', 1234))
907+
loop(fut)
908+
909+
self.assertTrue(conn.close.called)
910+
self.assertFalse(self.sock.close.called)
911+
self.assertTrue(self.proactor.accept.called)
912+
869913
def test_create_server_cancel(self):
870914
pf = mock.Mock()
871915
call_soon = self.loop.call_soon = mock.Mock()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a socket leak in :mod:`asyncio` when ``protocol_factory()`` or transport
2+
creation fails for a connection accepted by a proactor-based server (the
3+
default event loop on Windows). The accepted socket is now closed and the
4+
error no longer stops the server from accepting new connections.

0 commit comments

Comments
 (0)