Stop worker-owning nodes leaking forever when New Chat interrupts a generation#93
Merged
Merged
Conversation
…eneration
Started as a narrow fix (give ArtifactNode a GC-time __del__, matching
CodeSandboxNode) and grew once verification showed the __del__ alone
wouldn't do anything.
Root cause 1: ChatScene._teardown_items_before_clear() already calls
dispose() explicitly for chart_nodes on every clear() (New Chat /
chat-switch), but never did for ANY worker-owning node type
(artifact/conversation/pycoder/code_sandbox/gitlink) - it only stopped
their hover-animation timers. A GC-time __del__ backstop is inherently
non-deterministic (it can only fire once Python decides the object is
collectible) and, per root cause 2, sometimes can't fire at all.
_teardown_items_before_clear() now calls dispose() on all five
worker-owning node lists, exactly mirroring the chart_nodes treatment it
already had - New Chat now stops an in-flight generation the instant it
happens, not "eventually, if GC gets around to it."
Root cause 2 (found while checking whether the __del__ fix would even
matter): ArtifactNode was not collectible by Python's GC at all, ever,
regardless of clear()/dispose(). Two independent causes, both confirmed
empirically:
- ArtifactNode's own instruction_input.submit_requested was wired via
`lambda: self.artifact_requested.emit(self)` - a lambda closing over
self, connected to a custom Signal. PySide6's GC does not reclaim this
shape (a bound-method connection to the same signal is reclaimed
fine) - fixed by connecting a bound method instead.
- graphlink_window_actions.py wires each of ArtifactNode's/
ConversationNode's/PyCoderNode's/CodeSandboxNode's worker's OWN
finished/error(/status/cancelled/log_update/approval_requested)
signals via a lambda carrying node=/thread= as default args. As long
as those connections stood, BOTH the worker and the node were immortal
for the rest of the process - dispose()'s is_disposed guard never
mattered because nothing could ever collect the node to run a GC-time
__del__ in the first place. GitlinkNode was the one sibling whose
dispose() already disconnected the worker's own signals
(graphlink_plugin_gitlink.py:1385-1408); that pattern is now applied to
the other four.
Net changes: dispose() on all four affected node types now disconnects
the worker's own signals (mirroring GitlinkNode) before nulling the
reference; ArtifactNode's internal lambda replaced with a bound method;
ArtifactNode/ConversationNode gained a guarded __del__ (defense-in-depth,
matching CodeSandboxNode's existing convention - no longer the primary
mechanism now that clear() calls dispose() deterministically).
Verified: an adversarial workflow (2 independent agents, 120 tool calls)
re-confirmed the fix and reproduced every leak empirically with the real
worker classes (no mocks - a mock silently absorbs a bad connect() call
and hides this exact bug class). New regression tests reproduce the real
graphlink_window_actions.py wiring for all four node types and prove both
the node and the worker become GC-collectible after dispose(). Full suite
1705 passed. Real app launch verified clean (zero errors in the log).
The workflow also found the identical lambda-over-self pattern in three
unrelated context-menu classes and ColorPickerDialog - a different feature
area, not touched here; flagged separately as a follow-up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
This was referenced Jul 22, 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.
Summary
Started as a narrow fix (give
ArtifactNodea GC-time__del__, matchingCodeSandboxNode) and grew once verification showed the__del__alone wouldn't do anything.Root cause 1:
ChatScene._teardown_items_before_clear()already callsdispose()explicitly forchart_nodeson everyclear()(New Chat / chat-switch), but never did for any worker-owning node type (artifact/conversation/pycoder/code_sandbox/gitlink) — it only stopped their hover-animation timers. A GC-time__del__backstop is inherently non-deterministic, and (per root cause 2) sometimes can't fire at all._teardown_items_before_clear()now callsdispose()on all five worker-owning node lists, mirroring the existingchart_nodestreatment — New Chat now stops an in-flight generation the instant it happens.Root cause 2 (found while checking whether the
__del__fix would even matter):ArtifactNodewas not collectible by Python's GC at all, ever. Two independent causes, both confirmed empirically:ArtifactNode's owninstruction_input.submit_requestedwas wired via a lambda closing overselfon a custom Signal — fixed with a bound method.graphlink_window_actions.pywires each ofArtifactNode's/ConversationNode's/PyCoderNode's/CodeSandboxNode's worker's own signals via a lambda carryingnode=/thread=as default args. PySide6's GC does not reclaim a lambda connected to a custom Signal (a bound-method connection to the same signal is reclaimed fine) — so as long as those connections stood, both the worker and the node were immortal for the rest of the process, regardless ofdispose().GitlinkNodewas the one sibling whosedispose()already disconnected the worker's own signals; that pattern is now applied to the other four.Changes
dispose()onArtifactNode/ConversationNode/PyCoderNode/CodeSandboxNodenow disconnects the worker's own signals (mirroringGitlinkNode) before nulling the referenceArtifactNode's internal lambda replaced with a bound methodArtifactNode/ConversationNodegained a guarded__del__(defense-in-depth, matchingCodeSandboxNode's existing convention — no longer the primary mechanism now thatclear()callsdispose()deterministically)ChatScene._teardown_items_before_clear()callsdispose()on all 5 worker-owning node listsTest plan
connect()call and hides this exact bug classgraphlink_window_actions.pywiring for all four node types and prove both the node and the worker become GC-collectible afterdispose()The workflow also found the identical lambda-over-self pattern in three unrelated context-menu classes and
ColorPickerDialog— a different feature area (right-click menus, not worker-owning nodes), not touched here; flagged separately as a follow-up task.Third of the fixes from the recent adversarial bug-scan (after #91, #92).