Skip to content

Do not leak the SSL upgrade future when connect() is cancelled early - #1398

Open
alex-feel wants to merge 1 commit into
MagicStack:masterfrom
alex-feel:fix/ssl-upgrade-cancelled-connect
Open

alex-feel wants to merge 1 commit into
MagicStack:masterfrom
alex-feel:fix/ssl-upgrade-cancelled-connect

Conversation

@alex-feel

Copy link
Copy Markdown

Fixes #1397.

_create_ssl_connection() creates TLSUpgradeProto inside loop.create_connection(). When that call fails or is cancelled after the protocol exists, the transport is closed and connection_lost(None) sets ConnectionError('unexpected connection_lost() call') on on_data, which nothing awaits any more, so asyncio logs "Future exception was never retrieved". Since 0.32.0 Pool.close() cancels the floor-maintenance connect, so closing a pool while the server is unresponsive can log it.

on_data is now cancelled, or its exception retrieved, when create_connection() does not return, the way __connect_addr() handles the connected future. test_connection_cancelled_during_transport_setup covers ssl='prefer' as well; it fails on master with the two unretrieved futures and passes with the change.

_create_ssl_connection() creates TLSUpgradeProto inside
loop.create_connection(). When that call fails or is cancelled after the
protocol exists, the transport is closed and connection_lost(None) sets
ConnectionError('unexpected connection_lost() call') on on_data, which
nothing awaits any more, so asyncio logs "Future exception was never
retrieved". Since 0.32.0 the pool reaches this on its own: Pool.close()
cancels the floor-maintenance connect, and with an unresponsive server
the cancellation can land in that window.

Handle on_data the way __connect_addr() already handles the connected
future: cancel it, or retrieve its exception, when create_connection()
does not return.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cancelling connect() during the SSLRequest transport setup leaves an unretrieved ConnectionError

1 participant