diff --git a/src/sql/platform/connection/common/connectionConfig.ts b/src/sql/platform/connection/common/connectionConfig.ts index 19b4b51d25..db7e0a535c 100644 --- a/src/sql/platform/connection/common/connectionConfig.ts +++ b/src/sql/platform/connection/common/connectionConfig.ts @@ -13,6 +13,7 @@ import { generateUuid } from 'vs/base/common/uuid'; import * as nls from 'vs/nls'; import { ConfigurationTarget, IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { deepClone } from 'vs/base/common/objects'; +import { isDisposable } from 'vs/base/common/lifecycle'; export const GROUPS_CONFIG_KEY = 'datasource.connectionGroups'; export const CONNECTIONS_CONFIG_KEY = 'datasource.connections'; @@ -99,8 +100,10 @@ export class ConnectionConfig { // Remove the profile if already set let sameProfileInList = profiles.find(value => { - let providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService); - return matcher(providerConnectionProfile, connectionProfile); + const providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService); + const match = matcher(providerConnectionProfile, connectionProfile); + providerConnectionProfile.dispose(); + return match; }); if (sameProfileInList) { let profileIndex = profiles.findIndex(value => value === sameProfileInList); @@ -111,7 +114,14 @@ export class ConnectionConfig { profiles.push(newProfile); } - return this.configurationService.updateValue(CONNECTIONS_CONFIG_KEY, profiles, ConfigurationTarget.USER).then(() => connectionProfile); + return this.configurationService.updateValue(CONNECTIONS_CONFIG_KEY, profiles, ConfigurationTarget.USER).then(() => { + profiles.forEach(p => { + if (isDisposable(p)) { + p.dispose(); + } + }); + return connectionProfile; + }); }); } else { return Promise.resolve(profile); diff --git a/src/sql/platform/connection/common/connectionStore.ts b/src/sql/platform/connection/common/connectionStore.ts index 1135ca3b02..a2089d4c11 100644 --- a/src/sql/platform/connection/common/connectionStore.ts +++ b/src/sql/platform/connection/common/connectionStore.ts @@ -11,6 +11,7 @@ import { ConnectionProfile } from 'sql/platform/connection/common/connectionProf import { ConnectionProfileGroup, IConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup'; import { IConnectionProfile, ProfileMatcher } from 'sql/platform/connection/common/interfaces'; import { ICredentialsService } from 'sql/platform/credentials/common/credentialsService'; +import { isDisposable } from 'vs/base/common/lifecycle'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; @@ -112,6 +113,9 @@ export class ConnectionStore { // Add the profile to the saved list, taking care to clear out the password field if necessary const savedProfile = forceWritePlaintextPassword ? profile : this.getProfileWithoutPassword(profile); const savedConnectionProfile = await this.saveProfileToConfig(savedProfile, matcher); + if (isDisposable(savedProfile)) { + savedProfile.dispose(); + } profile.groupId = savedConnectionProfile.groupId; profile.id = savedConnectionProfile.id; // Only save if we successfully added the profile @@ -177,9 +181,9 @@ export class ConnectionStore { public getProfileWithoutPassword(conn: IConnectionProfile): ConnectionProfile { let savedConn = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, conn); - savedConn = savedConn.withoutPassword(); - - return savedConn; + let newSavedConn = savedConn.withoutPassword(); + savedConn.dispose(); + return newSavedConn; } /** @@ -225,7 +229,9 @@ export class ConnectionStore { list.unshift(savedProfile); - return list.filter(n => n !== undefined).map(c => c.toIConnectionProfile()); + const profiles = list.filter(n => n !== undefined).map(c => c.toIConnectionProfile()); + list.forEach(c => c.dispose()); + return profiles; } private removeFromConnectionList(conn: IConnectionProfile, list: ConnectionProfile[]): IConnectionProfile[] { diff --git a/src/sql/platform/connection/common/providerConnectionInfo.ts b/src/sql/platform/connection/common/providerConnectionInfo.ts index 3d15c988b8..754aa5e724 100644 --- 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, dispose, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable } from 'vs/base/common/lifecycle'; import { isString } from 'vs/base/common/types'; import * as azdata from 'azdata'; @@ -18,8 +18,6 @@ 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'; @@ -33,8 +31,8 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect this.providerName = isString(model) ? model : model.providerName; if (!isString(model)) { - if (model.options && this._serverCapabilities) { - this._serverCapabilities.connectionOptions.forEach(option => { + if (model.options && this.serverCapabilities) { + this.serverCapabilities.connectionOptions.forEach(option => { let value = model.options[option.name]; this.options[option.name] = value; }); @@ -69,26 +67,13 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect public set providerName(name: string) { this._providerName = name; - if (!this._serverCapabilities && this.capabilitiesService) { - let capabilities = this.capabilitiesService.getCapabilities(this.providerName); - if (capabilities) { - this._serverCapabilities = capabilities.connection; - } - if (this._onCapabilitiesRegisteredDisposable) { - dispose(this._onCapabilitiesRegisteredDisposable); - } - this._onCapabilitiesRegisteredDisposable = this.capabilitiesService.onCapabilitiesRegistered(e => { - if (e.id === this.providerName) { - this._serverCapabilities = e.features.connection; - } - }); - } } public override dispose(): void { - if (this._onCapabilitiesRegisteredDisposable) { - dispose(this._onCapabilitiesRegisteredDisposable); - } + // Notes: + // 1. It is not recommended to add disposables to this class considering the way we create and use the connection objects, + // there are places that the connection objects are not being disposed properly. + // 2. Remember to dispose the listeners added to this class. super.dispose(); } @@ -99,7 +84,7 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect } public get serverCapabilities(): ConnectionProviderProperties | undefined { - return this._serverCapabilities; + return this.capabilitiesService?.getCapabilities(this.providerName)?.connection; } public get connectionName(): string { @@ -196,11 +181,11 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect public isPasswordRequired(): boolean { // if there is no provider capabilities metadata assume a password is not required - if (!this._serverCapabilities) { + if (!this.serverCapabilities) { return false; } - let optionMetadata = this._serverCapabilities.connectionOptions.find( + let optionMetadata = this.serverCapabilities.connectionOptions.find( option => option.specialValueType === ConnectionOptionSpecialType.password)!; // i guess we are going to assume there is a password field let isPasswordRequired = optionMetadata.isRequired; if (this.providerName === Constants.mssqlProviderName) { @@ -224,8 +209,8 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect */ public getOptionsKey(): string { let idNames = []; - if (this._serverCapabilities) { - idNames = this._serverCapabilities.connectionOptions.map(o => { + if (this.serverCapabilities) { + idNames = this.serverCapabilities.connectionOptions.map(o => { if ((o.specialValueType || o.isIdentity) && o.specialValueType !== ConnectionOptionSpecialType.password && o.specialValueType !== ConnectionOptionSpecialType.connectionName) { @@ -270,8 +255,8 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect } public getSpecialTypeOptionName(type: string): string | undefined { - if (this._serverCapabilities) { - let optionMetadata = this._serverCapabilities.connectionOptions.find(o => o.specialValueType === type); + if (this.serverCapabilities) { + let optionMetadata = this.serverCapabilities.connectionOptions.find(o => o.specialValueType === type); return !!optionMetadata ? optionMetadata.name : undefined; } else { return type.toString(); @@ -286,7 +271,7 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect } public get authenticationTypeDisplayName(): string { - let optionMetadata = this._serverCapabilities ? this._serverCapabilities.connectionOptions.find(o => o.specialValueType === ConnectionOptionSpecialType.authType) : undefined; + let optionMetadata = this.serverCapabilities ? this.serverCapabilities.connectionOptions.find(o => o.specialValueType === ConnectionOptionSpecialType.authType) : undefined; let authType = this.authenticationType; let displayName: string = authType; @@ -301,7 +286,7 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect } public getProviderOptions(): azdata.ConnectionOption[] | undefined { - return this._serverCapabilities?.connectionOptions; + return this.serverCapabilities?.connectionOptions; } public static get idSeparator(): string { @@ -319,8 +304,8 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect parts.push(this.databaseName); parts.push(this.authenticationTypeDisplayName); - if (this._serverCapabilities) { - this._serverCapabilities.connectionOptions.forEach(element => { + if (this.serverCapabilities) { + this.serverCapabilities.connectionOptions.forEach(element => { if (element.specialValueType !== ConnectionOptionSpecialType.serverName && element.specialValueType !== ConnectionOptionSpecialType.databaseName && element.specialValueType !== ConnectionOptionSpecialType.authType && diff --git a/src/sql/workbench/contrib/objectExplorer/browser/serverTreeView.ts b/src/sql/workbench/contrib/objectExplorer/browser/serverTreeView.ts index 662ffc2564..948fd8ad04 100644 --- a/src/sql/workbench/contrib/objectExplorer/browser/serverTreeView.ts +++ b/src/sql/workbench/contrib/objectExplorer/browser/serverTreeView.ts @@ -276,6 +276,7 @@ export class ServerTreeView extends Disposable implements IServerTreeView { if (profile) { newProfile = profile; } + groups.forEach(group => group.dispose()); } const currentSelections = this._tree!.getSelection(); diff --git a/src/sql/workbench/services/connection/browser/connectionDialogService.ts b/src/sql/workbench/services/connection/browser/connectionDialogService.ts index 2ab518c9d6..a0e5354b68 100644 --- a/src/sql/workbench/services/connection/browser/connectionDialogService.ts +++ b/src/sql/workbench/services/connection/browser/connectionDialogService.ts @@ -480,6 +480,9 @@ export class ConnectionDialogService implements IConnectionDialogService { this._connectionDialog.onFillinConnectionInputs((input) => this.handleFillInConnectionInputs(input as IConnectionProfile)); this._connectionDialog.onResetConnection(() => this.handleProviderOnResetConnection()); this._connectionDialog.render(); + this._connectionDialog.onClosed(() => { + this._model?.dispose(); + }); } this._connectionDialog.newConnectionParams = params; this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[this._currentProviderType]); diff --git a/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts b/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts index c2c698a98e..effdecb0cf 100644 --- a/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts +++ b/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts @@ -8,7 +8,14 @@ import { WorkbenchAsyncDataTree } from 'vs/platform/list/browser/listService'; import { FuzzyScore } from 'vs/base/common/filters'; import { TreeNode } from 'sql/workbench/services/objectExplorer/common/treeNode'; import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile'; +import { IAsyncDataTreeViewState } from 'vs/base/browser/ui/tree/asyncDataTree'; -export class AsyncServerTree extends WorkbenchAsyncDataTree { } +export class AsyncServerTree extends WorkbenchAsyncDataTree { + override async setInput(input: ConnectionProfileGroup, viewState?: IAsyncDataTreeViewState): Promise { + const originalInput = this.getInput(); + await super.setInput(input, viewState); + originalInput?.dispose(); + } +} export type ServerTreeElement = ConnectionProfile | ConnectionProfileGroup | TreeNode; diff --git a/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts b/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts index dbb5b5ed8d..9fb467428c 100644 --- a/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts +++ b/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts @@ -11,7 +11,7 @@ import { IObjectExplorerService } from 'sql/workbench/services/objectExplorer/br import { NodeType } from 'sql/workbench/services/objectExplorer/common/nodeType'; import { TreeNode } from 'sql/workbench/services/objectExplorer/common/treeNode'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, isDisposable } from 'vs/base/common/lifecycle'; import { onUnexpectedError } from 'vs/base/common/errors'; import { AsyncServerTree, ServerTreeElement } from 'sql/workbench/services/objectExplorer/browser/asyncServerTree'; @@ -125,8 +125,12 @@ export class TreeUpdateUtils { let treeInput = TreeUpdateUtils.getTreeInput(connectionManagementService); if (treeInput) { - if (treeInput !== tree.getInput()) { + const originalInput = tree.getInput(); + if (treeInput !== originalInput) { return tree.setInput(treeInput).then(async () => { + if (isDisposable(originalInput)) { + originalInput.dispose(); + } // Make sure to expand all folders that where expanded in the previous session if (targetsToExpand) { await tree.expandAll(targetsToExpand);