Fix some connection listener leaks (#6357)

* Fix some connection listener leaks

* More descriptive name and update summary

* Dispose some more connections and fix a few spelling errors
This commit is contained in:
Charles Gagnon
2019-07-29 11:00:11 -07:00
committed by GitHub
parent 1d56a17f32
commit 86cde4c511
9 changed files with 66 additions and 28 deletions

View File

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

View File

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

View File

@@ -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 {

View File

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

View File

@@ -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,14 +75,24 @@ 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 {
let instance = new ProviderConnectionInfo(this.capabilitiesService, this.providerName);
instance.options = Object.assign({}, this.options);

View File

@@ -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.

View File

@@ -117,22 +117,22 @@ 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)) {
@@ -140,9 +140,10 @@ export class ConnectionController implements IConnectionComponentController {
} 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 {

View File

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

View File

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