resurrected asyncio _SelectorTransport unregisters fds it doesn't own

Bug report

Bug description:

Short story:

Socket is closed in _SelectorTransport.__del__, so it doesn't own the fd anymore.
Anything else in the process can create a new file descriptor at this point.
If the transport is gc-resurrected to call close(), it can then remove the reader/writer from loop._selector here or here for a fd it doesn't control anymore, causing access to that fd to hang.

Long story:

Introduce a GC cycle, such that an asyncio.selector_events._SelectorTransport and the object owning it are collected at the same time.
The transport __del__ will be called, closing the socket, freeing its file descriptor, but not performing any other cleanup.
The owning object, being a good citizen, resurrects the transport during gc to have close() called on it properly. In the case of this issue, an asyncio task was created to close it later. (I definitely don't love seeing asyncio.create_task in a __del__ method, but I expect that's not the only way to trigger this)
At this point, anything in your asyncio runloop can open a file descriptor. In my case, it was asyncio.create_subprocess_exec calling os.pidfd_create() in PidfdChildWatcher.add_child_handler. If you're lucky (or running a lot of tasks at once), the new fd will be the same as the resurrected socket's fd.
Now transport.close() can run any time later, which will call loop._selector.remove_child_watcher(fd), despite not really owning the fd anymore.
Now the pidfd has been removed from loop._selector, so await proc.wait() will hang forever. Or whatever asyncio stream you were trying to use got removed from the selector and you won't be able to wait on it.

I definitely think the non-stdlib code in play here was unsound, but I don't think that should trigger such a cursed result in the stdlib, so I'd rather defend against this in asyncio.

I think this is the minimum patch to mitigate the issue, to prevent the remove_child_reader and remove_child_writer calls by breaking their pre-conditions. I'm not sure if it's worth trying to interact with the loop or selector from __del__ to clean up the fd. (Selecting on a missing fd is probably handled somewhere else anyway?)

--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -871,6 +871,8 @@ def close(self):
     def __del__(self, _warn=warnings.warn):
         if self._sock is not None:
             _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
+            self._closing = True
+            self._buffer.clear()
             self._sock.close()
             if self._server is not None:
                 self._server._detach(self)

Another consideration is that asyncio is using a cached self._sock_fd here without really knowing if the underlying socket was closed and fd was recycled or not. I think this isn't the first time the cached fd has caused a surprising behavior, see also #88968

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

pythongh-130141: mark _SelectorTransport as closing during del

@colesbury

This seems like a bigger problem especially wrt free-threading of reusing of sockets and file descriptors.

cc @colesbury @gvanrossum

I'm not sure how free-threading makes things worse here -- even without it, another thread might execute and open a new file whose fd equals the old fd of the closed socket.

It looks like __del__ here was written without considering the consequences of resurrection.

I feel the need to think in terms of invariants.

For example, self._sock_fd should roughly behave as if it was a property returning self._sock.fileno(). Therefore, every place where we either call self._sock.close() or set self._sock = None, we should set self._sock_fd to some clearly invalid value -- maybe -1 when self._sock is closed (since that's what fileno() returns for a closed socket -- a universally invalid fd) and None when self._sock = None is executed (hopefully after the socket is closed :-). But I am also okay with always setting it to None.

An invariant that seems to be already maintained is to call self._server.detach(self), except we should also set self._server = None as soon as that is done.

I feel it's also important to remove the fd from the loop; otherwise the loop may be polling events for an unrelated fd (see first line). IIUC, I think it's fine to call loop._remove_{reader,writer}, since they are not blocking calls (not async!) -- though they might make system calls for certain selector types: I assume those don't block, since they "just" have to update a kernel data structure. In fact, this should probably be done before closing the socket!

Finally I also believe we have a commitment to tell the protocol that the connection is lost. But since the code currently warns that the transport is unclosed, I believe that we can punt on that -- otherwise __del__ might as well just call self.close(). :-)

All in all, I think this should do it:

diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index 22147451fa7..2b0ac995a89 100644
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -870,10 +870,19 @@ def close(self):
 
     def __del__(self, _warn=warnings.warn):
         if self._sock is not None:
-            _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
-            self._sock.close()
-            if self._server is not None:
-                self._server._detach(self)
+            if self._protocol_connected and self._protocol is not None:
+                _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
+            self._protocol_connected = False
+            self._protocol = None
+            server = self._server
+            if server is not None:
+                self._server = None
+                server._detach(self)
+            self._loop._remove_reader(self._sock_fd)
+            if self._buffer:
+                self._buffer.clear()
+                self._loop._remove_writer(self._sock_fd)
+            self._loop = None
 
     def _fatal_error(self, exc, message='Fatal error on transport'):
         # Should be called from exception handler only.

This passes the existing asyncio tests (though it could use a test that specifically tests the resurrection use case).

I'm not sure if you intended to remove self._sock.close() in that patch but it otherwise makes sense. I'm also in favor of setting sock_fd to -1 immediately anytime we close the socket.

Good call; and I think I need to research how self._buffer is actually used.

I will go over the other places where the invariants might be violated, and make them all uniform. Then the only thing that close does that __del__ doesn't is schedule the call to self._call_connection_lost -- instead we have the warning and set _protocol to None and _protocol_connected to False.

(Notes for myself:)

self._sock = sock; self._sock_fileno = sock.fileno() (sock is an argument to __init__)
self.protocol_connected = False; self.set_protocol(protocol) (ditto)
self._server = server (optional argument)
self._buffer = collections.deque()
self._conn_lost = 0 (set to 1 when _call_connection_lost is scheduled, after that counts how many times we've seen write called since that call was scheduled -- if there are more than 5, we log a warning)
self._closing = False (set by close())
self._paused = False (managed by {pause,resume}_reading)

[EDIT: I rewrote this paragraph for greater clarity, and added the rest.]
There is an invariant that if self._buffer is empty, no writer is configured, and when it is non-empty, a writer is configured. But close() apparently doesn't fully trust invariant this and removes the writer when the buffer is empty (which is harmless since the socket is still open and thus it can't be anyone else's fd). If the buffer isn't empty, close() assumes that the write callback does the rest of the cleanup. That callback is either _write_sendmsg or _write_send (both defined on _SelectorSocketTransport), which are identical where it comes to cleanup. Each calls _remove_writer, and then also (since close has set self._closing) calls (not schedules!) _call_connection_lost, which does most of the work for close() and friends. Those callbacks may be called multiple times, until they manage to empty the buffer. They do the cleanup part only in that case.

The intended invariant around self._buffer, and _add_writer/_remove_writer is then:

Either the buffer is empty and no writer callback is configured
Or the buffer is not empty, a writer callback is configured

The invariants around the writer callback are:

When it is called, the buffer is non-empty (and a writer callback is configured)
When it returns:
- Either we're still in the same state
- Or the buffer is empty and the callback is cleared

Note that in the special case where self._conn_lost is nonzero, it returns immediately, maintaining the invariant. In this case, the _call_connection_lost callback is always scheduled and when it runs, will finish the closing of the transport.

TO BE CONTINUED.

I'm not sure how free-threading makes things worse here -- even without it, another thread might execute and open a new file whose fd equals the old fd of the closed socket.

When running free-threading with tsan, it will report these as data races, I haven't seen in practise on gil builds but i think it can happen with gil too, just that it is much more likely to happen in free-threading. See #121544

So the tasks of _call_connection_lost are:

Call the protocol's connection_lost, but only if self._protocol_connected is set. (by set_protocol). If this fails, it still does the rest of the cleanup (re-raising the exception in the end).
Close socket and break reference to it (and it should set _sock_fd to -1 too).
Break the references to protocol and loop.
Detach from server (if set) and break reference.

It does all of these unconditionally (except for the server detach). I don't know how much it cares about the order

It is scheduled from close and _force_close, and is called directly from _write_sendmsg and _write_send (in the subclass _SelectorSocketTransport). It is also called directly from _sendto_ready in _SelectorDatagramTransport.

It looks as if everywhere it's called or scheduled, _remove_writer is called first, and before that we ensure that the buffer is empty -- either by clearing it (_force_close) or by checking that it's empty. If the buffer is not empty, we normally wait for it to become empty, which it will eventually.

A plan is emerging...

Going through __del__ should be the same as going through _force_close, except we never call the protocol's connection_lost -- instead, we issue the warning (if self._protocol_connected is set).

If the protocol is connected, issue warning, and set self._protocol_connected = False.
If there is a buffer, clear it, and remove the writer callback.
If we're not yet closing, set self._closing = True, and remove the reader callback.
Close the socket, set self._sock = None, and self._sock_fd = -1 (actually, the latter happens first).
Unlink protocol and loop.
Detach from server (if set) and unlink it.

The bullets through "close the socket" are done conditionally (only if the socket is still there).

We could bicker about the order in which to do these things, but I propose to keep them in the same order as _force_close and _call_connection_lost do them. There's one difference in order with close (it sets self._closing and removes the reader before checking the buffer) and it probably doesn't matter, but I think I prefer aligning with _force_close. (Alternatively, if we decide the close order is better, we should swap this in _force_close.)

Other than that, cosmetic changes to close and _call_connection_lost.

diff --git a/Lib/asyncio/selector_events.py b/Lib/asyncio/selector_events.py
index 22147451fa7..eb9ee4641dc 100644
--- a/Lib/asyncio/selector_events.py
+++ b/Lib/asyncio/selector_events.py
@@ -864,16 +864,37 @@ def close(self):
         self._closing = True
         self._loop._remove_reader(self._sock_fd)
         if not self._buffer:
-            self._conn_lost += 1
             self._loop._remove_writer(self._sock_fd)
+            self._conn_lost += 1
             self._loop.call_soon(self._call_connection_lost, None)
 
     def __del__(self, _warn=warnings.warn):
         if self._sock is not None:
-            _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
+            if self._protocol_connected:
+                self._protocol_connected = False
+                _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
+
+            if self._buffer:
+                self._buffer.clear()
+                self._loop._remove_writer(self._sock_fd)
+
+            if not self._closing:
+                self._closing = True
+                self._loop._remove_reader(self._sock_fd)
+
+            self._conn_lost += 1
+
+            self._sock_fd = -1
             self._sock.close()
-            if self._server is not None:
-                self._server._detach(self)
+            self._sock = None
+
+        self._protocol = None
+        self._loop = None
+
+        server = self._server
+        if server is not None:
+            self._server = None
+            server._detach(self)
 
     def _fatal_error(self, exc, message='Fatal error on transport'):
         # Should be called from exception handler only.
@@ -904,8 +925,10 @@ def _force_close(self, exc):
     def _call_connection_lost(self, exc):
         try:
             if self._protocol_connected:
+                self._protocol_connected = False
                 self._protocol.connection_lost(exc)
         finally:
+            self._sock_fd = -1
             self._sock.close()
             self._sock = None
             self._protocol = None

Then again, the minimal version would be:

Remove reader and writer
Detach server if present
Close socket if present
Clear buffer
Set all variables to None or -1 as appropriate
Issue warning, if protocol connected (since warnings may become exceptions, I'd do this last)

We do still have to worry about being deleted during shutdown -- what if the loop is already dead? Or the server? (Could this happen if everything was part of a big cycle?)

@lunixbochs

@lunixbochs @kumaraditya303 Any thoughts? Anybody want to prepare an actual PR? Or even just write a test that reproduces the original issue? so we can appreciate it's actually being fixed?

pythongh-130141: clean up asyncio._SelectorTransport during __del__

@gvanrossum

couldn't get the GC to behave in a small example, so here's a repro that calls __del__ directly, works for me on Linux (where PidfdChildWatcher is used)

import asyncio

async def subprocess():
    print("subprocess starting")
    p = await asyncio.create_subprocess_exec("sleep", "0.5")
    await p.wait()
    print("subprocess finished")

async def start_client(port: int):
    client = await asyncio.open_connection("127.0.0.1", port)
    task = asyncio.create_task(subprocess())
    client[0]._transport.__del__()
    await asyncio.sleep(0)
    return task, client

async def ignore_client(r, w):
    await asyncio.sleep(1000)

async def main():
    server = await asyncio.start_server(ignore_client, "127.0.0.1", 0)
    s = server.sockets[0]
    _, port = s.getsockname()
    task, client = await start_client(port)

    client[1].close()

    await task
    print("end of main")

if __name__ == "__main__":
    asyncio.run(main())

@gvanrossum's patch fixes this repro, so I've updated my existing PR with that patch #130142

Great! Do you feel up to the task of turning that into an actual unittest in your PR? We need a unittest before the PR can be merged.

I'm not sure how to unit test this exact repro correctly, as it relies on Linux happening to reuse a file descriptor and then the wait on the selector hanging, and the function call ordering here is fragile to changes in both the gc and asyncio.

The more direct test would be to manually check the side effects of del -> close, and I usually feel weird about making tests which are 1:1 with the code and mostly test "the code didn't change".

bumping this - I have no idea how to write a reasonable test for this

If a test is required, I still need advice on how to test for this in a non-fragile way. The above repro relies on implicit behavior of both asyncio and Linux. I don't think it actually tests the issue, just the current circumstances. I have absolutely no idea how to correctly test for this and I'm not hearing any other ideas. This is a cpython bug with a known fix that is causing issues for people today. Please advise.

I'm on vacation and I've not followed the whole thread, but the fact that the open() and other fd-creating syscalls on Unix-ish systems uses the lowest unused file descriptor is not a "implicit behavior". IIUC this behavior has been specified since at least UNIX v6 (50 years ago!). Until the addition of dup2() much later, the official way to e.g. equate stderr (fd 2) to stdout (fd 1) was close(2); dup(1). If this is what you worry about, please stop worrying!

@lunixbochs

@lunixbochs Azure-Samples/azure-search-openai-demo#2586 same issue

We encountered a related asyncio transport-finalizer failure in Pounce, an ASGI server targeting free-threaded Python 3.14. The failure appeared under real SIGTERM load on Python 3.14t on both Ubuntu and macOS:

Exception ignored in: <function _SelectorTransport.__del__ ...>
Traceback (most recent call last):
  ... selector_events.py, in __del__
    self._server._detach(self)
  ... base_events.py, in _detach
    self._wakeup()
  ... base_events.py, in _wakeup
    for waiter in waiters:
TypeError: 'NoneType' object is not iterable

The sequence we observed was:

asyncio.Server.close() saw an empty transport WeakSet, called _wakeup(), and set _waiters = None.
A delayed _SelectorTransport.__del__() subsequently called Server._detach().
_detach() called _wakeup() a second time, which attempted to iterate None.

Our integration reproducer uses two workers with concurrent short, slow, streaming, keep-alive, and newly arriving connections during real SIGTERM. Retaining every handler task through writer.wait_closed() was correct downstream ownership but did not eliminate this pre-callback/finalizer path. An explicit gc.collect() immediately before Server.close() also still reproduced the traceback under concurrent stress.

Our downstream compatibility guard makes the per-server _wakeup() idempotent once _waiters is None. The same high-concurrency profile that reproduced the failure before the guard passed 120/120 runs afterward on an exact CPython 3.14.6 free-threaded build. The fix also passed Pounce's Ubuntu and macOS 3.14t CI lanes.

References:

Downstream report: lbliii/pounce#301
Fix, discussion, and proof: lbliii/pounce#302
Mixed-load SIGTERM regression test: test_sigterm_drains_under_mixed_load

By inspection, #130142 addresses finalizer cleanup and stale fd ownership, but it still calls server._detach(self) and leaves Server._wakeup() non-idempotent. So the Pounce failure may be an additional invariant exposed by the same late-finalizer family rather than proof that #130142 alone fixes our case.

Would you prefer this double-wakeup path as a separate CPython issue/test, or is it useful to track it here alongside #130142?

Add axis 2: complete classification of all 73 open topic-asyncio issues

Export wave-2 asyncio patches (pythongh-130141 revived + tested, pyth…

Navigation Menu

Repository navigation

resurrected asyncio `_SelectorTransport` unregisters fds it doesn't own #130141

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

resurrected asyncio _SelectorTransport unregisters fds it doesn't own #130141

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

kumaraditya303 commented on Feb 15, 2025

gvanrossum commented on Feb 15, 2025

lunixbochs commented on Feb 15, 2025

gvanrossum commented on Feb 15, 2025

gvanrossum commented on Feb 16, 2025

kumaraditya303 commented on Feb 16, 2025

gvanrossum commented on Feb 16, 2025

gvanrossum commented on Feb 16, 2025

gvanrossum commented on Feb 18, 2025

lunixbochs commented on Feb 19, 2025

gvanrossum commented on Feb 19, 2025

lunixbochs commented on Feb 19, 2025

lunixbochs commented on Mar 25, 2025

lunixbochs commented on Apr 17, 2025

gvanrossum commented on Apr 18, 2025

cforce commented on Jun 26, 2025

lbliii commented on Jul 9, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

resurrected asyncio `_SelectorTransport` unregisters fds it doesn't own #130141