-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix SSL shutdown timeout for streaming connections #11094
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
Conversation
This PR addresses issue #11091 by adding a configurable `ssl_shutdown_timeout` parameter to control the SSL shutdown handshake timeout. This fixes the 30-second blocking issue when closing SSL connections, particularly affecting Azure Blob Storage streaming endpoints. In aiohttp 3.12.4, users experience a 30-second delay when closing SSL connections to streaming endpoints (like Azure Blob Storage). The root cause is that the SSL shutdown handshake can timeout when the remote server doesn't respond to the shutdown notification, causing Python's asyncio to wait for the full default timeout period. Added a new `ssl_shutdown_timeout` parameter with a default value of 0.1 seconds (100ms) to: - `TCPConnector` - Controls the timeout passed to `loop.start_tls()` (Python 3.11+ only) - `ClientSession` - Convenience parameter that passes through to the connector This provides a reasonable balance between: 1. Giving servers time to acknowledge the SSL shutdown (preventing "connection reset" errors) 2. Avoiding excessive delays during connection cleanup (fixing the 30-second block) **Note:** The `ssl_shutdown_timeout` parameter only takes effect on Python 3.11+ where `loop.start_tls()` supports this parameter. On earlier Python versions, the parameter is accepted but has no effect. - **aiohttp/connector.py**: Added `ssl_shutdown_timeout` parameter to `TCPConnector.__init__()` and conditionally passed it to `loop.start_tls()` on Python 3.11+ - **aiohttp/client.py**: Added `ssl_shutdown_timeout` parameter to `ClientSession.__init__()` and passed it to `TCPConnector` when creating one - Updated `docs/client_reference.rst` with parameter documentation for both classes - Added clear explanation of the parameter's purpose and default value - Documented that the parameter only takes effect on Python 3.11+ - Marked as added in version 3.12.5 - Added unit tests to verify parameter storage and passing - Added functional test to verify the timeout works correctly with streaming connections ```python async with aiohttp.ClientSession() as session: # SSL connections will have 0.1s shutdown timeout async with aiohttp.ClientSession(ssl_shutdown_timeout=0.5) as session: # SSL connections will have 0.5s shutdown timeout async with aiohttp.ClientSession(ssl_shutdown_timeout=None) as session: # SSL connections will wait indefinitely for shutdown ``` This change is fully backward compatible: - Default value (0.1s) provides reasonable behavior for most use cases - Existing code continues to work without modification - Users experiencing issues can adjust the timeout as needed The PR includes comprehensive tests: 1. Unit tests verify the parameter is correctly stored and passed between components 2. Functional test simulates a streaming SSL connection and verifies the timeout behavior 3. All existing tests continue to pass - Fixes #11091 - Related to Python's asyncio SSL implementation changes - Similar issues have been reported with other async libraries using SSL
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11094 +/- ##
==========================================
+ Coverage 98.81% 98.82% +0.01%
==========================================
Files 129 129
Lines 41395 41493 +98
Branches 2231 2234 +3
==========================================
+ Hits 40906 41007 +101
+ Misses 339 337 -2
+ Partials 150 149 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #11094 will not alter performanceComparing Summary
|
This issue burned us pretty badly today. Down for almost 8 hours. I get it's not your fault, just adding my 2c that something needs to be done and we all know Microsoft isn't the speediest roadrunner in the race. |
Backport to 3.12: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 50bb06b on top of patchback/backports/3.12/50bb06be197b249ab91a4cef33ef7e10ffd4d95a/pr-11094 Backporting merged PR #11094 into master
🤖 @patchback |
Backport to 3.13: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 50bb06b on top of patchback/backports/3.13/50bb06be197b249ab91a4cef33ef7e10ffd4d95a/pr-11094 Backporting merged PR #11094 into master
🤖 @patchback |
(cherry picked from commit 50bb06b)
(cherry picked from commit 50bb06b)
## Summary & Motivation - Pinned Spark<4 (but upgraded to latest Spark 3 and related deps) - Fixed `mc` command, as in apache/iceberg-python#2049 - aio-libs/aiohttp#11094 fixed aio-libs/aiohttp#11091, so the `aiohttp` pin is no longer necessary ## How I Tested These Changes - CI
What do these changes do?
This PR addresses issue #11091 by adding a configurable
ssl_shutdown_timeout
parameter to control the SSL shutdown handshake timeout. This fixes the 30-second blocking issue when closing SSL connections, particularly affecting Azure Blob Storage streaming endpoints.Problem
In aiohttp 3.12.4, users experience a 30-second delay when closing SSL connections to streaming endpoints (like Azure Blob Storage). The root cause is that the SSL shutdown handshake can timeout when the remote server doesn't respond to the shutdown notification, causing Python's asyncio to wait for the full default timeout period.
Solution
Added a new
ssl_shutdown_timeout
parameter with a default value of 0.1 seconds (100ms) to:TCPConnector
- Controls the timeout passed to both:loop.create_connection()
for direct HTTPS connections (Python 3.11+ only)loop.start_tls()
for HTTPS proxy CONNECT tunnels (Python 3.11+ only)ClientSession
- Convenience parameter that passes through to the connectorThis provides a reasonable balance between:
Note: The
ssl_shutdown_timeout
parameter only takes effect on Python 3.11+ where the asyncio SSL methods support this parameter. On earlier Python versions, the parameter is accepted but has no effect.Changes
Core Implementation
ssl_shutdown_timeout
parameter toTCPConnector.__init__()
_create_direct_connection()
to unconditionally pass the timeout toloop.create_connection()
for all connections (Python 3.11+)_start_tls_connection()
to pass the timeout toloop.start_tls()
(Python 3.11+)ssl_shutdown_timeout
parameter toClientSession.__init__()
and passed it toTCPConnector
when creating oneDocumentation
docs/client_reference.rst
with parameter documentation for both classesTests
Usage
Backward Compatibility
This change is fully backward compatible:
Testing
The PR includes comprehensive tests:
References
Are there changes in behavior for the user?
shutdown will no longer take 30s max
Is it a substantial burden for the maintainers to support this?
no