Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2025-12-24 20:26:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

debt - careful with dispose() and assignments

Browse Source
This commit is contained in:
Benjamin Pasero
2020-02-04 08:53:23 +01:00
parent a07225a389
commit c6750ffa87
10 changed files with 59 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/vs/base/browser/ui/menu/menubar.ts
View File

@@ -88,7 +88,7 @@ export class MenuBar extends Disposable {
private numMenusShown: number = 0;
private menuStyle: IMenuStyles | undefined;
private overflowLayoutScheduled: IDisposable | null = null;
private overflowLayoutScheduled: IDisposable | undefined = undefined;
constructor(private container: HTMLElement, private options: IMenuBarOptions = {}) {
super();
@@ -419,9 +419,8 @@ export class MenuBar extends Disposable {
DOM.removeNode(this.overflowMenu.titleElement);
DOM.removeNode(this.overflowMenu.buttonElement);
if (this.overflowLayoutScheduled) {
this.overflowLayoutScheduled = dispose(this.overflowLayoutScheduled);
}
dispose(this.overflowLayoutScheduled);
this.overflowLayoutScheduled = undefined;
}
blur(): void {
@@ -561,7 +560,7 @@ export class MenuBar extends Disposable {
if (!this.overflowLayoutScheduled) {
this.overflowLayoutScheduled = DOM.scheduleAtNextAnimationFrame(() => {
this.updateOverflowAction();
this.overflowLayoutScheduled = null;
this.overflowLayoutScheduled = undefined;
});
}

7
src/vs/base/common/lifecycle.ts
View File

@@ -91,6 +91,9 @@ export function toDisposable(fn: () => void): IDisposable {
}
export class DisposableStore implements IDisposable {
static DISABLE_DISPOSED_WARNING = false;
private _toDispose = new Set<IDisposable>();
private _isDisposed = false;
@@ -127,7 +130,9 @@ export class DisposableStore implements IDisposable {
markTracked(t);
if (this._isDisposed) {
console.warn(new Error('Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!').stack);
if (!DisposableStore.DISABLE_DISPOSED_WARNING) {
console.warn(new Error('Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!').stack);
}
} else {
this._toDispose.add(t);
}

6
src/vs/editor/contrib/suggest/suggestWidget.ts
View File

@@ -342,7 +342,8 @@ class SuggestionDetails {
}
renderItem(item: CompletionItem, explainMode: boolean): void {
this.renderDisposeable = dispose(this.renderDisposeable);
dispose(this.renderDisposeable);
this.renderDisposeable = undefined;
let { documentation, detail } = item.completion;
// --- documentation
@@ -449,7 +450,8 @@ class SuggestionDetails {
dispose(): void {
this.disposables.dispose();
this.renderDisposeable = dispose(this.renderDisposeable);
dispose(this.renderDisposeable);
this.renderDisposeable = undefined;
}
}

5
src/vs/platform/list/browser/listService.ts
View File

@@ -952,7 +952,7 @@ class WorkbenchTreeInternals<TInput, T, TFilterData> {
private hasMultiSelection: IContextKey<boolean>;
private _useAltAsMultipleSelectionModifier: boolean;
private disposables: IDisposable[] = [];
private styler!: IDisposable;
private styler: IDisposable | undefined;
constructor(
private tree: WorkbenchObjectTree<T, TFilterData> | CompressibleObjectTree<T, TFilterData> | WorkbenchDataTree<TInput, T, TFilterData> | WorkbenchAsyncDataTree<TInput, T, TFilterData> | WorkbenchCompressibleAsyncDataTree<TInput, T, TFilterData>,
@@ -1048,7 +1048,8 @@ class WorkbenchTreeInternals<TInput, T, TFilterData> {
dispose(): void {
this.disposables = dispose(this.disposables);
this.styler = dispose(this.styler);
dispose(this.styler);
this.styler = undefined;
}
}

5
src/vs/platform/storage/browser/storageService.ts
View File

@@ -136,7 +136,7 @@ export class BrowserStorageService extends Disposable implements IStorageService
private doFlushWhenIdle(): void {
// Dispose any previous idle runner
this.runWhenIdleDisposable = dispose(this.runWhenIdleDisposable);
dispose(this.runWhenIdleDisposable);
// Run when idle
this.runWhenIdleDisposable = runWhenIdle(() => {
@@ -180,7 +180,8 @@ export class BrowserStorageService extends Disposable implements IStorageService
}
dispose(): void {
this.runWhenIdleDisposable = dispose(this.runWhenIdleDisposable);
dispose(this.runWhenIdleDisposable);
this.runWhenIdleDisposable = undefined;
super.dispose();
}

5
src/vs/platform/storage/node/storageService.ts
View File

@@ -203,7 +203,7 @@ export class NativeStorageService extends Disposable implements IStorageService
private doFlushWhenIdle(): void {
// Dispose any previous idle runner
this.runWhenIdleDisposable = dispose(this.runWhenIdleDisposable);
dispose(this.runWhenIdleDisposable);
// Run when idle
this.runWhenIdleDisposable = runWhenIdle(() => {
@@ -224,7 +224,8 @@ export class NativeStorageService extends Disposable implements IStorageService
// Stop periodic scheduler and idle runner as we now collect state normally
this.periodicFlushScheduler.dispose();
this.runWhenIdleDisposable = dispose(this.runWhenIdleDisposable);
dispose(this.runWhenIdleDisposable);
this.runWhenIdleDisposable = undefined;
// Signal as event so that clients can still store data
this._onWillSaveState.fire({ reason: WillSaveStateReason.SHUTDOWN });

4
src/vs/workbench/contrib/debug/browser/loadedScriptsView.ts
View File

@@ -606,8 +606,8 @@ export class LoadedScriptsView extends ViewPane {
}
dispose(): void {
this.tree = dispose(this.tree);
this.treeLabels = dispose(this.treeLabels);
dispose(this.tree);
dispose(this.treeLabels);
super.dispose();
}
}

11
src/vs/workbench/contrib/files/common/editors/fileEditorInput.ts
View File

@@ -240,10 +240,12 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
// re-emit some events from the model
this._register(model.onDidChangeDirty(() => this._onDidChangeDirty.fire()));
this._register(model.onDidSave(() => this._onDidChangeDirty.fire()));
this._register(model.onDidSaveError(() => this._onDidChangeDirty.fire()));
this._register(model.onDidRevert(() => this._onDidChangeDirty.fire()));
this._register(model.onDidChangeOrphaned(() => this._onDidChangeLabel.fire()));
// important: treat save errors as potential dirty change because
// a file that is in save conflict or error will report dirty even
// if auto save is turned on.
this._register(model.onDidSaveError(() => this._onDidChangeDirty.fire()));
}
private async doResolveAsBinary(): Promise<BinaryEditorModel> {
@@ -269,7 +271,8 @@ export class FileEditorInput extends TextResourceEditorInput implements IFileEdi
dispose(): void {
// Model reference
this.cachedTextFileModelReference = dispose(this.cachedTextFileModelReference);
dispose(this.cachedTextFileModelReference);
this.cachedTextFileModelReference = undefined;
super.dispose();
}

35
src/vs/workbench/contrib/files/test/browser/fileEditorInput.test.ts
View File

@@ -17,6 +17,7 @@ import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textF
import { IModelService } from 'vs/editor/common/services/modelService';
import { timeout } from 'vs/base/common/async';
import { ModesRegistry, PLAINTEXT_MODE_ID } from 'vs/editor/common/modes/modesRegistry';
import { DisposableStore } from 'vs/base/common/lifecycle';
class ServiceAccessor {
constructor(
@@ -66,23 +67,29 @@ suite('Files - FileEditorInput', () => {
resolved = await inputToResolve.resolve();
assert(resolvedModelA === resolved); // OK: Resolved Model cached globally per input
const otherResolved = await sameOtherInput.resolve();
assert(otherResolved === resolvedModelA); // OK: Resolved Model cached globally per input
inputToResolve.dispose();
try {
DisposableStore.DISABLE_DISPOSED_WARNING = true; // prevent unwanted warning output from occuring
resolved = await inputToResolve.resolve();
assert(resolvedModelA === resolved); // Model is still the same because we had 2 clients
inputToResolve.dispose();
sameOtherInput.dispose();
resolvedModelA.dispose();
const otherResolved = await sameOtherInput.resolve();
assert(otherResolved === resolvedModelA); // OK: Resolved Model cached globally per input
inputToResolve.dispose();
resolved = await inputToResolve.resolve();
assert(resolvedModelA !== resolved); // Different instance, because input got disposed
resolved = await inputToResolve.resolve();
assert(resolvedModelA === resolved); // Model is still the same because we had 2 clients
inputToResolve.dispose();
sameOtherInput.dispose();
resolvedModelA.dispose();
const stat = (resolved as TextFileEditorModel).getStat();
resolved = await inputToResolve.resolve();
await timeout(0);
assert(stat !== (resolved as TextFileEditorModel).getStat()); // Different stat, because resolve always goes to the server for refresh
resolved = await inputToResolve.resolve();
assert(resolvedModelA !== resolved); // Different instance, because input got disposed
const stat = (resolved as TextFileEditorModel).getStat();
resolved = await inputToResolve.resolve();
await timeout(0);
assert(stat !== (resolved as TextFileEditorModel).getStat()); // Different stat, because resolve always goes to the server for refresh
} finally {
DisposableStore.DISABLE_DISPOSED_WARNING = false;
}
});
test('preferred mode', async function () {

9
src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
View File

@@ -795,9 +795,12 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
dispose(this._windowsShellHelper);
this._windowsShellHelper = undefined;
this._linkHandler = dispose(this._linkHandler);
this._commandTrackerAddon = dispose(this._commandTrackerAddon);
this._widgetManager = dispose(this._widgetManager);
dispose(this._linkHandler);
this._linkHandler = undefined;
dispose(this._commandTrackerAddon);
this._commandTrackerAddon = undefined;
dispose(this._widgetManager);
this._widgetManager = undefined;
if (this._xterm && this._xterm.element) {
this._hadFocusOnExit = dom.hasClass(this._xterm.element, 'focus');