Make sure `Dispose.register` does not allow registering new disposables after...

Make sure `Dispose.register` does not allow registering new disposables after the base has been disposed

`register` can be called after `dispose` in some race conditions, especially when dealing with events. Previously in this case, `register` would continue to hold onto the newly registered objects and they would never be properly disposed. With this change, we now dispose of newly registered objects immediately and print a warning if the base has already been disposed

src/vs/base/common/lifecycle.ts

@@ -49,12 +49,20 @@ export abstract class Disposable implements IDisposable {
 	protected _toDispose: IDisposable[] = [];
 	protected get toDispose(): IDisposable[] { return this._toDispose; }
 
+	private _lifecycle_disposable_isDisposed = false;
+
 	public dispose(): void {
+		this._lifecycle_disposable_isDisposed = true;
 		this._toDispose = dispose(this._toDispose);
 	}
 
 	protected _register<T extends IDisposable>(t: T): T {
+		if (this._lifecycle_disposable_isDisposed) {
+			console.warn('Registering disposable on object that has already been disposed.');
+			t.dispose();
+		} else {
 			this._toDispose.push(t);
+		}
 
 		return t;
 	}

src/vs/base/test/common/lifecycle.test.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 import * as assert from 'assert';
-import { IDisposable, dispose, ReferenceCollection } from 'vs/base/common/lifecycle';
+import { IDisposable, dispose, ReferenceCollection, Disposable as DisposableBase, toDisposable } from 'vs/base/common/lifecycle';
 
 class Disposable implements IDisposable {
 	isDisposed = false;
@@ -49,6 +49,38 @@ suite('Lifecycle', () => {
 	});
 });
 
+suite('DisposableBase', () => {
+	test('register should not leak if object has already been disposed', () => {
+		let aCount = 0;
+		let bCount = 0;
+
+		const disposable = new class extends DisposableBase {
+			register(other: IDisposable) {
+				this._register(other);
+			}
+		};
+
+		disposable.register(toDisposable(() => ++aCount));
+
+		assert.strictEqual(aCount, 0);
+		assert.strictEqual(bCount, 0);
+
+		disposable.dispose();
+		assert.strictEqual(aCount, 1);
+		assert.strictEqual(bCount, 0);
+
+		// Any newly added disposables should be disposed of immediately
+		disposable.register(toDisposable(() => ++bCount));
+		assert.strictEqual(aCount, 1);
+		assert.strictEqual(bCount, 1);
+
+		// Further dispose calls should have no effect
+		disposable.dispose();
+		assert.strictEqual(aCount, 1);
+		assert.strictEqual(bCount, 1);
+	});
+});
+
 suite('Reference Collection', () => {
 	class Collection extends ReferenceCollection<number> {
 		private _count = 0;