fix: improve memory leak prevention in Task and ClineProvider disposal

- Ensure RooIgnoreController is always disposed even if errors occur
- Add finally block to guarantee reference cleanup
- Improve event listener cleanup order in ClineProvider
- Add defensive error handling for all disposal operations
- Clear array references to help garbage collection
- Fix TypeScript error in removeClineFromStack method

Fixes #7561

Author: roomote

src/core/task/Task.ts

@@ -1425,11 +1425,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	public dispose(): void {
 		console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`)
 
-		// Remove all event listeners to prevent memory leaks.
+		// Remove all event listeners first to prevent any callbacks during disposal
 		try {
 			this.removeAllListeners()
 		} catch (error) {
-			console.error("Error removing event listeners:", error)
+			console.error(`[Task#dispose] Error removing event listeners for task ${this.taskId}:`, error)
 		}
 
 		// Stop waiting for child task completion.
@@ -1438,60 +1438,78 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 			this.pauseInterval = undefined
 		}
 
+		// Unsubscribe from bridge if enabled
 		if (this.enableBridge) {
 			BridgeOrchestrator.getInstance()
 				?.unsubscribeFromTask(this.taskId)
 				.catch((error) =>
 					console.error(
-						`[Task#dispose] BridgeOrchestrator#unsubscribeFromTask() failed: ${error instanceof Error ? error.message : String(error)}`,
+						`[Task#dispose] BridgeOrchestrator#unsubscribeFromTask() failed for task ${this.taskId}: ${error instanceof Error ? error.message : String(error)}`,
 					),
 				)
 		}
 
 		// Release any terminals associated with this task.
 		try {
-			// Release any terminals associated with this task.
 			TerminalRegistry.releaseTerminalsForTask(this.taskId)
 		} catch (error) {
-			console.error("Error releasing terminals:", error)
+			console.error(`[Task#dispose] Error releasing terminals for task ${this.taskId}:`, error)
 		}
 
+		// Close browsers
 		try {
 			this.urlContentFetcher.closeBrowser()
 		} catch (error) {
-			console.error("Error closing URL content fetcher browser:", error)
+			console.error(`[Task#dispose] Error closing URL content fetcher browser for task ${this.taskId}:`, error)
 		}
 
 		try {
 			this.browserSession.closeBrowser()
 		} catch (error) {
-			console.error("Error closing browser session:", error)
+			console.error(`[Task#dispose] Error closing browser session for task ${this.taskId}:`, error)
 		}
 
-		try {
-			if (this.rooIgnoreController) {
+		// CRITICAL: Always dispose RooIgnoreController to prevent memory leaks
+		// This must be done even if it throws an error
+		if (this.rooIgnoreController) {
+			try {
 				this.rooIgnoreController.dispose()
+			} catch (error) {
+				console.error(
+					`[Task#dispose] CRITICAL: Error disposing RooIgnoreController for task ${this.taskId}:`,
+					error,
+				)
+			} finally {
+				// Always clear the reference to allow garbage collection
 				this.rooIgnoreController = undefined
 			}
-		} catch (error) {
-			console.error("Error disposing RooIgnoreController:", error)
-			// This is the critical one for the leak fix.
 		}
 
+		// Dispose file context tracker
 		try {
 			this.fileContextTracker.dispose()
 		} catch (error) {
-			console.error("Error disposing file context tracker:", error)
+			console.error(`[Task#dispose] Error disposing file context tracker for task ${this.taskId}:`, error)
 		}
 
+		// Revert any pending diff changes
 		try {
-			// If we're not streaming then `abortStream` won't be called.
 			if (this.isStreaming && this.diffViewProvider.isEditing) {
-				this.diffViewProvider.revertChanges().catch(console.error)
+				this.diffViewProvider
+					.revertChanges()
+					.catch((error) =>
+						console.error(`[Task#dispose] Error reverting diff changes for task ${this.taskId}:`, error),
+					)
 			}
 		} catch (error) {
-			console.error("Error reverting diff changes:", error)
+			console.error(`[Task#dispose] Error checking/reverting diff changes for task ${this.taskId}:`, error)
 		}
+
+		// Clear any remaining references to help garbage collection
+		this.assistantMessageContent = []
+		this.userMessageContent = []
+		this.apiConversationHistory = []
+		this.clineMessages = []
 	}
 
 	public async abortTask(isAbandoned = false) {

src/core/task/__tests__/Task.dispose.test.ts

@@ -117,8 +117,11 @@ describe("Task dispose method", () => {
 		// Call dispose - should not throw
 		expect(() => task.dispose()).not.toThrow()
 
-		// Verify error was logged
-		expect(consoleErrorSpy).toHaveBeenCalledWith("Error removing event listeners:", expect.any(Error))
+		// Verify error was logged with the improved format
+		expect(consoleErrorSpy).toHaveBeenCalledWith(
+			`[Task#dispose] Error removing event listeners for task ${task.taskId}:`,
+			expect.any(Error),
+		)
 
 		// Restore
 		task.removeAllListeners = originalRemoveAllListeners

src/core/webview/ClineProvider.ts

@@ -350,32 +350,43 @@ export class ClineProvider
 		}
 
 		// Pop the top Cline instance from the stack.
-		let task = this.clineStack.pop()
+		const task = this.clineStack.pop()
 
 		if (task) {
-			task.emit(RooCodeEventName.TaskUnfocused)
+			const taskId = task.taskId
+			const instanceId = task.instanceId
 
+			// Emit unfocused event before cleanup
 			try {
-				// Abort the running task and set isAbandoned to true so
-				// all running promises will exit as well.
-				await task.abortTask(true)
-			} catch (e) {
-				this.log(
-					`[ClineProvider#removeClineFromStack] abortTask() failed ${task.taskId}.${task.instanceId}: ${e.message}`,
-				)
+				task.emit(RooCodeEventName.TaskUnfocused)
+			} catch (error) {
+				this.log(`[ClineProvider#removeClineFromStack] Error emitting TaskUnfocused for ${taskId}: ${error}`)
 			}
 
-			// Remove event listeners before clearing the reference.
+			// Remove event listeners BEFORE aborting to prevent any callbacks during cleanup
 			const cleanupFunctions = this.taskEventListeners.get(task)
-
 			if (cleanupFunctions) {
-				cleanupFunctions.forEach((cleanup) => cleanup())
+				cleanupFunctions.forEach((cleanup) => {
+					try {
+						cleanup()
+					} catch (error) {
+						this.log(
+							`[ClineProvider#removeClineFromStack] Error cleaning up event listener for ${taskId}: ${error}`,
+						)
+					}
+				})
 				this.taskEventListeners.delete(task)
 			}
 
-			// Make sure no reference kept, once promises end it will be
-			// garbage collected.
-			task = undefined
+			try {
+				// Abort the running task and set isAbandoned to true so
+				// all running promises will exit as well.
+				await task.abortTask(true)
+			} catch (e) {
+				this.log(
+					`[ClineProvider#removeClineFromStack] abortTask() failed ${taskId}.${instanceId}: ${e.message}`,
+				)
+			}
 		}
 	}
 
@@ -418,13 +429,31 @@ export class ClineProvider
 	async dispose() {
 		this.log("Disposing ClineProvider...")
 
+		// Clear all event listeners for all tasks first
+		for (const task of this.clineStack) {
+			const cleanupFunctions = this.taskEventListeners.get(task)
+			if (cleanupFunctions) {
+				cleanupFunctions.forEach((cleanup) => {
+					try {
+						cleanup()
+					} catch (error) {
+						this.log(`[ClineProvider#dispose] Error cleaning up event listener: ${error}`)
+					}
+				})
+				this.taskEventListeners.delete(task)
+			}
+		}
+
 		// Clear all tasks from the stack.
 		while (this.clineStack.length > 0) {
 			await this.removeClineFromStack()
 		}
 
 		this.log("Cleared all tasks")
 
+		// Clear the task event listeners map completely
+		this.taskEventListeners = new WeakMap()
+
 		if (this.view && "dispose" in this.view) {
 			this.view.dispose()
 			this.log("Disposed webview")
@@ -441,21 +470,61 @@ export class ClineProvider
 			const x = this.disposables.pop()
 
 			if (x) {
-				x.dispose()
+				try {
+					x.dispose()
+				} catch (error) {
+					this.log(`[ClineProvider#dispose] Error disposing disposable: ${error}`)
+				}
+			}
+		}
+
+		// Clean up workspace tracker
+		if (this._workspaceTracker) {
+			try {
+				this._workspaceTracker.dispose()
+			} catch (error) {
+				this.log(`[ClineProvider#dispose] Error disposing workspace tracker: ${error}`)
+			}
+			this._workspaceTracker = undefined
+		}
+
+		// Unregister from MCP hub
+		if (this.mcpHub) {
+			try {
+				await this.mcpHub.unregisterClient()
+			} catch (error) {
+				this.log(`[ClineProvider#dispose] Error unregistering MCP client: ${error}`)
+			}
+			this.mcpHub = undefined
+		}
+
+		// Clean up marketplace manager
+		if (this.marketplaceManager) {
+			try {
+				this.marketplaceManager.cleanup()
+			} catch (error) {
+				this.log(`[ClineProvider#dispose] Error cleaning up marketplace manager: ${error}`)
+			}
+		}
+
+		// Dispose custom modes manager
+		if (this.customModesManager) {
+			try {
+				this.customModesManager.dispose()
+			} catch (error) {
+				this.log(`[ClineProvider#dispose] Error disposing custom modes manager: ${error}`)
 			}
 		}
 
-		this._workspaceTracker?.dispose()
-		this._workspaceTracker = undefined
-		await this.mcpHub?.unregisterClient()
-		this.mcpHub = undefined
-		this.marketplaceManager?.cleanup()
-		this.customModesManager?.dispose()
 		this.log("Disposed all disposables")
 		ClineProvider.activeInstances.delete(this)
 
 		// Clean up any event listeners attached to this provider
-		this.removeAllListeners()
+		try {
+			this.removeAllListeners()
+		} catch (error) {
+			this.log(`[ClineProvider#dispose] Error removing provider event listeners: ${error}`)
+		}
 
 		McpServerManager.unregisterProvider(this)
 	}