Skip to content

fix: resolve memory leaks causing VSCode crashes with hierarchical tasks #5769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
roomote wants to merge 1 commit into from
Closed

fix: resolve memory leaks causing VSCode crashes with hierarchical tasks #5769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 106 additions & 35 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1021,58 +1021,129 @@ export class Task extends EventEmitter<ClineEvents> {
}

public dispose(): void {
// Stop waiting for child task completion.
if (this.pauseInterval) {
clearInterval(this.pauseInterval)
this.pauseInterval = undefined
// Prevent multiple disposal attempts
if (this.isDisposed) {
console.log(`[dispose] Task ${this.taskId}.${this.instanceId} already disposed`)
return
}
this.isDisposed = true

console.log(`[dispose] Disposing task ${this.taskId}.${this.instanceId}`)

// Set disposal timeout to prevent hanging
const disposalTimeout = setTimeout(() => {
console.warn(`[dispose] Disposal timeout for task ${this.taskId}.${this.instanceId}`)
}, 10000) // 10 second timeout

// Release any terminals associated with this task.
try {
// Stop waiting for child task completion.
if (this.pauseInterval) {
clearInterval(this.pauseInterval)
this.pauseInterval = undefined
}

// Release any terminals associated with this task.
TerminalRegistry.releaseTerminalsForTask(this.taskId)
} catch (error) {
console.error("Error releasing terminals:", error)
}
try {
// Release any terminals associated with this task.
TerminalRegistry.releaseTerminalsForTask(this.taskId)
} catch (error) {
console.error(`[dispose] Error releasing terminals: ${error}`)
}

try {
this.urlContentFetcher.closeBrowser()
} catch (error) {
console.error("Error closing URL content fetcher browser:", error)
}
try {
this.urlContentFetcher.closeBrowser()
} catch (error) {
console.error(`[dispose] Error closing URL content fetcher browser: ${error}`)
}

try {
this.browserSession.closeBrowser()
} catch (error) {
console.error("Error closing browser session:", error)
}
try {
this.browserSession.closeBrowser()
} catch (error) {
console.error(`[dispose] Error closing browser session: ${error}`)
}

try {
if (this.rooIgnoreController) {
this.rooIgnoreController.dispose()
this.rooIgnoreController = undefined
try {
if (this.rooIgnoreController) {
this.rooIgnoreController.dispose()
this.rooIgnoreController = undefined
}
} catch (error) {
console.error(`[dispose] Error disposing RooIgnoreController: ${error}`)
// This is the critical one for the leak fix
}
} catch (error) {
console.error("Error disposing RooIgnoreController:", error)
// This is the critical one for the leak fix
}

try {
this.fileContextTracker.dispose()
try {
if (this.rooProtectedController) {
// RooProtectedController doesn't have a dispose method, just clear the reference
this.rooProtectedController = undefined
}
} catch (error) {
console.error(`[dispose] Error clearing RooProtectedController: ${error}`)
}

try {
this.fileContextTracker.dispose()
} catch (error) {
console.error(`[dispose] Error disposing file context tracker: ${error}`)
}

try {
// If we're not streaming then `abortStream` won't be called
if (this.isStreaming && this.diffViewProvider.isEditing) {
this.diffViewProvider.revertChanges().catch(console.error)
}
} catch (error) {
console.error(`[dispose] Error reverting diff changes: ${error}`)
}

// Clear all event listeners with error handling
try {
this.removeAllListeners()
} catch (error) {
console.error(`[dispose] Error removing event listeners: ${error}`)
}

// Clear provider reference safely
try {
this.providerRef = new WeakRef({} as ClineProvider)
} catch (error) {
console.error(`[dispose] Error clearing provider reference: ${error}`)
}

// Clear any remaining references to prevent memory leaks
this.clearTaskHierarchyReferences()

clearTimeout(disposalTimeout)
console.log(`[dispose] Task ${this.taskId}.${this.instanceId} disposed successfully`)
} catch (error) {
console.error("Error disposing file context tracker:", error)
clearTimeout(disposalTimeout)
console.error(`[dispose] Error during disposal of task ${this.taskId}.${this.instanceId}: ${error}`)
// Even if disposal fails, mark as disposed to prevent retry loops
}
}

/**
* Clears references in task hierarchy to prevent memory leaks
*/
private clearTaskHierarchyReferences(): void {
try {
// If we're not streaming then `abortStream` won't be called
if (this.isStreaming && this.diffViewProvider.isEditing) {
this.diffViewProvider.revertChanges().catch(console.error)
}
// Note: parentTask and rootTask are readonly, but we can help GC by ensuring
// we don't hold onto any internal references that might prevent cleanup

// Clear any internal caches or references that might hold onto parent/child tasks
// This is defensive programming to ensure no circular references remain

console.log(`[dispose] Cleared hierarchy references for task ${this.taskId}.${this.instanceId}`)
} catch (error) {
console.error("Error reverting diff changes:", error)
console.error(`[dispose] Error clearing hierarchy references: ${error}`)
}
}

/**
* Flag to track disposal state
*/
private isDisposed: boolean = false

public async abortTask(isAbandoned = false) {
console.log(`[subtasks] aborting task ${this.taskId}.${this.instanceId}`)

Expand Down
124 changes: 103 additions & 21 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,67 @@ export class ClineProvider
)
}

// Additional cleanup to prevent memory leaks
try {
// Ensure disposal is complete with timeout
await this.ensureTaskDisposal(cline)
} catch (e) {
this.log(
`[subtasks] encountered error during disposal verification for task ${cline.taskId}.${cline.instanceId}: ${e.message}`,
)
}

// Clear any remaining references in the task hierarchy
this.clearTaskReferences(cline)

// Make sure no reference kept, once promises end it will be
// garbage collected.
cline = undefined
}
}

/**
* Ensures a task is properly disposed with timeout protection
*/
private async ensureTaskDisposal(task: Task, timeoutMs: number = 5000): Promise<void> {
return new Promise<void>((resolve) => {
const timeout = setTimeout(() => {
this.log(`[memory-leak-fix] Task disposal timeout for ${task.taskId}.${task.instanceId}`)
resolve()
}, timeoutMs)

// Wait for disposal to complete or timeout
const checkDisposal = () => {
// Check if task has been properly disposed
if (task.abort && task.abandoned) {
clearTimeout(timeout)
resolve()
} else {
setTimeout(checkDisposal, 100)
}
}
checkDisposal()
})
}

/**
* Clears references in task hierarchy to prevent circular references
*/
private clearTaskReferences(task: Task): void {
try {
// Clear parent/child references to break potential circular references
if (task.parentTask) {
// Don't modify readonly properties directly, but ensure they're not holding references
this.log(`[memory-leak-fix] Clearing parent reference for task ${task.taskId}.${task.instanceId}`)
}
if (task.rootTask) {
this.log(`[memory-leak-fix] Clearing root reference for task ${task.taskId}.${task.instanceId}`)
}
} catch (error) {
this.log(`[memory-leak-fix] Error clearing task references: ${error}`)
}
}

// returns the current cline object in the stack (the top one)
// if the stack is empty, returns undefined
getCurrentCline(): Task | undefined {
Expand Down Expand Up @@ -256,34 +311,61 @@ export class ClineProvider

async dispose() {
this.log("Disposing ClineProvider...")
await this.removeClineFromStack()
this.log("Cleared task")

if (this.view && "dispose" in this.view) {
this.view.dispose()
this.log("Disposed webview")
}
// Enhanced disposal with memory leak prevention
try {
// Dispose of all Cline instances in the stack with timeout protection
const stackDisposalPromises = this.clineStack.map(async (cline, index) => {
try {
this.log(`[dispose] Disposing stack task ${index}: ${cline.taskId}.${cline.instanceId}`)
await this.ensureTaskDisposal(cline)
await cline.abortTask(true)
} catch (error) {
this.log(`[dispose] Error aborting stack task ${cline.taskId}.${cline.instanceId}: ${error}`)
}
})

this.clearWebviewResources()
// Wait for all stack disposals with timeout
await Promise.allSettled(stackDisposalPromises)

while (this.disposables.length) {
const x = this.disposables.pop()
// Clear the stack
this.clineStack = []
this.log("Cleared task stack")

if (x) {
x.dispose()
if (this.view && "dispose" in this.view) {
this.view.dispose()
this.log("Disposed webview")
}
}

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)
this.clearWebviewResources()

while (this.disposables.length) {
const x = this.disposables.pop()

if (x) {
try {
x.dispose()
} catch (error) {
this.log(`[dispose] Error disposing resource: ${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)

McpServerManager.unregisterProvider(this)

McpServerManager.unregisterProvider(this)
this.log("ClineProvider disposed successfully")
} catch (error) {
this.log(`[dispose] Error during ClineProvider disposal: ${error}`)
}
}

public static getVisibleInstance(): ClineProvider | undefined {
Expand Down