Skip to content

gh-135836: Fix IndexError in asyncio.create_connection() #135875

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 32 additions & 30 deletions Lib/asyncio/base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -1016,38 +1016,40 @@ async def _connect_sock(self, exceptions, addr_info, local_addr_infos=None):
family, type_, proto, _, address = addr_info
sock = None
try:
sock = socket.socket(family=family, type=type_, proto=proto)
sock.setblocking(False)
if local_addr_infos is not None:
for lfamily, _, _, _, laddr in local_addr_infos:
# skip local addresses of different family
if lfamily != family:
continue
try:
sock.bind(laddr)
break
except OSError as exc:
msg = (
f'error while attempting to bind on '
f'address {laddr!r}: {str(exc).lower()}'
)
exc = OSError(exc.errno, msg)
my_exceptions.append(exc)
else: # all bind attempts failed
if my_exceptions:
raise my_exceptions.pop()
else:
raise OSError(f"no matching local address with {family=} found")
await self.sock_connect(sock, address)
return sock
except OSError as exc:
my_exceptions.append(exc)
if sock is not None:
sock.close()
raise
try:
sock = socket.socket(family=family, type=type_, proto=proto)
sock.setblocking(False)
if local_addr_infos is not None:
for lfamily, _, _, _, laddr in local_addr_infos:
# skip local addresses of different family
if lfamily != family:
continue
try:
sock.bind(laddr)
break
except OSError as exc:
msg = (
f'error while attempting to bind on '
f'address {laddr!r}: {str(exc).lower()}'
)
exc = OSError(exc.errno, msg)
my_exceptions.append(exc)
else: # all bind attempts failed
if my_exceptions:
raise my_exceptions.pop()
else:
raise OSError(f"no matching local address with {family=} found")
await self.sock_connect(sock, address)
return sock
except OSError as exc:
my_exceptions.append(exc)
raise
except:
if sock is not None:
sock.close()
try:
sock.close()
except OSError:
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if suppressing this error is correct here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is okay. It is an error in closing just created connection, it is less interesting than the original non-OSError error.

We do not even know what is this error, because we have no reproducer. ECONNRESET is already ignored in socket.close(). I suspect that this is similar error (maybe EINTR?) which should always be ignored.

pass
raise
finally:
exceptions = my_exceptions = None
Expand Down
29 changes: 29 additions & 0 deletions Lib/test/test_asyncio/test_base_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
MOCK_ANY = mock.ANY


class CustomError(Exception):
pass


def tearDownModule():
asyncio._set_event_loop_policy(None)

Expand Down Expand Up @@ -1296,6 +1300,31 @@ def getaddrinfo_task(*args, **kwds):
self.assertEqual(len(cm.exception.exceptions), 1)
self.assertIsInstance(cm.exception.exceptions[0], OSError)

@patch_socket
def test_create_connection_connect_non_os_err_close_err(self, m_socket):
# Test the case when sock_connect() raises non-OSError exception
# and sock.close() raises OSError.
async def getaddrinfo(*args, **kw):
return [(2, 1, 6, '', ('107.6.106.82', 80))]

def getaddrinfo_task(*args, **kwds):
return self.loop.create_task(getaddrinfo(*args, **kwds))

self.loop.getaddrinfo = getaddrinfo_task
self.loop.sock_connect = mock.Mock()
self.loop.sock_connect.side_effect = CustomError
sock = mock.Mock()
m_socket.socket.return_value = sock
sock.close.side_effect = OSError

coro = self.loop.create_connection(MyProto, 'example.com', 80)
self.assertRaises(
CustomError, self.loop.run_until_complete, coro)

coro = self.loop.create_connection(MyProto, 'example.com', 80, all_errors=True)
self.assertRaises(
CustomError, self.loop.run_until_complete, coro)

def test_create_connection_multiple(self):
async def getaddrinfo(*args, **kw):
return [(2, 1, 6, '', ('0.0.0.1', 80)),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :exc:`IndexError` in :meth:`asyncio.loop.create_connection` that could
occur when non-\ :exc:`OSError` exception is raised during connection and
socket's ``close()`` raises :exc:`!OSError`.
Loading