Fix for unexpected socket closures and data leakage under heavy load by todddialpad · Pull Request #646 · MagicStack/uvloop · GitHub

This is to address issue #645 and in aiohttp/aiohappyeyeballs#93 and aiohttp/aiohappyeyeballs#112

The approach looks correct -- but I'm wondering how vanilla asyncio handles the same thing?

I think vanilla asyncio has an easier problem in that it can just have python sockets "all the way down", so just let reference counting take care of cleanup, while here we need to manage the disconnect with libuv dealing in file descriptors. I am suspecting there is some error handling path where a file descriptor is closed while the python socket object remains alive and not detached, so when it is finally closed, it messes up any new socket that happens to have the same file descriptor.
e.g. create socket s, call a loop method passing in an explicit socket, <bad error path which will end with sock.close()> overlapping with an .accept. I think the .accept never results in a python socket object being created.

So with the methods accepting sockets and other methods that internally work directly in file descriptors can there be a discrepancy?

@todddialpad very nice! Do you know does it help with the other issue #506 which seems to be also related to incorrect sharing of sockets etc?

Any possibility to add some test here?

@todddialpad very nice! Do you know does it help with the other issue #506 which seems to be also related to incorrect sharing of sockets etc?

Any possibility to add some test here?

I am trying to get a stable test. It is tricky because it is a race condition, if my guess is correct. I think it is a race if TLS negotiation during a call to loop.create_connection with an explicit socket is cancelled, and a subsequent incoming connection is accepted before the CancelledError is propagated. I think both libuv (or uvloop) first and aiohttp second close the underlying file descriptor.

So if this is the case, I don't think this will fix issue #506 , which could be a similar but different root cause.

So if this is the case, I don't think this will fix issue #506 , which could be a similar but different root cause.

Ok I see, the linked issue was also concerning as it looked as it was trying to write data into some incorrect socket. The error was also something we observed at similar time instances when we observed the response data getting leaked to incorrect requests. But we dont know is that issue actually related to the data leakage or just something else. (These RuntimeErrors dont happen with vanilla asyncio)

@todddialpad very nice! Do you know does it help with the other issue #506 which seems to be also related to incorrect sharing of sockets etc?
Any possibility to add some test here?

I am trying to get a stable test. It is tricky because it is a race condition, if my guess is correct. I think it is a race if TLS negotiation during a call to loop.create_connection with an explicit socket is cancelled, and a subsequent incoming connection is accepted before the CancelledError is propagated. I think both libuv (or uvloop) first and aiohttp second close the underlying file descriptor.

So if this is the case, I don't think this will fix issue #506 , which could be a similar but different root cause.

I still haven't been able to isolate a standalone, self-contained test. The test environment in which I generated the same error we see in production involves 2 VMs with significant network latency between them. The first of the VMs is just a web server, the second is a web server that accepts requests, and then makes outgoing client requests (using aiohttp) to the first webserver with TLS and a short timeout (around 1 second).

With this setup, I quite reliably get a failure within 250 connections. When I run with this patch applied, I have never had a failure in 20,000 connections.

We have also run this in our production environment. When we first encountered this failure, we hit it within 1 hour of using aiohttp >= 3.10. Since running with this patch we have been running for 5 days with no failures.

Is accepting this blocked on the tests that are failing? I don't think those failures are related to this change, as they are also failing for PR #644, which is solely a documentation change.

I looked at the test logs and I would guess that a dependency is causing the changed results. Related to this, I notice that in the failing tests, and alpha release of Cython 3.1 is being used (Using cached Cython-3.1.0a1-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata). Is this intentional?
In the last test run that passed, the release version was used (Using cached Cython-3.0.11-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata)

Hello everyone. Did i think right that this MR fix issues below?

"RuntimeError: File descriptor 2877 is used by transport <TCPTransport closed=False reading=True 0x55b8dc9baa90>"

Hello everyone :)

Like many other users of this library, I would be happy for this fix to be implemented in one of the upcoming releases.
Could you figure out roughly when to expect this fix?

@1st1 @fantix Is anyone available to merge and release this? We're getting asked to put workarounds into aiohttp to deal with this, would be nice to have this fixed here instead.

We added a workaround for this issue in aio-libs/aiohttp#10464 but its causing issues when using with asyncio SelectorEventLoop aio-libs/aiohttp#10617 so we will likely be reverting it and waiting for this PR instead

Hey @elprans @1st1, do you think you'll be able to spare a minute to get this in?

…10464 fixes #10617 alternative fix is MagicStack/uvloop#646

…10464 (#10656) Reverts #10464 While this change improved the situation for uvloop users, it caused a regression with `SelectorEventLoop` (issue #10617) The alternative fix is MagicStack/uvloop#646 (not merged at the time of this PR) issue #10617 appears to be very similar to python/cpython@d5aeccf If someone can come up with a working reproducer for #10617 we can revisit this. cc @top-oai Minimal implementation that shows on cancellation the socket is cleaned up without the explicit `close` #10617 (comment) so this should be unneeded unless I've missed something (very possible with all the moving parts here) ## Related issue number fixes #10617

…10464 (#10656) Reverts #10464 While this change improved the situation for uvloop users, it caused a regression with `SelectorEventLoop` (issue #10617) The alternative fix is MagicStack/uvloop#646 (not merged at the time of this PR) issue #10617 appears to be very similar to python/cpython@d5aeccf If someone can come up with a working reproducer for #10617 we can revisit this. cc @top-oai Minimal implementation that shows on cancellation the socket is cleaned up without the explicit `close` #10617 (comment) so this should be unneeded unless I've missed something (very possible with all the moving parts here) ## Related issue number fixes #10617 (cherry picked from commit 06db052)

…'s a failure in start_connection() #10464 (#10657) **This is a backport of PR #10656 as merged into master (06db052).** Reverts #10464 While this change improved the situation for uvloop users, it caused a regression with `SelectorEventLoop` (issue #10617) The alternative fix is MagicStack/uvloop#646 (not merged at the time of this PR) issue #10617 appears to be very similar to python/cpython@d5aeccf If someone can come up with a working reproducer for #10617 we can revisit this. cc @top-oai Minimal implementation that shows on cancellation the socket is cleaned up without the explicit `close` #10617 (comment) so this should be unneeded unless I've missed something (very possible with all the moving parts here) ## Related issue number fixes #10617 Co-authored-by: J. Nick Koston <nick@koston.org>

Hey @elprans @1st1, do you think you'll be able to spare a minute to get this in?

Sorry, @fantix and I will be going through this PR and others this week.

Hey! I noticed that aiohttp 3.11.14 has been yanked. For those of us using uvloop and aiohttp and running into the File descriptor 91 is used by transport error, do you happen to know if there’s a temporary workaround or a specific combination of versions we can pin to in the meantime? Totally understand if we need to wait for this to be merged, just trying to keep things running smoothly in the short term. Thanks a lot!

You can pin to a yanked version.

I folks
We are also running into below issue,

File descriptor 91 is used by transport

Wonder if their is a fix

Our setup

aiohappyeyeballs==2.6.1
aiohttp==3.11.18
aiohttp-cors==0.8.1
uvloop==0.21.0

Pinning to aiohttp==3.11.14 solved it for me

Would a manual execution of the tests on the main branch still pass (assuming it will grab Cython 3.1.0)?

I forked the main branch and tried running the tests. It fails with Cython 3.1.0. I pinned Cython to < 3.1.0 and the tests pass. I included this PR, and with the pinned Cython, all tests pass.

So I believe this PR could be merged. I created an issue for Cython 3.1.0 #677 .

Hi checking back on this, any ETA on when it would be merged

Following this PR waiting for the fix

Uvloop has a bug that is preventing from updating the libraries so it's being deactivated for now. MagicStack/uvloop#646

Hi folks any ETA on this

Hi Folks
can someone provide an update on this
We are facing an issue where we suddenly start seeing File descriptor 91 is used by transport on a running process and while the error happens the process is not able to serve traffic. This is impacting our service stability

We stopped using uvloop and didnt really observe any performance impact. Probably better to stop using it until the issue is fixed. Especially as we also observed information to get leaked under heavy load. (that issue is hard to reproduce locally)

Hi FolksHi Folks 大家好 can someone provide an update on this can someone provide an update on this有人能提供一下这个的最新情况吗 We are facing an issue where we suddenly start seeing We are facing an issue where we suddenly start seeing 我们遇到了一个问题，即突然开始看到File descriptor 91 is used by transport on a running process and while the error happens the process is not able to serve traffic. This is impacting our service stability on a running process and while the error happens the process is not able to serve traffic. This is impacting our service stability在一个正在运行的进程上，当错误发生时，该进程无法处理流量。这正影响着我们的服务稳定性。

Hello, I also have this problem. This problem occurs when calling a third-party interface times out and is in a high-concurrency scenario. Have you solved it? Please advise.

Have you solved it? Please advise.

Uninstall uvloop? Several users have reported that the performance difference is small today, so if it's breaking your application...

Have you solved it?

Uninstall uvloop? Several users have reported that the performance difference is small today, so if it's breaking your application...

Yes we uninstalled it and did not observe any change in performance. (We process non-trivial amount of requests, +30K RPS, in highly concurrent servers.).

gently ping @fantix @1st1

> Briefly describe what this PR accomplishes and why it's needed. Our serve ingress keeps running into below error related to `uvloop` under heavy load ``` File descriptor 97 is used by transport ``` The uvloop team have a [PR](MagicStack/uvloop#646) to fix it, but seems like no one is working on it One of workaround mentioned in the ([PR](MagicStack/uvloop#646 (comment))) is to just turn off uvloop . We tried it in our env and didn't see any major performance difference Hence as part of this PR, we are defining a new env for controlling UVloop Signed-off-by: jugalshah291 <shah.jugal291@gmail.com>

> Briefly describe what this PR accomplishes and why it's needed. Our serve ingress keeps running into below error related to `uvloop` under heavy load ``` File descriptor 97 is used by transport ``` The uvloop team have a [PR](MagicStack/uvloop#646) to fix it, but seems like no one is working on it One of workaround mentioned in the ([PR](MagicStack/uvloop#646 (comment))) is to just turn off uvloop . We tried it in our env and didn't see any major performance difference Hence as part of this PR, we are defining a new env for controlling UVloop Signed-off-by: jugalshah291 <shah.jugal291@gmail.com> Signed-off-by: Future-Outlier <eric901201@gmail.com>

This is now fixed in #740 with a different approach, which preserved the whole transport._attach_fileobj() setup for asyncio compatibility (see also python/asyncio#449). #740 also fixed the same issue on UDS, and added simulation tests.

Fix for unexpected socket closures and data leakage under heavy load

752f7c1

1st1 reviewed Nov 25, 2024

View reviewed changes

bdraco mentioned this pull request Jan 27, 2025

RuntimeError: File descriptor 2877 is used by transport <TCPTransport closed=False reading=True 0x55b8dc9baa90> aio-libs/aiohttp#10362

Closed

1 task

bdraco mentioned this pull request Feb 14, 2025

Close the socket if there's a failure in start_connection() aio-libs/aiohttp#10464

Merged

5 tasks

top-oai mentioned this pull request Feb 16, 2025

RuntimeError: File descriptor 2877 is used by transport <TCPTransport closed=False reading=True 0x55b8dc9baa90> #653

Open

bdraco mentioned this pull request Mar 12, 2025

OSError: [Errno 9] Bad file descriptor with uvloop aio-libs/aiohttp#10506

Open

1 task

bdraco mentioned this pull request Mar 31, 2025

BlockingIOError and File descriptor xx is used by transport aio-libs/aiohttp#10617

Closed

1 task

bdraco added a commit to aio-libs/aiohttp that referenced this pull request Apr 1, 2025

Revert: Close the socket if there's a failure in start_connection() #…

1928761

…10464 fixes #10617 alternative fix is MagicStack/uvloop#646

bdraco mentioned this pull request Apr 1, 2025

Revert: Close the socket if there's a failure in start_connection() #10464 aio-libs/aiohttp#10656

Merged

patchback Bot mentioned this pull request Apr 1, 2025

[PR #10656/06db052e backport][3.11] Revert: Close the socket if there's a failure in start_connection() #10464 aio-libs/aiohttp#10657

Merged

patchback Bot mentioned this pull request Apr 1, 2025

[PR #10656/06db052e backport][3.12] Revert: Close the socket if there's a failure in start_connection() #10464 aio-libs/aiohttp#10658

Merged

bdraco mentioned this pull request May 26, 2025

aiohttp.ClientSession() does not send TCP FIN aio-libs/aiohttp#4685

Open

GabrielSalla added a commit to GabrielSalla/sentinela that referenced this pull request Jun 1, 2025

remove uvloop package

66946cb

Uvloop has a bug that is preventing from updating the libraries so it's being deactivated for now. MagicStack/uvloop#646

GabrielSalla added a commit to GabrielSalla/sentinela that referenced this pull request Jun 1, 2025

remove uvloop package

821fc81

Uvloop has a bug that is preventing from updating the libraries so it's being deactivated for now. MagicStack/uvloop#646

bdraco mentioned this pull request Sep 24, 2025

Workaround uvloop losing track of sockets when passing a socket to create_connection aio-libs/aiohttp#11539

Closed

5 tasks

jugalshah291 mentioned this pull request Nov 7, 2025

Define an env for controlling UVloop ray-project/ray#58442

Merged

jauntysankey mentioned this pull request Apr 28, 2026

Detach socket on create_connection cancellation to prevent fd double-close #740

Merged

Merge branch 'master' into master

450737a

fantix closed this in dc680eb May 1, 2026

+                              sockfd = sock.detach()
                               # libuv will make socket non-blocking
-                              tr._open(sock.fileno())
+                              tr._open(sockfd)

Fix for unexpected socket closures and data leakage under heavy load by todddialpad · Pull Request #646 · MagicStack/uvloop · GitHub

Uh oh!

Fix for unexpected socket closures and data leakage under heavy load - #646

Fix for unexpected socket closures and data leakage under heavy load#646todddialpad wants to merge 3 commits intoMagicStack:masterMagicStack/uvloop:masterfrom todddialpad:mastertodddialpad/uvloop:masterCopy head branch name to clipboard

Conversation

todddialpad commented Nov 25, 2024

Uh oh!

1st1 Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

todddialpad Nov 28, 2024

Choose a reason for hiding this comment

Uh oh!

MarkusSintonen commented Nov 26, 2024

Uh oh!

todddialpad commented Nov 26, 2024

Uh oh!

MarkusSintonen commented Nov 26, 2024

Uh oh!

todddialpad commented Dec 2, 2024

Uh oh!

todddialpad commented Dec 4, 2024

Uh oh!

AntonArsentiev commented Jan 27, 2025

Uh oh!

AntonArsentiev commented Feb 3, 2025

Uh oh!

Dreamsorcerer commented Feb 16, 2025

Uh oh!

bdraco commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webknjaz commented Mar 31, 2025

Uh oh!

1st1 commented Apr 16, 2025

Uh oh!

jperezrealini commented Apr 24, 2025

Uh oh!

Dreamsorcerer commented Apr 24, 2025

Uh oh!

jugalshah291 commented May 8, 2025

Uh oh!

jperezrealini commented May 8, 2025

Uh oh!

todddialpad commented May 11, 2025

Uh oh!

jugalshah291 commented May 26, 2025

Uh oh!

GabrielSalla commented Jun 1, 2025

Uh oh!

jugalshah291 commented Jun 16, 2025

Uh oh!

jugalshah291 commented Jul 30, 2025

Uh oh!

MarkusSintonen commented Jul 31, 2025

Uh oh!

yybdyybd commented Sep 25, 2025

Uh oh!

Dreamsorcerer commented Sep 25, 2025

Uh oh!

MarkusSintonen commented Sep 25, 2025

Uh oh!

x0day commented Sep 28, 2025

Uh oh!

fantix commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

Fix for unexpected socket closures and data leakage under heavy load#646
todddialpad wants to merge 3 commits into
MagicStack:masterfrom
todddialpad:master

bdraco commented Mar 31, 2025 •
edited
Loading