Fix some connection listener leaks (microsoft#6357)

* Fix some connection listener leaks * More descriptive name and update summary * Dispose some more connections and fix a few spelling errors
bbbbb38438 · Jul 29, 2019 · 86cde4c · 86cde4c
1 parent 1d56a17
commit 86cde4c
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 28 deletions.
diff --git a/src/sql/platform/capabilities/common/capabilitiesService.ts b/src/sql/platform/capabilities/common/capabilitiesService.ts
@@ -55,7 +55,7 @@ export interface ICapabilitiesService {
 	isFeatureAvailable(action: IAction, connectionManagementInfo: ConnectionManagementInfo): boolean;
 
 	/**
-	 * When a new capabilities is registered, it emits the provider name, be to use to get the new capabilities
+	 * When new capabilities are registered, it emits the @see ProviderFeatures, which can be used to get the new capabilities
 	 */
 	readonly onCapabilitiesRegistered: Event<ProviderFeatures>;
 

diff --git a/src/sql/platform/capabilities/common/capabilitiesServiceImpl.ts b/src/sql/platform/capabilities/common/capabilitiesServiceImpl.ts
@@ -34,7 +34,7 @@ interface CapabilitiesMomento {
 
 /**
  * Capabilities service implementation class.  This class provides the ability
- * to discover the DMP capabilties that a DMP provider offers.
+ * to discover the DMP capabilities that a DMP provider offers.
  */
 export class CapabilitiesService extends Disposable implements ICapabilitiesService {
 	_serviceBrand: any;
@@ -50,7 +50,7 @@ export class CapabilitiesService extends Disposable implements ICapabilitiesServ
 	constructor(
 		@IStorageService private _storageService: IStorageService,
 		@IExtensionService extensionService: IExtensionService,
-		@IExtensionManagementService extentionManagementService: IExtensionManagementService
+		@IExtensionManagementService extensionManagementService: IExtensionManagementService
 	) {
 		super();
 
@@ -78,7 +78,7 @@ export class CapabilitiesService extends Disposable implements ICapabilitiesServ
 
 		_storageService.onWillSaveState(() => this.shutdown());
 
-		this._register(extentionManagementService.onDidUninstallExtension(({ identifier }) => {
+		this._register(extensionManagementService.onDidUninstallExtension(({ identifier }) => {
 			const connectionProvider = 'connectionProvider';
 			extensionService.getExtensions().then(i => {
 				let extension = i.find(c => c.identifier.value.toLowerCase() === identifier.id.toLowerCase());

diff --git a/src/sql/platform/connection/common/connectionManagementService.ts b/src/sql/platform/connection/common/connectionManagementService.ts
@@ -691,7 +691,10 @@ export class ConnectionManagementService extends Disposable implements IConnecti
 	}
 
 	public hasRegisteredServers(): boolean {
-		return this.doHasRegisteredServers(this.getConnectionGroups());
+		const groups: ConnectionProfileGroup[] = this.getConnectionGroups();
+		const hasRegisteredServers: boolean = this.doHasRegisteredServers(groups);
+		groups.forEach(cpg => cpg.dispose());
+		return hasRegisteredServers;
 	}
 
 	private doHasRegisteredServers(root: ConnectionProfileGroup[]): boolean {

diff --git a/src/sql/platform/connection/common/connectionProfileGroup.ts b/src/sql/platform/connection/common/connectionProfileGroup.ts
@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
+import { Disposable } from 'vs/base/common/lifecycle';
 
 export interface IConnectionProfileGroup {
 	id: string;
@@ -13,7 +14,7 @@ export interface IConnectionProfileGroup {
 	description: string;
 }
 
-export class ConnectionProfileGroup implements IConnectionProfileGroup {
+export class ConnectionProfileGroup extends Disposable implements IConnectionProfileGroup {
 
 	public children: ConnectionProfileGroup[];
 	public connections: ConnectionProfile[];
@@ -26,6 +27,7 @@ export class ConnectionProfileGroup implements IConnectionProfileGroup {
 		public color: string,
 		public description: string
 	) {
+		super();
 		this.parentId = parent ? parent.id : undefined;
 		if (this.name === ConnectionProfileGroup.RootGroupName) {
 			this.name = '';
@@ -100,9 +102,8 @@ export class ConnectionProfileGroup implements IConnectionProfileGroup {
 		}
 	}
 
-	public getChildren(): any {
-		let allChildren = [];
-
+	public getChildren(): (ConnectionProfile | ConnectionProfileGroup)[] {
+		let allChildren: (ConnectionProfile | ConnectionProfileGroup)[] = [];
 		if (this.connections) {
 			this.connections.forEach((conn) => {
 				allChildren.push(conn);
@@ -131,6 +132,7 @@ export class ConnectionProfileGroup implements IConnectionProfileGroup {
 		connections.forEach((conn) => {
 			this.connections = this.connections.filter((curConn) => { return curConn.id !== conn.id; });
 			conn.parent = this;
+			this._register(conn);
 			this.connections.push(conn);
 		});
 
@@ -143,6 +145,7 @@ export class ConnectionProfileGroup implements IConnectionProfileGroup {
 		groups.forEach((group) => {
 			this.children = this.children.filter((grp) => { return group.id !== grp.id; });
 			group.parent = this;
+			this._register(group);
 			this.children.push(group);
 		});
 	}

diff --git a/src/sql/platform/connection/common/providerConnectionInfo.ts b/src/sql/platform/connection/common/providerConnectionInfo.ts
@@ -3,7 +3,7 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { Disposable } from 'vs/base/common/lifecycle';
+import { Disposable, dispose, IDisposable } from 'vs/base/common/lifecycle';
 import { isString } from 'vs/base/common/types';
 
 import * as azdata from 'azdata';
@@ -19,6 +19,7 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect
 	options: { [name: string]: any } = {};
 
 	private _providerName: string;
+	private _onCapabilitiesRegisteredDisposable: IDisposable;
 	protected _serverCapabilities: ConnectionProviderProperties;
 	private static readonly SqlAuthentication = 'SqlLogin';
 	public static readonly ProviderPropertyName = 'providerName';
@@ -74,12 +75,22 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect
 			if (capabilities) {
 				this._serverCapabilities = capabilities.connection;
 			}
-			this._register(this.capabilitiesService.onCapabilitiesRegistered(e => {
+			if (this._onCapabilitiesRegisteredDisposable) {
+				dispose(this._onCapabilitiesRegisteredDisposable);
+			}
+			this._onCapabilitiesRegisteredDisposable = this.capabilitiesService.onCapabilitiesRegistered(e => {
 				if (e.connection.providerId === this.providerName) {
 					this._serverCapabilities = e.connection;
 				}
-			}));
+			});
+		}
+	}
+
+	public dispose(): void {
+		if (this._onCapabilitiesRegisteredDisposable) {
+			dispose(this._onCapabilitiesRegisteredDisposable);
 		}
+		super.dispose();
 	}
 
 	public clone(): ProviderConnectionInfo {

diff --git a/src/sql/workbench/parts/objectExplorer/browser/treeUpdateUtils.ts b/src/sql/workbench/parts/objectExplorer/browser/treeUpdateUtils.ts
@@ -13,6 +13,7 @@ import { NodeType } from 'sql/workbench/parts/objectExplorer/common/nodeType';
 import { TreeNode } from 'sql/workbench/parts/objectExplorer/common/treeNode';
 import * as errors from 'vs/base/common/errors';
 import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
+import { Disposable } from 'vs/base/common/lifecycle';
 
 export interface IExpandableTree extends ITree {
 	// {{SQL CARBON EDIT }}	- add back deleted VS Code tree methods
@@ -72,8 +73,11 @@ export class TreeUpdateUtils {
 		} else if (viewKey === 'saved') {
 			treeInput = TreeUpdateUtils.getTreeInput(connectionManagementService, providers);
 		}
-
+		const previousTreeInput: any = tree.getInput();
 		return tree.setInput(treeInput).then(() => {
+			if (previousTreeInput instanceof Disposable) {
+				previousTreeInput.dispose();
+			}
 			// Make sure to expand all folders that where expanded in the previous session
 			if (targetsToExpand) {
 				tree.expandAll(targetsToExpand);
@@ -135,6 +139,7 @@ export class TreeUpdateUtils {
 		if (groups && groups.length > 0) {
 			let treeInput = groups[0];
 			treeInput.name = 'root';
+			groups.forEach(cpg => cpg.dispose());
 			return treeInput;
 		}
 		// Should never get to this case.

diff --git a/src/sql/workbench/services/connection/browser/connectionController.ts b/src/sql/workbench/services/connection/browser/connectionController.ts
@@ -117,32 +117,33 @@ export class ConnectionController implements IConnectionComponentController {
 		this._connectionWidget.createConnectionWidget(container);
 	}
 
-	private getServerGroupHelper(group: ConnectionProfileGroup, groupNames: IConnectionProfileGroup[]): void {
+	private flattenGroups(group: ConnectionProfileGroup, allGroups: IConnectionProfileGroup[]): void {
 		if (group) {
 			if (group.fullName !== '') {
-				groupNames.push(group);
+				allGroups.push(group);
 			}
 			if (group.hasChildren()) {
-				group.children.forEach((child) => this.getServerGroupHelper(child, groupNames));
+				group.children.forEach((child) => this.flattenGroups(child, allGroups));
 			}
 		}
 	}
 
 	private getAllServerGroups(providers?: string[]): IConnectionProfileGroup[] {
 		let connectionGroupRoot = this._connectionManagementService.getConnectionGroups(providers);
-		let connectionGroupNames: IConnectionProfileGroup[] = [];
+		let allGroups: IConnectionProfileGroup[] = [];
 		if (connectionGroupRoot && connectionGroupRoot.length > 0) {
-			this.getServerGroupHelper(connectionGroupRoot[0], connectionGroupNames);
+			this.flattenGroups(connectionGroupRoot[0], allGroups);
 		}
 		let defaultGroupId: string;
 		if (connectionGroupRoot && connectionGroupRoot.length > 0 && ConnectionProfileGroup.isRoot(connectionGroupRoot[0].name)) {
 			defaultGroupId = connectionGroupRoot[0].id;
 		} else {
 			defaultGroupId = Utils.defaultGroupId;
 		}
-		connectionGroupNames.push(Object.assign({}, this._connectionWidget.DefaultServerGroup, { id: defaultGroupId }));
-		connectionGroupNames.push(this._connectionWidget.NoneServerGroup);
-		return connectionGroupNames;
+		allGroups.push(Object.assign({}, this._connectionWidget.DefaultServerGroup, { id: defaultGroupId }));
+		allGroups.push(this._connectionWidget.NoneServerGroup);
+		connectionGroupRoot.forEach(cpg => cpg.dispose());
+		return allGroups;
 	}
 
 	public initDialog(providers: string[], connectionInfo: IConnectionProfile): void {

diff --git a/src/sql/workbench/services/connection/browser/connectionDialogService.ts b/src/sql/workbench/services/connection/browser/connectionDialogService.ts
@@ -316,8 +316,11 @@ export class ConnectionDialogService implements IConnectionDialogService {
 			}
 		}
 		this._model.providerName = this._currentProviderType;
-
+		const previousModel = this._model;
 		this._model = new ConnectionProfile(this._capabilitiesService, this._model);
+		if (previousModel) {
+			previousModel.dispose();
+		}
 		if (this._inputModel && this._inputModel.options) {
 			this.uiController.showUiComponent(input.container,
 				this._inputModel.options.authTypeChanged);
@@ -333,9 +336,12 @@ export class ConnectionDialogService implements IConnectionDialogService {
 
 	private handleFillInConnectionInputs(connectionInfo: IConnectionProfile): void {
 		this._connectionManagementService.addSavedPassword(connectionInfo).then(connectionWithPassword => {
-			let model = this.createModel(connectionWithPassword);
-			this._model = model;
-			this.uiController.fillInConnectionInputs(model);
+			if (this._model) {
+				this._model.dispose();
+			}
+			this._model = this.createModel(connectionWithPassword);
+
+			this.uiController.fillInConnectionInputs(this._model);
 		});
 		this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[connectionInfo.providerName]);
 	}
@@ -349,6 +355,9 @@ export class ConnectionDialogService implements IConnectionDialogService {
 	}
 
 	private updateModelServerCapabilities(model: IConnectionProfile) {
+		if (this._model) {
+			this._model.dispose();
+		}
 		this._model = this.createModel(model);
 		if (this._model.providerName) {
 			this._currentProviderType = this._model.providerName;
@@ -452,8 +461,10 @@ export class ConnectionDialogService implements IConnectionDialogService {
 		this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[this._currentProviderType]);
 
 		return new Promise<void>(() => {
-			this._connectionDialog.open(this._connectionManagementService.getRecentConnections(params.providers).length > 0);
+			const recentConnections: ConnectionProfile[] = this._connectionManagementService.getRecentConnections(params.providers);
+			this._connectionDialog.open(recentConnections.length > 0);
 			this.uiController.focusOnOpen();
+			recentConnections.forEach(conn => conn.dispose());
 		});
 	}
 

diff --git a/src/sql/workbench/services/connection/browser/connectionDialogWidget.ts b/src/sql/workbench/services/connection/browser/connectionDialogWidget.ts
@@ -321,10 +321,14 @@ export class ConnectionDialogWidget extends Modal {
 		const actionProvider = this._instantiationService.createInstance(RecentConnectionActionsProvider);
 		const controller = new RecentConnectionTreeController(leftClick, actionProvider, this._connectionManagementService, this._contextMenuService);
 		actionProvider.onRecentConnectionRemoved(() => {
-			this.open(this._connectionManagementService.getRecentConnections().length > 0);
+			const recentConnections: ConnectionProfile[] = this._connectionManagementService.getRecentConnections();
+			this.open(recentConnections.length > 0);
+			recentConnections.forEach(conn => conn.dispose());
 		});
 		controller.onRecentConnectionRemoved(() => {
-			this.open(this._connectionManagementService.getRecentConnections().length > 0);
+			const recentConnections: ConnectionProfile[] = this._connectionManagementService.getRecentConnections();
+			this.open(recentConnections.length > 0);
+			recentConnections.forEach(conn => conn.dispose());
 		});
 		this._recentConnectionTree = TreeCreationUtils.createConnectionTree(treeContainer, this._instantiationService, controller);