GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||
GeezerVison is showing github.com — Detach socket on create_connection cancellation to prevent fd double-close by junjzhang · Pull Request #740 · MagicStack/uvloop Conversation
Contributor
SummaryFixes #738. When This adds Changes
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsWhen create_connection(sock=sock) or create_unix_connection(sock=sock) is cancelled or raises, tr._close() closes the fd via libuv but the Python socket object still believes it owns that fd number. Its __del__ later closes whatever fd the OS recycled into that slot, corrupting unrelated transports. Call sock.detach() on the error path so the Python socket sets its internal fd to -1, matching the semantics of standard asyncio where the transport always takes full ownership of the socket. Fixes MagicStack#738
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
|
Hey @junjzhang @6matt, perhaps one option for you would be to switch to aiofastnet? Not only it is faster, its networking source code is following python 3.14 asyncio implementation, which doesn't have this bug. And if there is any other bug, I can fix it and release much faster. I waited for more than a year to get some of my uvloop PRs merged, and they haven't been released yet. Just saying |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I think this is a duplicate of #646 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
- Move test_create_connection_sock_cancel_detaches to _TestTCP so it runs on both uvloop and asyncio - Add test_create_connection_sock_cancel_fd_leak that reproduces the full data leak chain: cancel → fd reuse → stale close → writev to wrong socket (see MagicStack#645, aio-libs/aiohttp#10506) - Fix ConnectionAbortedError in detach test server handler Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Good call! Though, #646 fixed it in a way that breaks asyncio compatibility. I think this PR is a better approach. Let me also add a theoretical test to reproduce #645, as well as aio-libs/aiohttp#10506. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@1st1 wdyt?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
6d9acea to
9cc1d79
Compare
Mirror the TCP cancel/detach and data-leak tests for the create_unix_connection(sock=) path, covering the fix in both create_connection and create_unix_connection. Also fix server handlers to close writers (Python 3.12+ wait_closed() blocks until all connections are closed) and fix flake8 blank line issue. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
9cc1d79 to
74c24f1
Compare
There was an error while loading. Please reload this page.
Thanks for the background. I think the #646 approach was to make passed in socket (PSO) handling more like internally generated socket handling. When libuv generates the socket (such as through an |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Yeah, what you said is indeed an ideal resolution, but it is no longer identical to asyncio - where the passed-in |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hey first of all thanks for the fix @junjzhang |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
We hit what appears to be the same failure mode in a production disaggregated inference service using Python 3.12, uvloop 0.22.1, aiohttp 3.14.1, and Uvicorn. Under concurrent aiohttp connection attempts, retries, and cancellation, the API process first returned readiness, then its connections were reset, and the process terminated natively with SIGABRT ( We have switched the affected proxy and Python API serving paths to the standard-library asyncio loop in our release candidate. We plan to keep uvloop disabled until a release containing Thank you for fixing this and for adding the TCP and Unix socket regression coverage. An approximate release timeline would be very helpful for downstream users deciding when it is safe to re-enable uvloop. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Changes ======= * Add Python 3.15 and 3.15t wheel builds and CI coverage (MagicStack#758) (by @honglei @fantix in f7c0547) * Add support for the `eager_start` keyword argument in create_task() (MagicStack#748) (by @samypr100 in 3cbb095 for MagicStack#746 MagicStack#718) * Add thread name prefix to the default thread pool executor (MagicStack#636) (by @inikolaev @fantix in 0582f94 for MagicStack#562) * Add support for special hostname `<broadcast>` (MagicStack#592) (by @jpbede in 3060ceb for MagicStack#540) * Upgrade libuv to v1.52.1 (MagicStack#753) (by @fantix in e8efea4 for MagicStack#752) * Improve performance by using Python C API to enter/exit context (MagicStack#627) (by @tarasko in 837ef22) * Improve performance/latency of Transport.write (MagicStack#619) (by @tarasko in 1d9b6e0) * Replace some SSL vectorcall with direct methods (MagicStack#626) (by @tarasko in 6a27cbe) * Optimize SSL buffered reads using C values (MagicStack#629) (by @tarasko in a308f75) Fixes ===== * Detach socket on create_connection cancellation to prevent fd double-close (MagicStack#740) (by @junjzhang @fantix in dc680eb for MagicStack#645 MagicStack#738) * Remove `loop._ready_len` in favor of `len(loop._ready)` (MagicStack#721) (by @x42005e1f in 5910a18 for MagicStack#720) * Prefer inspect.iscoroutinefunction (MagicStack#705) (by @MatthieuDartiailh in fd65027 for MagicStack#703) * Fix context tests by explicitly yielding after run_in_executor (MagicStack#743) (by @samypr100 in 6cd24cb) * Fix test_create_connection_open_con_addr with Python 3.13.9+ (MagicStack#713) (by @shadchin in b93141a for MagicStack#701) * Skip flaky test_cancel_post_init on asyncio 3.13+ (MagicStack#714) (by @fantix in 3ea5c85 for MagicStack#709) * Fix flaky test_fs_event (MagicStack#717) (by @fantix in 8da4547) * Use C __atomic builtins for debug counters (MagicStack#719) (by @fantix in 836e3b2) * Update the example to work with Python 3.14 (MagicStack#710) (by @Jamie-Chang in b74c2f1) Build ===== * Replace pkg_resources with packaging and use Cython 3.1 (MagicStack#742) (by @samypr100 in b377b7c for MagicStack#729) * Remove wheel as a build dependency (MagicStack#696) (by @DimitriPapadopoulos in 173e88c) * Support any number of flags in UVLOOP_OPT_CFLAGS (MagicStack#630) (by @mgorny in 963a5f3)
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
create_connection(sock=sock) does not detach socket on cancellation, causing fd double-close
There was an error while loading. Please reload this page.