Skip to content

Connect-DbaInstance - Give the cloned server ownership of its database connection #10584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andreasjordan wants to merge 2 commits into
base: development
Choose a base branch
from
+135 −11
Open

Connect-DbaInstance - Give the cloned server ownership of its database connection #10584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bdaaeb
Connect-DbaInstance - Give the cloned server ownership of its databas…
andreasjordan Aug 16, 2026
bf69183
Connect-DbaInstance - Put settings a fixed connection string blocks i…
andreasjordan Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 67 additions & 11 deletions public/Connect-DbaInstance.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,35 +750,91 @@ function Connect-DbaInstance {
} elseif ($copyContext) {
$isNewConnection = $true
$connContext = $inputObject.ConnectionContext.Copy()
# A ServerConnection that was built from a SqlConnection has its connection string set
# explicitly, and SMO then refuses to let some of its properties be assigned:
# Property <name> cannot be changed or read after a connection string has been set.
# DatabaseName, NonPooledConnection and ServerInstance are all in that group, which is
# why -Database, -NonPooledConnection and -DedicatedAdminConnection used to fail on a
# server that was created from a SqlConnection. Those settings are collected here and
# put into the connection string further down instead, where the same three are
# Initial Catalog, Pooling and Data Source. No readable property tells the two kinds of
# context apart - every one of them reads fine on both - so we find out by trying.
$connectionStringKeyword = @{ }
if ($ApplicationIntent) {
$connContext.ApplicationIntent = $ApplicationIntent
}
if ($NonPooledConnection) {
$connContext.NonPooledConnection = $true
try {
$connContext.NonPooledConnection = $true
} catch {
$connectionStringKeyword["Pooling"] = $false
}
}
if (Test-Bound -Parameter StatementTimeout) {
$connContext.StatementTimeout = $StatementTimeout
}
if ($DedicatedAdminConnection -and $inputObject.ConnectionContext.ServerInstance -notmatch '^ADMIN:') {
if ($DedicatedAdminConnection -and $inputObject.ConnectionContext.ServerInstance -notmatch "^ADMIN:") {
if ($instance.IsLocalHost) {
# Use localhost to avoid multiple IP resolution on multi-homed servers (issue #10151)
if ($instance.InstanceName -ne 'MSSQLSERVER') {
$connContext.ServerInstance = "ADMIN:localhost\$($instance.InstanceName)"
if ($instance.InstanceName -ne "MSSQLSERVER") {
$dedicatedAdminServerInstance = "ADMIN:localhost\$($instance.InstanceName)"
} else {
$connContext.ServerInstance = "ADMIN:localhost"
$dedicatedAdminServerInstance = "ADMIN:localhost"
}
# Trust the server certificate because 'localhost' may not match the certificate CN (e.g., FQDN), issue #10254
$connContext.TrustServerCertificate = $true
} else {
$connContext.ServerInstance = 'ADMIN:' + $connContext.ServerInstance
$dedicatedAdminServerInstance = "ADMIN:$($connContext.ServerInstance)"
}
try {
$connContext.ServerInstance = $dedicatedAdminServerInstance
} catch {
$connectionStringKeyword["Data Source"] = $dedicatedAdminServerInstance
}
try {
$connContext.NonPooledConnection = $true
} catch {
$connectionStringKeyword["Pooling"] = $false
}
$connContext.NonPooledConnection = $true
}
if ($Database) {
# Save StatementTimeout because it might be reset on GetDatabaseConnection
$savedStatementTimeout = $connContext.StatementTimeout
$connContext = $connContext.GetDatabaseConnection($Database, $false)
$connContext.StatementTimeout = $savedStatementTimeout
# We set DatabaseName instead of calling GetDatabaseConnection on purpose.
# GetDatabaseConnection opens the connection on the copy we are holding and returns
# a *different* ConnectionContext, so the server we build below never owns that
# connection. Disconnect-DbaInstance can only reach the context of the server it is
# given, so nothing ever closed it: the session stayed open for the life of the
# process, holding a shared lock on the database. On model that is enough to make a
# later CREATE DATABASE on the same instance fail on the exclusive lock.
# Setting DatabaseName keeps the connection with the context we hand to the server,
# which is also what the connection string code paths of this command already do.
# It does not reset StatementTimeout either, so the save and restore that
# GetDatabaseConnection needed is gone with it.
try {
$connContext.DatabaseName = $Database
} catch {
# Falling back to GetDatabaseConnection here would bring back the very leak this
# change is about, so the database goes into the connection string instead. As
# Initial Catalog it is part of the pool key, which is what setting DatabaseName
# achieves as well, so the reason #9505 forced a non pooled connection still holds.
$connectionStringKeyword["Initial Catalog"] = $Database
}
}
if ($connectionStringKeyword.Count -gt 0) {
$changedKeywords = $connectionStringKeyword.Keys -join ", "
Write-Message -Level Debug -Message "Connection context does not accept property assignments, using the connection string for: $changedKeywords"
# The copy has to be disconnected first: setting the connection string of an open
# connection does not move it, and CurrentDatabase would silently stay where it was.
# The copy is a session of its own - a different SPID than the server that was passed
# in - so this does not touch the connection of the caller.
$connContext.Disconnect()
$connectionStringBuilder = New-Object -TypeName Microsoft.Data.SqlClient.SqlConnectionStringBuilder -ArgumentList $connContext.ConnectionString
foreach ($keyword in $connectionStringKeyword.Keys) {
# PowerShell routes property assignments on a connection string builder through its
# dictionary indexer, so the keyword has to be used rather than the property name:
# $connectionStringBuilder.InitialCatalog fails with "Keyword not supported".
$connectionStringBuilder[$keyword] = $connectionStringKeyword[$keyword]
}
$connContext.ConnectionString = $connectionStringBuilder.ConnectionString
}
$server = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server -ArgumentList $connContext
if ($Database -and $server.ConnectionContext.CurrentDatabase -ne $Database) {
Expand Down
68 changes: 68 additions & 0 deletions tests/Connect-DbaInstance.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,46 @@ Describe $CommandName -Tag IntegrationTests {
}
}

Context "connection is properly cloned from a connection that was created from a SqlConnection" {
BeforeAll {
# A ServerConnection that was built from a SqlConnection has its connection string set, and
# SMO then refuses assignments to DatabaseName, NonPooledConnection and ServerInstance, so
# cloning such a server has to go through the connection string instead. CI never noticed
# because it covers SqlConnection to Server and Server to another Database, but never the
# two of them chained. See #10584.
[Microsoft.Data.SqlClient.SqlConnection]$sqlConnectionToClone = "Data Source=$($TestConfig.InstanceMulti1);Integrated Security=True;Encrypt=False;Trust Server Certificate=True"
$serverFromSqlConnection = Connect-DbaInstance -SqlInstance $sqlConnectionToClone
}

AfterAll {
$null = $serverFromSqlConnection | Disconnect-DbaInstance
}

It "clones when using parameter Database" {
$serverClone = Connect-DbaInstance -SqlInstance $serverFromSqlConnection -Database tempdb
$serverClone.ConnectionContext.ExecuteScalar("select db_name()") | Should -Be "tempdb"
$serverFromSqlConnection.ConnectionContext.ExecuteScalar("select db_name()") | Should -Be "master"
$null = $serverClone | Disconnect-DbaInstance
}

It "clones when using parameter Database together with NonPooledConnection" {
$serverClone = Connect-DbaInstance -SqlInstance $serverFromSqlConnection -Database tempdb -NonPooledConnection
$serverClone.ConnectionContext.ExecuteScalar("select db_name()") | Should -Be "tempdb"
$serverClone.ConnectionContext.ConnectionString | Should -Match "Pooling=False"
$null = $serverClone | Disconnect-DbaInstance
}

It "clones when using parameter DedicatedAdminConnection" {
$serverClone = Connect-DbaInstance -SqlInstance $serverFromSqlConnection -DedicatedAdminConnection
$serverClone.ConnectionContext.ConnectionString | Should -Match "ADMIN:"
# The connection context cannot be asked here, because ServerInstance is one of the properties
# that cannot be assigned on such a context, so ask the session which endpoint it is on.
$dacQuery = "SELECT COUNT(*) FROM sys.dm_exec_sessions AS s JOIN sys.endpoints AS e ON e.endpoint_id = s.endpoint_id WHERE e.is_admin_endpoint = 1 AND s.session_id = @@SPID"
$serverClone.ConnectionContext.ExecuteScalar($dacQuery) | Should -Be 1
$null = $serverClone | Disconnect-DbaInstance
}
}

Context "connection is properly cloned from an existing connection" {
BeforeAll {
$server = Connect-DbaInstance -SqlInstance $TestConfig.InstanceMulti1
Expand All @@ -540,12 +580,40 @@ Describe $CommandName -Tag IntegrationTests {
$serverClone.ConnectionContext.ExecuteScalar("select db_name()") | Should -Be "tempdb"
}

It "hands the clone a connection that Disconnect-DbaInstance can close" {
# The clone used to be built with GetDatabaseConnection, which opened the connection on an
# intermediate copy and returned a different context. The clone therefore did not own its
# connection and Disconnect-DbaInstance closed nothing, so every call left a session parked
# in the database holding a shared lock on it.
# Repeated on purpose. With pooling, a sleeping session that stays behind is legitimate - it
# belongs to the pool and the next call reuses it. What must not happen is that their number
# grows with every call, which is what an orphaned connection looks like: nothing can reuse
# it and nothing can close it. Against the old implementation the count went up on almost
# every cycle, so a single cycle is not enough to tell the two apart.
$countPerCycle = @()
foreach ($cycle in 1..5) {
$serverParked = Connect-DbaInstance -SqlInstance $server -Database tempdb
$serverParked.ConnectionContext.ExecuteScalar("select db_name()") | Should -Be "tempdb"
$null = $serverParked | Disconnect-DbaInstance
$countPerCycle += @(Get-DbaProcess -SqlInstance $server -Database tempdb | Where-Object Program -match "dbatools").Count
}

$countPerCycle[-1] | Should -Be $countPerCycle[0]
}

It "clones when using parameter ApplicationIntent" {
$serverClone = Connect-DbaInstance -SqlInstance $server -ApplicationIntent ReadOnly
$server.ConnectionContext.ApplicationIntent | Should -BeNullOrEmpty
$serverClone.ConnectionContext.ApplicationIntent | Should -Be "ReadOnly"
}

It "clones when using parameter Database together with NonPooledConnection" {
$serverClone = Connect-DbaInstance -SqlInstance $server -Database tempdb -NonPooledConnection
$serverClone.ConnectionContext.ExecuteScalar("select db_name()") | Should -Be "tempdb"
$serverClone.ConnectionContext.NonPooledConnection | Should -Be $true
$null = $serverClone | Disconnect-DbaInstance
}

It "clones when using parameter NonPooledConnection" {
$serverClone = Connect-DbaInstance -SqlInstance $server -NonPooledConnection
$server.ConnectionContext.NonPooledConnection | Should -Be $false
Expand Down