Stop worker-owning nodes leaking forever when New Chat interrupts a generation

graphlink_app/graphlink_conversation_node.py

@@ -199,6 +199,20 @@ def __init__(self, parent_node, parent=None):
         self.proxy = QGraphicsProxyWidget(self)
         self.proxy.setWidget(self.widget)
 
+    def __del__(self):
+        # GC-time last resort, mirroring CodeSandboxNode.__del__ /
+        # ArtifactNode.__del__: dispose() is now also called deterministically
+        # from ChatScene._teardown_items_before_clear() on every clear() (New
+        # Chat / chat-switch), so this is defense-in-depth for any path that
+        # somehow bypasses both that and deleteSelectedItems(). Guarded
+        # because during interpreter shutdown the underlying C++ QThread/
+        # QObject may already be gone, making dispose()'s worker.isRunning()
+        # raise inside __del__.
+        try:
+            self.dispose()
+        except Exception:
+            pass
+
     def dispose(self):
         # Called by ChatScene.deleteSelectedItems when this node is removed. Cancel
         # the in-flight ChatWorkerThread so deletion doesn't orphan a live QThread:
@@ -213,12 +227,33 @@ def dispose(self):
             return
         self.is_disposed = True
         worker = getattr(self, "worker_thread", None)
-        try:
-            if worker and worker.isRunning():
-                worker.cancel()
-        except RuntimeError:
-            pass
         self.worker_thread = None
+        if worker:
+            try:
+                if worker.isRunning():
+                    worker.cancel()
+            except RuntimeError:
+                pass
+            # handle_conversation_node_request (graphlink_window_actions.py)
+            # wires this worker's own finished/error/cancelled to lambdas
+            # carrying node=/thread= default args - PySide6's GC does not
+            # reclaim a lambda connected to a custom Signal (empirically
+            # confirmed; a bound-method connection, like `status` below, is
+            # fine), so as long as those connections stand, this worker and
+            # this node are BOTH immortal for the rest of the process.
+            # Disconnecting here breaks that cycle. Mirrors GitlinkNode.dispose().
+            for signal in (worker.finished, worker.error, worker.status, worker.cancelled):
+                try:
+                    signal.disconnect()
+                except (TypeError, RuntimeError):
+                    pass
+            try:
+                if worker.isRunning():
+                    worker.finished.connect(worker.deleteLater)
+                else:
+                    worker.deleteLater()
+            except RuntimeError:
+                pass
 
     @property
     def width(self):

graphlink_app/graphlink_plugins/graphlink_plugin_artifact.py

@@ -179,9 +179,50 @@ def dispose(self):
             return
         self.is_disposed = True
         worker = getattr(self, "worker_thread", None)
-        if worker and worker.isRunning():
-            worker.stop()
         self.worker_thread = None
+        if worker:
+            try:
+                if worker.isRunning():
+                    worker.stop()
+            except Exception:
+                pass
+            # execute_artifact_node (graphlink_window_actions.py) wires this
+            # worker's own finished/error to lambdas carrying node=/thread=
+            # default args - PySide6's GC does not reclaim a lambda connected
+            # to a custom Signal (empirically confirmed; a bound-method
+            # connection is fine), so as long as those connections stand, this
+            # worker and this node are BOTH immortal for the rest of the
+            # process - dispose()'s own is_disposed=True guard never even runs
+            # again because __del__/dispose can no longer fire. Disconnecting
+            # here breaks that cycle. Mirrors GitlinkNode.dispose(), the one
+            # sibling that already did this correctly.
+            for signal in (worker.finished, worker.error):
+                try:
+                    signal.disconnect()
+                except (TypeError, RuntimeError):
+                    pass
+            try:
+                if worker.isRunning():
+                    worker.finished.connect(worker.deleteLater)
+                else:
+                    worker.deleteLater()
+            except RuntimeError:
+                pass
+
+    def __del__(self):
+        # GC-time last resort. ChatScene._teardown_items_before_clear() now
+        # also calls dispose() deterministically on every clear() (New Chat /
+        # chat-switch) - this is defense-in-depth for any path that somehow
+        # bypasses both that and deleteSelectedItems(). Guarded because during
+        # interpreter shutdown the underlying C++ QThread/QObject may already
+        # be deleted,
+        # making dispose()'s worker.isRunning() raise inside __del__ (same
+        # hazard as CodeSandboxNode.__del__, audit finding B5). Mirrors
+        # CodeSandboxNode exactly.
+        try:
+            self.dispose()
+        except Exception:
+            pass
 
     @property
     def width(self):
@@ -308,7 +349,7 @@ def _setup_ui(self):
             }
         """ + ARTIFACT_SCROLLBAR_STYLE)
         self.instruction_input.textChanged.connect(self._on_instruction_changed)
-        self.instruction_input.submit_requested.connect(lambda: self.artifact_requested.emit(self))
+        self.instruction_input.submit_requested.connect(self._on_instruction_submit_requested)
         input_layout.addWidget(self.instruction_input, stretch=1)
 
         self.update_button = QPushButton()
@@ -401,6 +442,16 @@ def _on_tab_changed(self, index):
     def _on_instruction_changed(self):
         self.instruction = self.instruction_input.toPlainText()
 
+    def _on_instruction_submit_requested(self):
+        # Was `submit_requested.connect(lambda: self.artifact_requested.emit(self))`.
+        # A lambda closing over self, connected to a custom (non-widget) Signal,
+        # is NOT a cycle PySide6's GC can break (confirmed empirically - unlike
+        # a signal connected to a bound method, which it collects fine); it held
+        # every ArtifactNode alive forever, silently defeating __del__'s
+        # GC-time dispose() below no matter what New Chat / chat-switch did.
+        # A plain bound method reaches the same target with no such trap.
+        self.artifact_requested.emit(self)
+
     def _on_content_changed(self):
         self.artifact_content = self.raw_editor.toPlainText()
 

graphlink_app/graphlink_plugins/graphlink_plugin_code_sandbox.py

@@ -769,9 +769,34 @@ def dispose(self):
             return
         self.is_disposed = True
         worker = getattr(self, "worker_thread", None)
-        if worker and worker.isRunning():
-            worker.stop()
         self.worker_thread = None
+        if worker:
+            try:
+                if worker.isRunning():
+                    worker.stop()
+            except Exception:
+                pass
+            # The run path (graphlink_window_actions.py) wires this worker's
+            # own finished/error/log_update/terminal_chunk/approval_requested
+            # to lambdas carrying node=/thread= default args - PySide6's GC
+            # does not reclaim a lambda connected to a custom Signal
+            # (empirically confirmed), so as long as those connections stand,
+            # this worker and this node are BOTH immortal for the rest of the
+            # process. Disconnecting here breaks that cycle. Mirrors
+            # GitlinkNode.dispose(), the one sibling that already did this.
+            for signal in (worker.finished, worker.error, worker.log_update,
+                           worker.terminal_chunk, worker.approval_requested):
+                try:
+                    signal.disconnect()
+                except (TypeError, RuntimeError):
+                    pass
+            try:
+                if worker.isRunning():
+                    worker.finished.connect(worker.deleteLater)
+                else:
+                    worker.deleteLater()
+            except RuntimeError:
+                pass
 
     def _on_prompt_changed(self):
         self.prompt = self.prompt_input.toPlainText()

graphlink_app/graphlink_pycoder.py

@@ -390,6 +390,20 @@ def __init__(self, parent_node, mode=PyCoderMode.AI_DRIVEN, parent=None):
 
         self._update_ui_for_mode()
 
+    def __del__(self):
+        # GC-time last resort, mirroring CodeSandboxNode.__del__ /
+        # ArtifactNode.__del__: dispose() is now also called deterministically
+        # from ChatScene._teardown_items_before_clear() on every clear() (New
+        # Chat / chat-switch), so this is defense-in-depth for any path that
+        # somehow bypasses both that and deleteSelectedItems(). Guarded
+        # because during interpreter shutdown the underlying C++ QThread/
+        # QObject may already be gone, making dispose()'s worker.isRunning()
+        # raise inside __del__.
+        try:
+            self.dispose()
+        except Exception:
+            pass
+
     def dispose(self):
         # Called by ChatScene.deleteSelectedItems when this node is removed. Stop the
         # running worker so deletion doesn't orphan a live QThread: once the node
@@ -402,9 +416,37 @@ def dispose(self):
             return
         self.is_disposed = True
         worker = getattr(self, "worker_thread", None)
-        if worker and worker.isRunning():
-            worker.stop()
         self.worker_thread = None
+        if worker:
+            try:
+                if worker.isRunning():
+                    worker.stop()
+            except Exception:
+                pass
+            # run_pycoder_node (graphlink_window_actions.py) wires this
+            # worker's own finished/error(/log_update/approval_requested for
+            # the AI-driven worker type) to lambdas carrying node=/thread=
+            # default args - PySide6's GC does not reclaim a lambda connected
+            # to a custom Signal (empirically confirmed), so as long as those
+            # connections stand, this worker and this node are BOTH immortal
+            # for the rest of the process. Disconnecting here breaks that
+            # cycle. Mirrors GitlinkNode.dispose(). getattr-guarded because
+            # the two worker types (CodeExecutionWorker for MANUAL,
+            # PyCoderExecutionWorker for AI_DRIVEN) don't share every signal.
+            for signal_name in ("finished", "error", "log_update", "approval_requested"):
+                signal = getattr(worker, signal_name, None)
+                if signal is not None:
+                    try:
+                        signal.disconnect()
+                    except (TypeError, RuntimeError):
+                        pass
+            try:
+                if worker.isRunning():
+                    worker.finished.connect(worker.deleteLater)
+                else:
+                    worker.deleteLater()
+            except RuntimeError:
+                pass
         # The REPL subprocess is owned by ChatWindow.pycoder_repl_manager, not this
         # node - stop it here for immediate, deterministic cleanup on this (the
         # right-click-delete) path. self.scene() is None for nodes never added to a

graphlink_app/graphlink_scene.py

@@ -1711,6 +1711,24 @@ def _try_teardown(item, method_name):
         for chart in self.chart_nodes:
             _try_teardown(chart, "dispose")  # ChartItem.dispose() is timer/figure-only
 
+        # Bug-scan finding: dispose() (stops the node's background QThread
+        # worker, and - for the 4 that wire it via a lambda closing over the
+        # node/thread, see each dispose()'s own comment - disconnects the
+        # worker's own signals so neither the worker nor the node is pinned
+        # alive forever) was only ever invoked from deleteSelectedItems(),
+        # never from clear(). New Chat / chat-switching mid-generation left
+        # the worker running with nothing to stop it, for every worker-owning
+        # node type, not just the one (ArtifactNode) this was first noticed
+        # on. GitlinkNode's dispose() already disconnected correctly - it was
+        # equally never being called here either.
+        worker_owning_node_lists = (
+            self.pycoder_nodes, self.code_sandbox_nodes,
+            self.conversation_nodes, self.artifact_nodes, self.gitlink_nodes,
+        )
+        for node_list in worker_owning_node_lists:
+            for node in node_list:
+                _try_teardown(node, "dispose")
+
     def clear(self):
         """
         Clears the entire scene, removing all items and resetting all tracking lists.