[fix] prevent memory leak in SubgraphNode event listeners (#1150)

Author: christian-byrne

src/subgraph/SubgraphNode.ts

@@ -39,6 +39,9 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
 
   override widgets: IBaseWidget[] = []
 
+  /** Manages lifecycle of all subgraph event listeners */
+  #eventAbortController = new AbortController()
+
   constructor(
     /** The (sub)graph that contains this subgraph instance. */
     override readonly graph: GraphOrSubgraph,
@@ -50,45 +53,47 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
 
     // Update this node when the subgraph input / output slots are changed
     const subgraphEvents = this.subgraph.events
+    const { signal } = this.#eventAbortController
+
     subgraphEvents.addEventListener("input-added", (e) => {
       const subgraphInput = e.detail.input
       const { name, type } = subgraphInput
       const input = this.addInput(name, type)
 
       this.#addSubgraphInputListeners(subgraphInput, input)
-    })
+    }, { signal })
 
     subgraphEvents.addEventListener("removing-input", (e) => {
       const widget = e.detail.input._widget
       if (widget) this.ensureWidgetRemoved(widget)
 
       this.removeInput(e.detail.index)
-    })
+    }, { signal })
 
     subgraphEvents.addEventListener("output-added", (e) => {
       const { name, type } = e.detail.output
       this.addOutput(name, type)
-    })
+    }, { signal })
 
     subgraphEvents.addEventListener("removing-output", (e) => {
       this.removeOutput(e.detail.index)
-    })
+    }, { signal })
 
     subgraphEvents.addEventListener("renaming-input", (e) => {
       const { index, newName } = e.detail
       const input = this.inputs.at(index)
       if (!input) throw new Error("Subgraph input not found")
 
       input.label = newName
-    })
+    }, { signal })
 
     subgraphEvents.addEventListener("renaming-output", (e) => {
       const { index, newName } = e.detail
       const output = this.outputs.at(index)
       if (!output) throw new Error("Subgraph output not found")
 
       output.label = newName
-    })
+    }, { signal })
 
     this.type = subgraph.id
     this.configure(instanceData)
@@ -327,6 +332,9 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
   }
 
   override onRemoved(): void {
+    // Clean up all subgraph event listeners
+    this.#eventAbortController.abort()
+
     // Clean up all promoted widgets
     for (const widget of this.widgets) {
       this.subgraph.events.dispatch("widget-demoted", { widget, subgraphNode: this })

test/subgraph/SubgraphNode.test.ts

@@ -5,7 +5,7 @@
  * IO synchronization, and edge cases.
  */
 
-import { describe, expect, it } from "vitest"
+import { describe, expect, it, vi } from "vitest"
 
 import { LGraph, Subgraph } from "@/litegraph"
 
@@ -492,3 +492,87 @@ describe("Foundation Test Utilities", () => {
     expect(parentGraph.nodes).toContain(subgraphNode)
   })
 })
+
+describe("SubgraphNode Cleanup", () => {
+  it("should clean up event listeners when removed", () => {
+    const rootGraph = new LGraph()
+    const subgraph = createTestSubgraph()
+
+    // Create and add two nodes
+    const node1 = createTestSubgraphNode(subgraph)
+    const node2 = createTestSubgraphNode(subgraph)
+    rootGraph.add(node1)
+    rootGraph.add(node2)
+
+    // Verify both nodes start with no inputs
+    expect(node1.inputs.length).toBe(0)
+    expect(node2.inputs.length).toBe(0)
+
+    // Remove node2
+    rootGraph.remove(node2)
+
+    // Now trigger an event - only node1 should respond
+    subgraph.events.dispatch("input-added", {
+      input: { name: "test", type: "number", id: "test-id" } as any,
+    })
+
+    // Only node1 should have added an input
+    expect(node1.inputs.length).toBe(1) // node1 responds
+    expect(node2.inputs.length).toBe(0) // node2 should NOT respond (but currently does)
+  })
+
+  it("should not accumulate handlers over multiple add/remove cycles", () => {
+    const rootGraph = new LGraph()
+    const subgraph = createTestSubgraph()
+
+    // Add and remove nodes multiple times
+    const removedNodes: SubgraphNode[] = []
+    for (let i = 0; i < 3; i++) {
+      const node = createTestSubgraphNode(subgraph)
+      rootGraph.add(node)
+      rootGraph.remove(node)
+      removedNodes.push(node)
+    }
+
+    // All nodes should have 0 inputs
+    for (const node of removedNodes) {
+      expect(node.inputs.length).toBe(0)
+    }
+
+    // Trigger an event - no nodes should respond
+    subgraph.events.dispatch("input-added", {
+      input: { name: "test", type: "number", id: "test-id" } as any,
+    })
+
+    // Without cleanup: all 3 removed nodes would have added an input
+    // With cleanup: no nodes should have added an input
+    for (const node of removedNodes) {
+      expect(node.inputs.length).toBe(0) // Should stay 0 after cleanup
+    }
+  })
+
+  it("should clean up input listener controllers on removal", () => {
+    const rootGraph = new LGraph()
+    const subgraph = createTestSubgraph({
+      inputs: [{ name: "in1", type: "number" }, { name: "in2", type: "string" }],
+    })
+
+    const subgraphNode = createTestSubgraphNode(subgraph)
+    rootGraph.add(subgraphNode)
+
+    // Verify listener controllers exist
+    expect(subgraphNode.inputs[0]._listenerController).toBeDefined()
+    expect(subgraphNode.inputs[1]._listenerController).toBeDefined()
+
+    // Track abort calls
+    const abortSpy1 = vi.spyOn(subgraphNode.inputs[0]._listenerController!, "abort")
+    const abortSpy2 = vi.spyOn(subgraphNode.inputs[1]._listenerController!, "abort")
+
+    // Remove node
+    rootGraph.remove(subgraphNode)
+
+    // Verify abort was called on each controller
+    expect(abortSpy1).toHaveBeenCalledTimes(1)
+    expect(abortSpy2).toHaveBeenCalledTimes(1)
+  })
+})