Skip to content

Commit a23d4da

Browse files
author
Benjamin Pasero
authored
Dangling text file models of deleted files hanging around in memory (microsoft#98154)
* Dangling text file models of deleted files hanging around in memory (fix microsoft#98057) * address feedback
1 parent 63d6a65 commit a23d4da

7 files changed

Lines changed: 163 additions & 43 deletions

File tree

‎extensions/vscode-api-tests/src/singlefolder-tests/languages.test.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,31 +29,34 @@ suite('vscode API - languages', () => {
2929
const doc = await vscode.workspace.openTextDocument(file);
3030
const langIdNow = doc.languageId;
3131
let clock = 0;
32+
const disposables: vscode.Disposable[] = [];
3233

3334
let close = new Promise(resolve => {
34-
vscode.workspace.onDidCloseTextDocument(e => {
35+
disposables.push(vscode.workspace.onDidCloseTextDocument(e => {
3536
if (e === doc) {
3637
assert.equal(doc.languageId, langIdNow);
3738
assert.equal(clock, 0);
3839
clock += 1;
3940
resolve();
4041
}
41-
});
42+
}));
4243
});
4344
let open = new Promise(resolve => {
44-
vscode.workspace.onDidOpenTextDocument(e => {
45+
disposables.push(vscode.workspace.onDidOpenTextDocument(e => {
4546
if (e === doc) { // same instance!
4647
assert.equal(doc.languageId, 'json');
4748
assert.equal(clock, 1);
4849
clock += 1;
4950
resolve();
5051
}
51-
});
52+
}));
5253
});
5354
let change = vscode.languages.setTextDocumentLanguage(doc, 'json');
5455
await Promise.all([change, close, open]);
5556
assert.equal(clock, 2);
5657
assert.equal(doc.languageId, 'json');
58+
disposables.forEach(disposable => disposable.dispose());
59+
disposables.length = 0;
5760
});
5861

5962
test('setTextDocumentLanguage -> error when language does not exist', async function () {

‎src/vs/workbench/api/browser/mainThreadDocuments.ts‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,18 @@ import { URI, UriComponents } from 'vs/base/common/uri';
1010
import { ITextModel } from 'vs/editor/common/model';
1111
import { IModelService, shouldSynchronizeModel } from 'vs/editor/common/services/modelService';
1212
import { ITextModelService } from 'vs/editor/common/services/resolverService';
13-
import { IFileService } from 'vs/platform/files/common/files';
13+
import { IFileService, FileOperation } from 'vs/platform/files/common/files';
1414
import { MainThreadDocumentsAndEditors } from 'vs/workbench/api/browser/mainThreadDocumentsAndEditors';
1515
import { ExtHostContext, ExtHostDocumentsShape, IExtHostContext, MainThreadDocumentsShape } from 'vs/workbench/api/common/extHost.protocol';
1616
import { ITextEditorModel } from 'vs/workbench/common/editor';
1717
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
1818
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
19-
import { toLocalResource } from 'vs/base/common/resources';
19+
import { toLocalResource, isEqualOrParent } from 'vs/base/common/resources';
20+
import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
2021

2122
export class BoundModelReferenceCollection {
2223

23-
private _data = new Array<{ length: number, dispose(): void }>();
24+
private _data = new Array<{ uri: URI, length: number, dispose(): void }>();
2425
private _length = 0;
2526

2627
constructor(
@@ -34,10 +35,18 @@ export class BoundModelReferenceCollection {
3435
this._data = dispose(this._data);
3536
}
3637

37-
add(ref: IReference<ITextEditorModel>): void {
38+
remove(uri: URI): void {
39+
for (const entry of [...this._data] /* copy array because dispose will modify it */) {
40+
if (isEqualOrParent(entry.uri, uri)) {
41+
entry.dispose();
42+
}
43+
}
44+
}
45+
46+
add(uri: URI, ref: IReference<ITextEditorModel>): void {
3847
const length = ref.object.textEditorModel.getValueLength();
3948
let handle: any;
40-
let entry: { length: number, dispose(): void };
49+
let entry: { uri: URI, length: number, dispose(): void };
4150
const dispose = () => {
4251
const idx = this._data.indexOf(entry);
4352
if (idx >= 0) {
@@ -48,7 +57,7 @@ export class BoundModelReferenceCollection {
4857
}
4958
};
5059
handle = setTimeout(dispose, this._maxAge);
51-
entry = { length, dispose };
60+
entry = { uri, length, dispose };
5261

5362
this._data.push(entry);
5463
this._length += length;
@@ -74,7 +83,7 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
7483
private _modelToDisposeMap: { [modelUrl: string]: IDisposable; };
7584
private readonly _proxy: ExtHostDocumentsShape;
7685
private readonly _modelIsSynced = new Set<string>();
77-
private _modelReferenceCollection = new BoundModelReferenceCollection();
86+
private readonly _modelReferenceCollection = new BoundModelReferenceCollection();
7887

7988
constructor(
8089
documentsAndEditors: MainThreadDocumentsAndEditors,
@@ -83,7 +92,8 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
8392
@ITextFileService textFileService: ITextFileService,
8493
@IFileService fileService: IFileService,
8594
@ITextModelService textModelResolverService: ITextModelService,
86-
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService
95+
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,
96+
@IWorkingCopyFileService workingCopyFileService: IWorkingCopyFileService
8797
) {
8898
this._modelService = modelService;
8999
this._textModelResolverService = textModelResolverService;
@@ -109,6 +119,12 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
109119
}
110120
}));
111121

122+
this._toDispose.add(workingCopyFileService.onDidRunWorkingCopyFileOperation(e => {
123+
if (e.source && (e.operation === FileOperation.MOVE || e.operation === FileOperation.DELETE)) {
124+
this._modelReferenceCollection.remove(e.source);
125+
}
126+
}));
127+
112128
this._modelToDisposeMap = Object.create(null);
113129
}
114130

@@ -199,7 +215,7 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
199215

200216
private _handleAsResourceInput(uri: URI): Promise<boolean> {
201217
return this._textModelResolverService.createModelReference(uri).then(ref => {
202-
this._modelReferenceCollection.add(ref);
218+
this._modelReferenceCollection.add(uri, ref);
203219
const result = !!ref.object;
204220
return result;
205221
});

‎src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor
2727
import { IPanelService } from 'vs/workbench/services/panel/common/panelService';
2828
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
2929
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
30+
import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
3031

3132
namespace delta {
3233

@@ -326,11 +327,12 @@ export class MainThreadDocumentsAndEditors {
326327
@IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService,
327328
@IBulkEditService bulkEditService: IBulkEditService,
328329
@IPanelService panelService: IPanelService,
329-
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService
330+
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,
331+
@IWorkingCopyFileService workingCopyFileService: IWorkingCopyFileService
330332
) {
331333
this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostDocumentsAndEditors);
332334

333-
const mainThreadDocuments = this._toDispose.add(new MainThreadDocuments(this, extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService));
335+
const mainThreadDocuments = this._toDispose.add(new MainThreadDocuments(this, extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService, workingCopyFileService));
334336
extHostContext.set(MainContext.MainThreadDocuments, mainThreadDocuments);
335337

336338
const mainThreadTextEditors = this._toDispose.add(new MainThreadTextEditors(this, extHostContext, codeEditorService, bulkEditService, this._editorService, this._editorGroupService));

‎src/vs/workbench/test/browser/api/mainThreadDocuments.test.ts‎

Lines changed: 96 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as assert from 'assert';
77
import { BoundModelReferenceCollection } from 'vs/workbench/api/browser/mainThreadDocuments';
88
import { createTextModel } from 'vs/editor/test/common/editorTestUtils';
99
import { timeout } from 'vs/base/common/async';
10+
import { URI } from 'vs/base/common/uri';
1011

1112
suite('BoundModelReferenceCollection', () => {
1213

@@ -20,12 +21,14 @@ suite('BoundModelReferenceCollection', () => {
2021

2122
let didDispose = false;
2223

23-
col.add({
24-
object: <any>{ textEditorModel: createTextModel('farboo') },
25-
dispose() {
26-
didDispose = true;
27-
}
28-
});
24+
col.add(
25+
URI.parse('test://farboo'),
26+
{
27+
object: <any>{ textEditorModel: createTextModel('farboo') },
28+
dispose() {
29+
didDispose = true;
30+
}
31+
});
2932

3033
await timeout(30);
3134
assert.equal(didDispose, true);
@@ -35,27 +38,95 @@ suite('BoundModelReferenceCollection', () => {
3538

3639
let disposed: number[] = [];
3740

38-
col.add({
39-
object: <any>{ textEditorModel: createTextModel('farboo') },
40-
dispose() {
41-
disposed.push(0);
42-
}
43-
});
44-
col.add({
45-
object: <any>{ textEditorModel: createTextModel('boofar') },
46-
dispose() {
47-
disposed.push(1);
48-
}
49-
});
50-
51-
col.add({
52-
object: <any>{ textEditorModel: createTextModel(new Array(71).join('x')) },
53-
dispose() {
54-
disposed.push(2);
55-
}
56-
});
41+
col.add(
42+
URI.parse('test://farboo'),
43+
{
44+
object: <any>{ textEditorModel: createTextModel('farboo') },
45+
dispose() {
46+
disposed.push(0);
47+
}
48+
});
49+
50+
col.add(
51+
URI.parse('test://boofar'),
52+
{
53+
object: <any>{ textEditorModel: createTextModel('boofar') },
54+
dispose() {
55+
disposed.push(1);
56+
}
57+
});
58+
59+
col.add(
60+
URI.parse('test://xxxxxxx'),
61+
{
62+
object: <any>{ textEditorModel: createTextModel(new Array(71).join('x')) },
63+
dispose() {
64+
disposed.push(2);
65+
}
66+
});
5767

5868
assert.deepEqual(disposed, [0, 1]);
5969
});
6070

71+
test('dispose uri', () => {
72+
73+
let disposed: number[] = [];
74+
75+
col.add(
76+
URI.parse('test:///farboo'),
77+
{
78+
object: <any>{ textEditorModel: createTextModel('farboo') },
79+
dispose() {
80+
disposed.push(0);
81+
}
82+
});
83+
84+
col.add(
85+
URI.parse('test:///boofar'),
86+
{
87+
object: <any>{ textEditorModel: createTextModel('boofar') },
88+
dispose() {
89+
disposed.push(1);
90+
}
91+
});
92+
93+
col.add(
94+
URI.parse('test:///boo/far1'),
95+
{
96+
object: <any>{ textEditorModel: createTextModel('boo/far1') },
97+
dispose() {
98+
disposed.push(2);
99+
}
100+
});
101+
102+
col.add(
103+
URI.parse('test:///boo/far2'),
104+
{
105+
object: <any>{ textEditorModel: createTextModel('boo/far2') },
106+
dispose() {
107+
disposed.push(3);
108+
}
109+
});
110+
111+
col.add(
112+
URI.parse('test:///boo1/far'),
113+
{
114+
object: <any>{ textEditorModel: createTextModel('boo1/far') },
115+
dispose() {
116+
disposed.push(4);
117+
}
118+
});
119+
120+
col.remove(URI.parse('test:///unknown'));
121+
assert.equal(disposed.length, 0);
122+
123+
col.remove(URI.parse('test:///farboo'));
124+
assert.deepEqual(disposed, [0]);
125+
126+
disposed = [];
127+
128+
col.remove(URI.parse('test:///boo'));
129+
assert.deepEqual(disposed, [2, 3]);
130+
});
131+
61132
});

‎src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { NullLogService } from 'vs/platform/log/common/log';
2525
import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService';
2626
import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService';
2727
import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
28-
import { TestTextResourcePropertiesService } from 'vs/workbench/test/common/workbenchTestServices';
28+
import { TestTextResourcePropertiesService, TestWorkingCopyFileService } from 'vs/workbench/test/common/workbenchTestServices';
2929

3030
suite('MainThreadDocumentsAndEditors', () => {
3131

@@ -88,7 +88,8 @@ suite('MainThreadDocumentsAndEditors', () => {
8888
return undefined;
8989
}
9090
},
91-
TestEnvironmentService
91+
TestEnvironmentService,
92+
new TestWorkingCopyFileService()
9293
);
9394
});
9495

‎src/vs/workbench/test/browser/api/mainThreadEditors.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ suite('MainThreadEditors', () => {
104104
};
105105
});
106106
services.set(IWorkingCopyFileService, new class extends mock<IWorkingCopyFileService>() {
107+
onDidRunWorkingCopyFileOperation = Event.None;
107108
move(source: URI, target: URI) {
108109
movedResources.set(source, target);
109110
return Promise.resolve(Object.create(null));

‎src/vs/workbench/test/common/workbenchTestServices.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@ import { IWorkspaceIdentifier, ISingleFolderWorkspaceIdentifier, isSingleFolderW
1414
import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService';
1515
import { isLinux, isMacintosh } from 'vs/base/common/platform';
1616
import { InMemoryStorageService, IWillSaveStateEvent } from 'vs/platform/storage/common/storage';
17-
import { WorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
17+
import { WorkingCopyService, IWorkingCopy } from 'vs/workbench/services/workingCopy/common/workingCopyService';
1818
import { NullExtensionService } from 'vs/workbench/services/extensions/common/extensions';
19+
import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
20+
import { IDisposable, Disposable } from 'vs/base/common/lifecycle';
21+
import { FileOperation, IFileStatWithMetadata } from 'vs/platform/files/common/files';
1922

2023
export class TestTextResourcePropertiesService implements ITextResourcePropertiesService {
2124

@@ -121,6 +124,29 @@ export class TestStorageService extends InMemoryStorageService {
121124

122125
export class TestWorkingCopyService extends WorkingCopyService { }
123126

127+
export class TestWorkingCopyFileService implements IWorkingCopyFileService {
128+
129+
_serviceBrand: undefined;
130+
131+
onWillRunWorkingCopyFileOperation: Event<WorkingCopyFileEvent> = Event.None;
132+
onDidFailWorkingCopyFileOperation: Event<WorkingCopyFileEvent> = Event.None;
133+
onDidRunWorkingCopyFileOperation: Event<WorkingCopyFileEvent> = Event.None;
134+
135+
addFileOperationParticipant(participant: IWorkingCopyFileOperationParticipant): IDisposable { return Disposable.None; }
136+
137+
async runFileOperationParticipants(target: URI, source: URI | undefined, operation: FileOperation): Promise<void> { }
138+
139+
async delete(resource: URI, options?: { useTrash?: boolean | undefined; recursive?: boolean | undefined; } | undefined): Promise<void> { }
140+
141+
registerWorkingCopyProvider(provider: (resourceOrFolder: URI) => IWorkingCopy[]): IDisposable { return Disposable.None; }
142+
143+
getDirty(resource: URI): IWorkingCopy[] { return []; }
144+
145+
move(source: URI, target: URI, overwrite?: boolean | undefined): Promise<IFileStatWithMetadata> { throw new Error('Method not implemented.'); }
146+
147+
copy(source: URI, target: URI, overwrite?: boolean | undefined): Promise<IFileStatWithMetadata> { throw new Error('Method not implemented.'); }
148+
}
149+
124150
export function mock<T>(): Ctor<T> {
125151
return function () { } as any;
126152
}

0 commit comments

Comments
 (0)