debt - careful with dispose() and assignments

src/vs/base/browser/ui/menu/menubar.ts

@@ -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;
 			});
 		}
 

src/vs/base/common/lifecycle.ts

@@ -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);
 		}

src/vs/editor/contrib/suggest/suggestWidget.ts

@@ -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;
 	}
 }
 

src/vs/platform/list/browser/listService.ts

@@ -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;
 	}
 }
 

src/vs/platform/storage/browser/storageService.ts

@@ -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();
 	}

src/vs/platform/storage/node/storageService.ts

@@ -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 });

src/vs/workbench/contrib/debug/browser/loadedScriptsView.ts

@@ -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();
 	}
 }

src/vs/workbench/contrib/files/common/editors/fileEditorInput.ts

@@ -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();
 	}

src/vs/workbench/contrib/files/test/browser/fileEditorInput.test.ts

@@ -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();
-
-		resolved = await inputToResolve.resolve();
-		assert(resolvedModelA === resolved); // Model is still the same because we had 2 clients
-		inputToResolve.dispose();
-		sameOtherInput.dispose();
-		resolvedModelA.dispose();
-
-		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
+		try {
+			DisposableStore.DISABLE_DISPOSED_WARNING = true; // prevent unwanted warning output from occuring
+
+			const otherResolved = await sameOtherInput.resolve();
+			assert(otherResolved === resolvedModelA); // OK: Resolved Model cached globally per input
+			inputToResolve.dispose();
+
+			resolved = await inputToResolve.resolve();
+			assert(resolvedModelA === resolved); // Model is still the same because we had 2 clients
+			inputToResolve.dispose();
+			sameOtherInput.dispose();
+			resolvedModelA.dispose();
+
+			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 () {

src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

@@ -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');