Skip to content

Commit 6b68b16

Browse files
committed
fix: prevent "TextEditor is closed/disposed" warnings
- Add disposal tracking and checks to DecorationController - Add try-catch blocks around TextEditor property access - Properly dispose decoration controllers in DiffViewProvider - Add comprehensive tests for disposal handling - Protect editor access in getEnvironmentDetails and registerCommands Fixes #5954
1 parent 37300ef commit 6b68b16

File tree

5 files changed

+405
-39
lines changed

5 files changed

+405
-39
lines changed

src/activate/registerCommands.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,16 @@ export const openClineInNewTab = async ({ context, outputChannel }: Omit<Registe
238238
}
239239

240240
const tabProvider = new ClineProvider(context, outputChannel, "editor", contextProxy, codeIndexManager, mdmService)
241-
const lastCol = Math.max(...vscode.window.visibleTextEditors.map((editor) => editor.viewColumn || 0))
241+
const lastCol = Math.max(
242+
...vscode.window.visibleTextEditors.map((editor) => {
243+
try {
244+
return editor.viewColumn || 0
245+
} catch {
246+
// Editor might be disposed
247+
return 0
248+
}
249+
}),
250+
)
242251

243252
// Check if there are any visible text editors, otherwise open a new group
244253
// to the right.

src/core/environment/getEnvironmentDetails.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,15 @@ export async function getEnvironmentDetails(cline: Task, includeFileDetails: boo
3737
details += "\n\n# VSCode Visible Files"
3838

3939
const visibleFilePaths = vscode.window.visibleTextEditors
40-
?.map((editor) => editor.document?.uri?.fsPath)
41-
.filter(Boolean)
40+
?.map((editor) => {
41+
try {
42+
return editor.document?.uri?.fsPath
43+
} catch {
44+
// Editor might be disposed
45+
return null
46+
}
47+
})
48+
.filter((fsPath): fsPath is string => fsPath !== null && fsPath !== undefined)
4249
.map((absolutePath) => path.relative(cline.cwd, absolutePath))
4350
.slice(0, maxWorkspaceFiles)
4451

src/integrations/editor/DecorationController.ts

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export class DecorationController {
1919
private decorationType: DecorationType
2020
private editor: vscode.TextEditor
2121
private ranges: vscode.Range[] = []
22+
private isDisposed: boolean = false
2223

2324
constructor(decorationType: DecorationType, editor: vscode.TextEditor) {
2425
this.decorationType = decorationType
@@ -35,8 +36,13 @@ export class DecorationController {
3536
}
3637

3738
addLines(startIndex: number, numLines: number) {
38-
// Guard against invalid inputs
39-
if (startIndex < 0 || numLines <= 0) {
39+
// Guard against invalid inputs or disposed state
40+
if (startIndex < 0 || numLines <= 0 || this.isDisposed) {
41+
return
42+
}
43+
44+
// Check if editor is still valid before using it
45+
if (!this.isEditorValid()) {
4046
return
4147
}
4248

@@ -48,15 +54,43 @@ export class DecorationController {
4854
this.ranges.push(new vscode.Range(startIndex, 0, endLine, Number.MAX_SAFE_INTEGER))
4955
}
5056

51-
this.editor.setDecorations(this.getDecoration(), this.ranges)
57+
try {
58+
this.editor.setDecorations(this.getDecoration(), this.ranges)
59+
} catch (error) {
60+
// Editor was disposed between check and use
61+
console.debug("DecorationController: Failed to set decorations, editor may be disposed", error)
62+
}
5263
}
5364

5465
clear() {
66+
if (this.isDisposed) {
67+
return
68+
}
69+
5570
this.ranges = []
56-
this.editor.setDecorations(this.getDecoration(), this.ranges)
71+
72+
if (!this.isEditorValid()) {
73+
return
74+
}
75+
76+
try {
77+
this.editor.setDecorations(this.getDecoration(), this.ranges)
78+
} catch (error) {
79+
// Editor was disposed between check and use
80+
console.debug("DecorationController: Failed to clear decorations, editor may be disposed", error)
81+
}
5782
}
5883

5984
updateOverlayAfterLine(line: number, totalLines: number) {
85+
if (this.isDisposed) {
86+
return
87+
}
88+
89+
// Check if editor is still valid before using it
90+
if (!this.isEditorValid()) {
91+
return
92+
}
93+
6094
// Remove any existing ranges that start at or after the current line
6195
this.ranges = this.ranges.filter((range) => range.end.line < line)
6296

@@ -71,11 +105,55 @@ export class DecorationController {
71105
}
72106

73107
// Apply the updated decorations
74-
this.editor.setDecorations(this.getDecoration(), this.ranges)
108+
try {
109+
this.editor.setDecorations(this.getDecoration(), this.ranges)
110+
} catch (error) {
111+
// Editor was disposed between check and use
112+
console.debug("DecorationController: Failed to update overlay, editor may be disposed", error)
113+
}
75114
}
76115

77116
setActiveLine(line: number) {
117+
if (this.isDisposed) {
118+
return
119+
}
120+
121+
// Check if editor is still valid before using it
122+
if (!this.isEditorValid()) {
123+
return
124+
}
125+
78126
this.ranges = [new vscode.Range(line, 0, line, Number.MAX_SAFE_INTEGER)]
79-
this.editor.setDecorations(this.getDecoration(), this.ranges)
127+
128+
try {
129+
this.editor.setDecorations(this.getDecoration(), this.ranges)
130+
} catch (error) {
131+
// Editor was disposed between check and use
132+
console.debug("DecorationController: Failed to set active line, editor may be disposed", error)
133+
}
134+
}
135+
136+
/**
137+
* Checks if the editor is still valid and not disposed
138+
*/
139+
private isEditorValid(): boolean {
140+
try {
141+
// Try to access a property that would throw if disposed
142+
// The document property is a good indicator
143+
const _ = this.editor.document
144+
return true
145+
} catch {
146+
// Editor is disposed
147+
this.isDisposed = true
148+
return false
149+
}
150+
}
151+
152+
/**
153+
* Marks this controller as disposed
154+
*/
155+
dispose() {
156+
this.isDisposed = true
157+
this.clear()
80158
}
81159
}

src/integrations/editor/DiffViewProvider.ts

Lines changed: 68 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,12 @@ export class DiffViewProvider {
9999
this.fadedOverlayController = new DecorationController("fadedOverlay", this.activeDiffEditor)
100100
this.activeLineController = new DecorationController("activeLine", this.activeDiffEditor)
101101
// Apply faded overlay to all lines initially.
102-
this.fadedOverlayController.addLines(0, this.activeDiffEditor.document.lineCount)
103-
this.scrollEditorToLine(0) // Will this crash for new files?
102+
try {
103+
this.fadedOverlayController.addLines(0, this.activeDiffEditor.document.lineCount)
104+
this.scrollEditorToLine(0)
105+
} catch (error) {
106+
console.debug("DiffViewProvider: Failed to initialize decorations", error)
107+
}
104108
this.streamedLines = []
105109
}
106110

@@ -117,16 +121,28 @@ export class DiffViewProvider {
117121
}
118122

119123
const diffEditor = this.activeDiffEditor
120-
const document = diffEditor?.document
124+
125+
// Check if editor is still valid
126+
let document: vscode.TextDocument | undefined
127+
try {
128+
document = diffEditor?.document
129+
} catch {
130+
throw new Error("Text editor is disposed, unable to edit file...")
131+
}
121132

122133
if (!diffEditor || !document) {
123134
throw new Error("User closed text editor, unable to edit file...")
124135
}
125136

126137
// Place cursor at the beginning of the diff editor to keep it out of
127138
// the way of the stream animation, but do this without stealing focus
128-
const beginningOfDocument = new vscode.Position(0, 0)
129-
diffEditor.selection = new vscode.Selection(beginningOfDocument, beginningOfDocument)
139+
try {
140+
const beginningOfDocument = new vscode.Position(0, 0)
141+
diffEditor.selection = new vscode.Selection(beginningOfDocument, beginningOfDocument)
142+
} catch (error) {
143+
// Editor might be disposed, continue with the update
144+
console.debug("DiffViewProvider: Failed to set selection", error)
145+
}
130146

131147
const endLine = accumulatedLines.length
132148
// Replace all content up to the current line with accumulated lines.
@@ -176,12 +192,15 @@ export class DiffViewProvider {
176192
await vscode.workspace.applyEdit(finalEdit)
177193

178194
// Clear all decorations at the end (after applying final edit).
179-
this.fadedOverlayController.clear()
180-
this.activeLineController.clear()
195+
this.fadedOverlayController?.clear()
196+
this.activeLineController?.clear()
181197
}
182198
}
183199

184-
async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
200+
async saveChanges(
201+
diagnosticsEnabled: boolean = true,
202+
writeDelayMs: number = DEFAULT_WRITE_DELAY_MS,
203+
): Promise<{
185204
newProblemsMessage: string | undefined
186205
userEdits: string | undefined
187206
finalContent: string | undefined
@@ -216,22 +235,22 @@ export class DiffViewProvider {
216235
// and can address them accordingly. If problems don't change immediately after
217236
// applying a fix, won't be notified, which is generally fine since the
218237
// initial fix is usually correct and it may just take time for linters to catch up.
219-
238+
220239
let newProblemsMessage = ""
221-
240+
222241
if (diagnosticsEnabled) {
223242
// Add configurable delay to allow linters time to process and clean up issues
224243
// like unused imports (especially important for Go and other languages)
225244
// Ensure delay is non-negative
226245
const safeDelayMs = Math.max(0, writeDelayMs)
227-
246+
228247
try {
229248
await delay(safeDelayMs)
230249
} catch (error) {
231250
// Log error but continue - delay failure shouldn't break the save operation
232251
console.warn(`Failed to apply write delay: ${error}`)
233252
}
234-
253+
235254
const postDiagnostics = vscode.languages.getDiagnostics()
236255

237256
const newProblems = await diagnosticsToProblemsString(
@@ -549,13 +568,19 @@ export class DiffViewProvider {
549568
}
550569

551570
private scrollEditorToLine(line: number) {
552-
if (this.activeDiffEditor) {
553-
const scrollLine = line + 4
571+
if (!this.activeDiffEditor) {
572+
return
573+
}
554574

575+
try {
576+
const scrollLine = line + 4
555577
this.activeDiffEditor.revealRange(
556578
new vscode.Range(scrollLine, 0, scrollLine, 0),
557579
vscode.TextEditorRevealType.InCenter,
558580
)
581+
} catch (error) {
582+
// Editor might be disposed
583+
console.debug("DiffViewProvider: Failed to scroll editor", error)
559584
}
560585
}
561586

@@ -564,25 +589,30 @@ export class DiffViewProvider {
564589
return
565590
}
566591

567-
const currentContent = this.activeDiffEditor.document.getText()
568-
const diffs = diff.diffLines(this.originalContent || "", currentContent)
592+
try {
593+
const currentContent = this.activeDiffEditor.document.getText()
594+
const diffs = diff.diffLines(this.originalContent || "", currentContent)
569595

570-
let lineCount = 0
596+
let lineCount = 0
571597

572-
for (const part of diffs) {
573-
if (part.added || part.removed) {
574-
// Found the first diff, scroll to it without stealing focus.
575-
this.activeDiffEditor.revealRange(
576-
new vscode.Range(lineCount, 0, lineCount, 0),
577-
vscode.TextEditorRevealType.InCenter,
578-
)
598+
for (const part of diffs) {
599+
if (part.added || part.removed) {
600+
// Found the first diff, scroll to it without stealing focus.
601+
this.activeDiffEditor.revealRange(
602+
new vscode.Range(lineCount, 0, lineCount, 0),
603+
vscode.TextEditorRevealType.InCenter,
604+
)
579605

580-
return
581-
}
606+
return
607+
}
582608

583-
if (!part.removed) {
584-
lineCount += part.count || 0
609+
if (!part.removed) {
610+
lineCount += part.count || 0
611+
}
585612
}
613+
} catch (error) {
614+
// Editor might be disposed
615+
console.debug("DiffViewProvider: Failed to scroll to first diff", error)
586616
}
587617
}
588618

@@ -599,15 +629,23 @@ export class DiffViewProvider {
599629
}
600630

601631
async reset(): Promise<void> {
632+
// Dispose decoration controllers before clearing references
633+
if (this.fadedOverlayController) {
634+
this.fadedOverlayController.dispose()
635+
this.fadedOverlayController = undefined
636+
}
637+
if (this.activeLineController) {
638+
this.activeLineController.dispose()
639+
this.activeLineController = undefined
640+
}
641+
602642
await this.closeAllDiffViews()
603643
this.editType = undefined
604644
this.isEditing = false
605645
this.originalContent = undefined
606646
this.createdDirs = []
607647
this.documentWasOpen = false
608648
this.activeDiffEditor = undefined
609-
this.fadedOverlayController = undefined
610-
this.activeLineController = undefined
611649
this.streamedLines = []
612650
this.preDiagnostics = []
613651
}

0 commit comments

Comments
 (0)