Break reference cycles keeping closed connections alive - #1750
Open
peteralm80 wants to merge 1 commit into
Open
Conversation
Closed connections could only be freed by the cyclic garbage collector, never by reference counting, because two 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. 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() also dereferences the keepalive task after cancelling it.
gm122921980-create
approved these changes
Aug 21, 2026
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
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