diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 655983db20..8c40018a82 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1425,11 +1425,11 @@ export class Task extends EventEmitter 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 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) { diff --git a/src/core/task/__tests__/Task.dispose.test.ts b/src/core/task/__tests__/Task.dispose.test.ts index 850b050fb8..b1a023b838 100644 --- a/src/core/task/__tests__/Task.dispose.test.ts +++ b/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 diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index a8d64d6600..f32c1e2470 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -350,10 +350,33 @@ 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 { + task.emit(RooCodeEventName.TaskUnfocused) + } catch (error) { + this.log(`[ClineProvider#removeClineFromStack] Error emitting TaskUnfocused for ${taskId}: ${error}`) + } + + // Remove event listeners BEFORE aborting to prevent any callbacks during cleanup + const cleanupFunctions = this.taskEventListeners.get(task) + if (cleanupFunctions) { + cleanupFunctions.forEach((cleanup) => { + try { + cleanup() + } catch (error) { + this.log( + `[ClineProvider#removeClineFromStack] Error cleaning up event listener for ${taskId}: ${error}`, + ) + } + }) + this.taskEventListeners.delete(task) + } try { // Abort the running task and set isAbandoned to true so @@ -361,21 +384,9 @@ export class ClineProvider await task.abortTask(true) } catch (e) { this.log( - `[ClineProvider#removeClineFromStack] abortTask() failed ${task.taskId}.${task.instanceId}: ${e.message}`, + `[ClineProvider#removeClineFromStack] abortTask() failed ${taskId}.${instanceId}: ${e.message}`, ) } - - // Remove event listeners before clearing the reference. - const cleanupFunctions = this.taskEventListeners.get(task) - - if (cleanupFunctions) { - cleanupFunctions.forEach((cleanup) => cleanup()) - this.taskEventListeners.delete(task) - } - - // Make sure no reference kept, once promises end it will be - // garbage collected. - task = undefined } } @@ -418,6 +429,21 @@ 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() @@ -425,6 +451,9 @@ export class ClineProvider 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) }