maxmustermann/vscode
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Dangling text file models of deleted files hanging around in memory (#98154)

Browse source 
* Dangling text file models of deleted files hanging around in memory (fix #98057)

* address feedback
This commit is contained in:
Benjamin Pasero 2020-05-26 16:29:12 +02:00 committed by GitHub
parent 63d6a654ec
commit a23d4daefa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 162 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
extensions/vscode-api-tests/src/singlefolder-tests/languages.test.ts
View file

@ -29,31 +29,34 @@ suite('vscode API - languages', () => {
const doc = await vscode.workspace.openTextDocument(file);
const langIdNow = doc.languageId;
let clock = 0;
const disposables: vscode.Disposable[] = [];
let close = new Promise(resolve => {
vscode.workspace.onDidCloseTextDocument(e => {
disposables.push(vscode.workspace.onDidCloseTextDocument(e => {
if (e === doc) {
assert.equal(doc.languageId, langIdNow);
assert.equal(clock, 0);
clock += 1;
resolve();
}
});
}));
});
let open = new Promise(resolve => {
vscode.workspace.onDidOpenTextDocument(e => {
disposables.push(vscode.workspace.onDidOpenTextDocument(e => {
if (e === doc) { // same instance!
assert.equal(doc.languageId, 'json');
assert.equal(clock, 1);
clock += 1;
resolve();
}
});
}));
});
let change = vscode.languages.setTextDocumentLanguage(doc, 'json');
await Promise.all([change, close, open]);
assert.equal(clock, 2);
assert.equal(doc.languageId, 'json');
disposables.forEach(disposable => disposable.dispose());
disposables.length = 0;
});
test('setTextDocumentLanguage -> error when language does not exist', async function () {

34
src/vs/workbench/api/browser/mainThreadDocuments.ts
View file

@ -10,17 +10,18 @@ import { URI, UriComponents } from 'vs/base/common/uri';
import { ITextModel } from 'vs/editor/common/model';
import { IModelService, shouldSynchronizeModel } from 'vs/editor/common/services/modelService';
import { ITextModelService } from 'vs/editor/common/services/resolverService';
import { IFileService } from 'vs/platform/files/common/files';
import { IFileService, FileOperation } from 'vs/platform/files/common/files';
import { MainThreadDocumentsAndEditors } from 'vs/workbench/api/browser/mainThreadDocumentsAndEditors';
import { ExtHostContext, ExtHostDocumentsShape, IExtHostContext, MainThreadDocumentsShape } from 'vs/workbench/api/common/extHost.protocol';
import { ITextEditorModel } from 'vs/workbench/common/editor';
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
import { toLocalResource } from 'vs/base/common/resources';
import { toLocalResource, isEqualOrParent } from 'vs/base/common/resources';
import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
export class BoundModelReferenceCollection {
private _data = new Array<{ length: number, dispose(): void }>();
private _data = new Array<{ uri: URI, length: number, dispose(): void }>();
private _length = 0;
constructor(
@ -34,10 +35,18 @@ export class BoundModelReferenceCollection {
this._data = dispose(this._data);
}
add(ref: IReference<ITextEditorModel>): void {
remove(uri: URI): void {
for (const entry of [...this._data] /* copy array because dispose will modify it */) {
if (isEqualOrParent(entry.uri, uri)) {
entry.dispose();
}
}
}
add(uri: URI, ref: IReference<ITextEditorModel>): void {
const length = ref.object.textEditorModel.getValueLength();
let handle: any;
let entry: { length: number, dispose(): void };
let entry: { uri: URI, length: number, dispose(): void };
const dispose = () => {
const idx = this._data.indexOf(entry);
if (idx >= 0) {
@ -48,7 +57,7 @@ export class BoundModelReferenceCollection {
}
};
handle = setTimeout(dispose, this._maxAge);
entry = { length, dispose };
entry = { uri, length, dispose };
this._data.push(entry);
this._length += length;
@ -74,7 +83,7 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
private _modelToDisposeMap: { [modelUrl: string]: IDisposable; };
private readonly _proxy: ExtHostDocumentsShape;
private readonly _modelIsSynced = new Set<string>();
private _modelReferenceCollection = new BoundModelReferenceCollection();
private readonly _modelReferenceCollection = new BoundModelReferenceCollection();
constructor(
documentsAndEditors: MainThreadDocumentsAndEditors,
@ -83,7 +92,8 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
@ITextFileService textFileService: ITextFileService,
@IFileService fileService: IFileService,
@ITextModelService textModelResolverService: ITextModelService,
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,
@IWorkingCopyFileService workingCopyFileService: IWorkingCopyFileService
) {
this._modelService = modelService;
this._textModelResolverService = textModelResolverService;
@ -109,6 +119,12 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
}
}));
this._toDispose.add(workingCopyFileService.onDidRunWorkingCopyFileOperation(e => {
if (e.source && (e.operation === FileOperation.MOVE || e.operation === FileOperation.DELETE)) {
this._modelReferenceCollection.remove(e.source);
}
}));
this._modelToDisposeMap = Object.create(null);
}
@ -199,7 +215,7 @@ export class MainThreadDocuments implements MainThreadDocumentsShape {
private _handleAsResourceInput(uri: URI): Promise<boolean> {
return this._textModelResolverService.createModelReference(uri).then(ref => {
this._modelReferenceCollection.add(ref);
this._modelReferenceCollection.add(uri, ref);
const result = !!ref.object;
return result;
});

6
src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts
View file

@ -27,6 +27,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor
import { IPanelService } from 'vs/workbench/services/panel/common/panelService';
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
namespace delta {
@ -326,11 +327,12 @@ export class MainThreadDocumentsAndEditors {
@IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService,
@IBulkEditService bulkEditService: IBulkEditService,
@IPanelService panelService: IPanelService,
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,
@IWorkingCopyFileService workingCopyFileService: IWorkingCopyFileService
) {
this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostDocumentsAndEditors);
const mainThreadDocuments = this._toDispose.add(new MainThreadDocuments(this, extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService));
const mainThreadDocuments = this._toDispose.add(new MainThreadDocuments(this, extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService, workingCopyFileService));
extHostContext.set(MainContext.MainThreadDocuments, mainThreadDocuments);
const mainThreadTextEditors = this._toDispose.add(new MainThreadTextEditors(this, extHostContext, codeEditorService, bulkEditService, this._editorService, this._editorGroupService));

119
src/vs/workbench/test/browser/api/mainThreadDocuments.test.ts
View file

@ -7,6 +7,7 @@ import * as assert from 'assert';
import { BoundModelReferenceCollection } from 'vs/workbench/api/browser/mainThreadDocuments';
import { createTextModel } from 'vs/editor/test/common/editorTestUtils';
import { timeout } from 'vs/base/common/async';
import { URI } from 'vs/base/common/uri';
suite('BoundModelReferenceCollection', () => {
@ -20,12 +21,14 @@ suite('BoundModelReferenceCollection', () => {
let didDispose = false;
col.add({
object: <any>{ textEditorModel: createTextModel('farboo') },
dispose() {
didDispose = true;
}
});
col.add(
URI.parse('test://farboo'),
{
object: <any>{ textEditorModel: createTextModel('farboo') },
dispose() {
didDispose = true;
}
});
await timeout(30);
assert.equal(didDispose, true);
@ -35,27 +38,95 @@ suite('BoundModelReferenceCollection', () => {
let disposed: number[] = [];
col.add({
object: <any>{ textEditorModel: createTextModel('farboo') },
dispose() {
disposed.push(0);
}
});
col.add({
object: <any>{ textEditorModel: createTextModel('boofar') },
dispose() {
disposed.push(1);
}
});
col.add(
URI.parse('test://farboo'),
{
object: <any>{ textEditorModel: createTextModel('farboo') },
dispose() {
disposed.push(0);
}
});
col.add({
object: <any>{ textEditorModel: createTextModel(new Array(71).join('x')) },
dispose() {
disposed.push(2);
}
});
col.add(
URI.parse('test://boofar'),
{
object: <any>{ textEditorModel: createTextModel('boofar') },
dispose() {
disposed.push(1);
}
});
col.add(
URI.parse('test://xxxxxxx'),
{
object: <any>{ textEditorModel: createTextModel(new Array(71).join('x')) },
dispose() {
disposed.push(2);
}
});
assert.deepEqual(disposed, [0, 1]);
});
test('dispose uri', () => {
let disposed: number[] = [];
col.add(
URI.parse('test:///farboo'),
{
object: <any>{ textEditorModel: createTextModel('farboo') },
dispose() {
disposed.push(0);
}
});
col.add(
URI.parse('test:///boofar'),
{
object: <any>{ textEditorModel: createTextModel('boofar') },
dispose() {
disposed.push(1);
}
});
col.add(
URI.parse('test:///boo/far1'),
{
object: <any>{ textEditorModel: createTextModel('boo/far1') },
dispose() {
disposed.push(2);
}
});
col.add(
URI.parse('test:///boo/far2'),
{
object: <any>{ textEditorModel: createTextModel('boo/far2') },
dispose() {
disposed.push(3);
}
});
col.add(
URI.parse('test:///boo1/far'),
{
object: <any>{ textEditorModel: createTextModel('boo1/far') },
dispose() {
disposed.push(4);
}
});
col.remove(URI.parse('test:///unknown'));
assert.equal(disposed.length, 0);
col.remove(URI.parse('test:///farboo'));
assert.deepEqual(disposed, [0]);
disposed = [];
col.remove(URI.parse('test:///boo'));
assert.deepEqual(disposed, [2, 3]);
});
});

5
src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts
View file

@ -25,7 +25,7 @@ import { NullLogService } from 'vs/platform/log/common/log';
import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService';
import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService';
import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
import { TestTextResourcePropertiesService } from 'vs/workbench/test/common/workbenchTestServices';
import { TestTextResourcePropertiesService, TestWorkingCopyFileService } from 'vs/workbench/test/common/workbenchTestServices';
suite('MainThreadDocumentsAndEditors', () => {
@ -88,7 +88,8 @@ suite('MainThreadDocumentsAndEditors', () => {
return undefined;
}
},
TestEnvironmentService
TestEnvironmentService,
new TestWorkingCopyFileService()
);
});

1
src/vs/workbench/test/browser/api/mainThreadEditors.test.ts
View file

@ -104,6 +104,7 @@ suite('MainThreadEditors', () => {
};
});
services.set(IWorkingCopyFileService, new class extends mock<IWorkingCopyFileService>() {
onDidRunWorkingCopyFileOperation = Event.None;
move(source: URI, target: URI) {
movedResources.set(source, target);
return Promise.resolve(Object.create(null));

28
src/vs/workbench/test/common/workbenchTestServices.ts
View file

@ -14,8 +14,11 @@ import { IWorkspaceIdentifier, ISingleFolderWorkspaceIdentifier, isSingleFolderW
import { ITextResourcePropertiesService } from 'vs/editor/common/services/textResourceConfigurationService';
import { isLinux, isMacintosh } from 'vs/base/common/platform';
import { InMemoryStorageService, IWillSaveStateEvent } from 'vs/platform/storage/common/storage';
import { WorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
import { WorkingCopyService, IWorkingCopy } from 'vs/workbench/services/workingCopy/common/workingCopyService';
import { NullExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
import { IDisposable, Disposable } from 'vs/base/common/lifecycle';
import { FileOperation, IFileStatWithMetadata } from 'vs/platform/files/common/files';
export class TestTextResourcePropertiesService implements ITextResourcePropertiesService {
@ -121,6 +124,29 @@ export class TestStorageService extends InMemoryStorageService {
export class TestWorkingCopyService extends WorkingCopyService { }
export class TestWorkingCopyFileService implements IWorkingCopyFileService {
_serviceBrand: undefined;
onWillRunWorkingCopyFileOperation: Event<WorkingCopyFileEvent> = Event.None;
onDidFailWorkingCopyFileOperation: Event<WorkingCopyFileEvent> = Event.None;
onDidRunWorkingCopyFileOperation: Event<WorkingCopyFileEvent> = Event.None;
addFileOperationParticipant(participant: IWorkingCopyFileOperationParticipant): IDisposable { return Disposable.None; }
async runFileOperationParticipants(target: URI, source: URI | undefined, operation: FileOperation): Promise<void> { }
async delete(resource: URI, options?: { useTrash?: boolean | undefined; recursive?: boolean | undefined; } | undefined): Promise<void> { }
registerWorkingCopyProvider(provider: (resourceOrFolder: URI) => IWorkingCopy[]): IDisposable { return Disposable.None; }
getDirty(resource: URI): IWorkingCopy[] { return []; }
move(source: URI, target: URI, overwrite?: boolean | undefined): Promise<IFileStatWithMetadata> { throw new Error('Method not implemented.'); }
copy(source: URI, target: URI, overwrite?: boolean | undefined): Promise<IFileStatWithMetadata> { throw new Error('Method not implemented.'); }
}
export function mock<T>(): Ctor<T> {
return function () { } as any;
}