Break reference cycles keeping closed connections alive - #1750
Open
peteralm80 wants to merge 2 commits into
Open
Break reference cycles keeping closed connections alive#1750peteralm80 wants to merge 2 commits into
peteralm80 wants to merge 2 commits into
Conversation
gm122921980-create
approved these changes
Aug 21, 2026
gm122921980-create
approved these changes
Aug 21, 2026
gm122921980-create
approved these changes
Aug 21, 2026
gm122921980-create
approved these changes
Aug 21, 2026
gm122921980-create
approved these changes
Aug 21, 2026
added 2 commits
August 21, 2026 10:25
Closed connections could only be freed by the cyclic garbage collector, never by reference counting, because reference cycles referenced the connection: * The LoggerAdapter that injects the websocket attribute into log records held a strong reference to the connection, in all implementations: connection -> protocol -> logger -> extra dict -> connection. * In the asyncio implementation, the keepalive task retained the CancelledError raised when connection_lost() cancelled it, whose traceback references the keepalive() frame and thus the connection: connection -> task -> exception -> traceback -> frame -> connection. * In the trio implementation, the assembler held bound methods of the connection for flow control: connection -> recv_messages -> pause() and resume() callbacks -> connection. On servers handling many connections with high connect/disconnect churn, closed connections accumulated until a full collection of the oldest generation, which can lag far behind and pause the event loop for several seconds on large heaps, especially since CPython 3.13 collects the oldest generation incrementally. ConnectionLoggerAdapter now holds a weak reference to the connection and injects it into log records only while the connection is alive; records created while the connection is alive keep their own strong reference, so logging filters and handlers are unaffected. connection_lost() dereferences the keepalive task after cancelling it. The assembler drops the flow control callbacks when it's closed. Garbage collection tests are skipped on Python 3.12, where the traceback of an exception raised while closing a connection keeps frames of connection methods alive via their f_back attribute. Python 3.13 fixed that behavior.
When recv_exc is raised in a method of the connection, its traceback references frames of connection methods, which reference the connection: connection -> recv_exc -> traceback -> frame -> connection. This prevents reference counting from freeing closed connections. It happens on any read error in the sync and trio implementations, whose recv_events() is a method of the connection, and on write errors while responding to incoming frames in all implementations. Clear the frames of recv_exc's traceback once they finished running: after connection_lost() in the asyncio implementation and after recv_events() terminates in the sync and trio implementations. Clearing frames doesn't affect formatting the traceback. In the sync implementation, the thread running recv_events() must also dereference the connection when it terminates because the traceback of recv_exc keeps a reference to its frame via the f_back attribute of the recv_events() frame.
peteralm80
force-pushed
the
fix-connection-reference-cycles
branch
from
August 21, 2026 08:29
8b14445 to
e5ebdf4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1749.
Closed connections could only be freed by the cyclic garbage collector, never
by reference counting, because of two reference cycles (details, production
numbers, and reproduction in the issue):
LoggerAdapterthat injects thewebsocketattribute into log recordsheld a strong reference to the connection (all implementations).
CancelledErrorwhose traceback references thekeepalive()frame andthus the connection.
Changes:
New
ConnectionLoggerAdapterinwebsockets/utils.pyholds a weakreference to the connection and injects it into log records at logging time
(the
weakrefapproach suggested in Reference cycle in websockets 10? #1059). Behavior is unchanged for anyrecord emitted while the connection object is alive — including "connection
handler failed" in the server, since the handler still references the
connection at that point — because each record receives its own strong
reference via
extra. The documented adapter-wrapping pattern(
kwargs["extra"]["websocket"]withexcept KeyError) keeps working.Used by the asyncio, sync, and trio implementations. The deprecated legacy
implementation is left untouched.
Connection.connection_lost()(asyncio) dereferenceskeepalive_taskaftercancelling it, so the task, its
CancelledError, and the pinned frame dieby refcount once the event loop finishes the cancellation.
Tests:
tests/asyncio/test_connection.py(graceful close andabort) and
tests/sync/test_connection.pyassert withgc.disable()ineffect that a
weakrefto a closed connection dies — i.e. the connection isfreed by reference counting alone. They fail without the fix and are skipped
on non-CPython implementations.
ConnectionLoggerAdapterintests/test_utils.py.keepalive_taskafter close nowcapture the task before closing.
🤖 Generated with Claude Code