Asyncio SSL keep-alive connections raise errors after loop close.

[tomchristie]

BPO	36709
Nosy	@tiran, @asvetlov, @tomchristie, @1st1, @kumaraditya303

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-03-10.02:07:51.550>
created_at = <Date 2019-04-24.08:48:27.580>
labels = ['expert-SSL', '3.11', 'type-bug', 'expert-asyncio']
title = 'Asyncio SSL keep-alive connections raise errors after loop close.'
updated_at = <Date 2022-03-10.02:07:51.549>
user = 'https://github.com/tomchristie'

bugs.python.org fields:

activity = <Date 2022-03-10.02:07:51.549>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-10.02:07:51.550>
closer = 'asvetlov'
components = ['asyncio', 'SSL']
creation = <Date 2019-04-24.08:48:27.580>
creator = 'tomchristie'
dependencies = []
files = []
hgrepos = []
issue_num = 36709
keywords = []
message_count = 14.0
messages = ['340764', '340765', '343776', '343780', '343883', '343977', '343978', '343979', '343980', '343981', '343984', '379222', '414425', '414825']
nosy_count = 5.0
nosy_names = ['christian.heimes', 'asvetlov', 'tomchristie', 'yselivanov', 'kumaraditya']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36709'
versions = ['Python 3.11']

[tomchristie]

If an asyncio SSL connection is left open (eg. any kind of keep-alive connection) then after closing the event loop, an exception will be raised...

Python:

import asyncio
import ssl
import certifi


async def f():
    ssl_context = ssl.create_default_context()
    ssl_context.load_verify_locations(cafile=certifi.where())
    await asyncio.open_connection('example.org', 443, ssl=ssl_context)


loop = asyncio.get_event_loop()
loop.run_until_complete(f())
loop.close()

Traceback:

$ python example.py 
Fatal write error on socket transport
protocol: <asyncio.sslproto.SSLProtocol object at 0x10e7874a8>
transport: <_SelectorSocketTransport fd=8>
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/selector_events.py", line 868, in write
    n = self._sock.send(data)
OSError: [Errno 9] Bad file descriptor
Fatal error on SSL transport
protocol: <asyncio.sslproto.SSLProtocol object at 0x10e7874a8>
transport: <_SelectorSocketTransport closing fd=8>
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/selector_events.py", line 868, in write
    n = self._sock.send(data)
OSError: [Errno 9] Bad file descriptor

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/sslproto.py", line 676, in _process_write_backlog
    self._transport.write(chunk)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/selector_events.py", line 872, in write
    self._fatal_error(exc, 'Fatal write error on socket transport')
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/selector_events.py", line 681, in _fatal_error
    self._force_close(exc)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/selector_events.py", line 693, in _force_close
    self._loop.call_soon(self._call_connection_lost, exc)
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py", line 677, in call_soon
    self._check_closed()
  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/base_events.py", line 469, in _check_closed
    raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed

It looks to me like the original "OSError: [Errno 9] Bad file descriptor" probably shouldn't be raised in any case - if when attempting to tear down the SSL connection, then we should probably pass silently in the case that the socket has already been closed uncleanly.

Bought to my attention via: encode/httpcore#16

[tomchristie]

This appears somewhat related: https://bugs.python.org/issue34506

As it also logs exceptions occuring during _fatal_error and _force_close.

[asvetlov]

From my understanding, the correct code should close all transports and wait for their connection_lost() callbacks before closing the loop.

[tomchristie]

From my understanding, the correct code should close all transports and wait for their connection_lost() callbacks before closing the loop.

Ideally, yes, although we should be able to expect that an SSL connection that hasn't been gracefully closed wouldn't loudly error on teardown like that.

In standard sync code, the equivelent would running something like this...

session = requests.Session()
session.get('https://example.com/')

We wouldn't expect a traceback to be raised on exiting. (Even though the user *hasn't* explicitly closed the session, and even though a keep alive SSL connection will be open at the point of exit.)

[asvetlov]

I would say that if requests a designed from scratch more idiomatic way could be

with requests.Session() as session:
    session.get('https://example.com/')

or

session = requests.Session()
session.get('https://example.com/')
session.close()

Like the recommended way to handle files.

[tomchristie]

Right, and requests does provide both those styles.

The point more being that *not* having closed the transport at the point of exit shouldn't end up raising a hard error. It doesn't raise errors in sync-land, and it shouldn't do so in async-land.

Similarly, we wouldn't expect an open file resource to cause errors to be raised at the point of exit.

[asvetlov]

The difference is that socket.close() is an instant call.
After socket.close() the socket is done.
But transport.close() doesn't close the transport instantly.
asyncio requires at least one loop iteration for calling protocol.connection_lost() and actual socket closing.

In case of SSL it may take much longer.

Sorry, that's how asyncio is designed.

[1st1]

Sorry, that's how asyncio is designed.

Andrew, couldn't we provide a "stream.terminate()" method (not a coroutine) that would do the following:

• close the transport
• set a flag in the protocol that the stream has been terminated. When the flag is set, the protocol simply ignores all errors (i.e. they are never shown to the user or logged)

This way Tom could have a weakref to the stream object from his high-level wrapper, and whenever the wrapper object is dereferenced it could terminate its stream.

Sorry, that's how asyncio is designed.

I think it's a real problem, let's try to find out if we can provide a solution.

[asvetlov]

It's not about streams only.
The stream protocol can have such flag, sure.

But transport emits a warning like "unclosed transport ..."

Not sure if we have to drop this warning, it enforces writing good code that controls all created resources lifecycle.

[1st1]

Not sure if we have to drop this warning, it enforces writing good code that controls all created resources lifecycle.

Right, maybe we add "transport.terminate()" as well? Synchronously & immediately closing a transport is a valid use case.

TBH I don't see why we absolutely must wait the "connection_lost" callback call. I mean it would be reasonable to allow users to terminate the connection (even if it means that in some cases, like SSL, it won't be correctly closed) and not care about what happens to it next.

[asvetlov]

Sorry, I'm not comfortable with such change just before the feature freeze.
The idea is maybe good but let's discuss and implement it later.

[tiran]

This seems to be an asyncio problem.

[kumaraditya303]

The original issue is fixed on main branch with bpo-44011 #75458,
It now only raises warnings but no exceptions:

-----------------------------------------------------------
(env) @kumaraditya303 ➜ /workspaces/cpython (latin1 ✗) $ python main.py
/workspaces/cpython/main.py:12: DeprecationWarning: There is no current event loop
loop = asyncio.get_event_loop()
/workspaces/cpython/env/lib/python3.11/site-packages/certifi/core.py:36: DeprecationWarning: path is deprecated. Use files() instead. Refer to https://importlib-resources.readthedocs.io/en/latest/using.html#migrating-from-legacy for migration advice.
_CACERT_CTX = get_path("certifi", "cacert.pem")
sys:1: ResourceWarning: unclosed <socket.socket fd=7, family=2, type=1, proto=6, laddr=('172.16.5.4', 49202), raddr=('93.184.216.34', 443)>
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/workspaces/cpython/Lib/asyncio/sslproto.py:116: ResourceWarning: unclosed transport <asyncio._SSLProtocolTransport object>
_warnings.warn(
/workspaces/cpython/Lib/asyncio/selector_events.py:710: ResourceWarning: unclosed transport <_SelectorSocketTransport fd=7>
_warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback

-----------------------------------------------------------

@asvetlvo This can be closed now.

[asvetlov]

Thanks!