Skip to content

Database-scoped SMO calls silently change the current database of the shared connection #10555

Description

@andreasjordan

Summary

$db.Query(...), $db.Invoke(...), $db.ExecuteNonQuery(...) and $db.ExecuteWithResults(...) on an SMO Database object do not run on a private connection. SMO's Database.ExecutionManager.ConnectionContext is the parent server's connection context, so these calls issue a USE [database] on the connection the caller owns and never switch back.

Every command that uses this pattern therefore hands the connection back to the caller pointing at a different database. Anything the caller runs afterwards through $server.ConnectionContext (or through any dbatools command that reuses that open connection) executes in the wrong database.

Proof that the execution manager is not a private connection:

$server = Connect-DbaInstance -SqlInstance $instance -NonPooledConnection
$db = $server.Databases["SomeDatabase"]
[object]::ReferenceEquals($db.ExecutionManager.ConnectionContext, $server.ConnectionContext)
# True

Steps to Reproduce

$server = Connect-DbaInstance -SqlInstance $instance -NonPooledConnection
$server.ConnectionContext.ExecuteScalar("SELECT DB_NAME()")     # master

$null = $server.Databases["SomeDatabase"].ExecuteNonQuery("SELECT 1")
$server.ConnectionContext.ExecuteScalar("SELECT DB_NAME()")     # SomeDatabase  <-- leaked

$null = $server.Databases["SomeDatabase"].Query("SELECT 1")
$server.ConnectionContext.ExecuteScalar("SELECT DB_NAME()")     # SomeDatabase  <-- leaked

Verified on SQL Server 2019 and 2022, dbatools 2.8.4 (PowerShell Gallery, Windows PowerShell 5.1) and the current development build (PowerShell 7.6), with Windows authentication and with a -SqlCredential holding a Windows account, on pooled and non-pooled connections. $db.Refresh() beforehand makes no difference.

$server.Query($sql, $database) is affected as well: the two-argument overload in xml/dbatools.Types.ps1xml routes through $this.Databases[$Database].ExecuteWithResults($Query), so it leaks exactly like the Database methods.

Why this is only sometimes visible

Two things mask the leak, which is why it has survived this long:

  1. Pooled connections that are currently closed. The leak lives on the ServerConnection; when the connection object is closed between calls, the next ExecuteScalar reconnects at the default database and the leak is gone. Non-pooled connections (Connect-DbaInstance -NonPooledConnection) and pooled connections that are still open keep it.
  2. Invoke-DbaQuery closing connections it does not own (Commands disconnect SQL Server connections they do not own #10554). Any dbatools command that ends by calling Invoke-DbaQuery -SqlInstance $server -Database <x> can close the caller's non-pooled connection, and the transparent reconnect resets the current database - wiping the leak by accident.

The second point makes the leak look unreproducible, because whether it survives depends on which command runs next. This provisioning script is what surfaced it: one shared connection, several commands against the same user database.

$server = Connect-DbaInstance -SqlInstance $sqlInstance -SqlCredential $Credential -NonPooledConnection

$null = Mount-DbaDatabase -SqlInstance $server -Database StackOverflow -FileStructure $fileStructure -DatabaseOwner sa
$null = Invoke-DbaDbUpgrade -SqlInstance $server -Database StackOverflow -NoCheckDb
$null = Set-DbaDbRecoveryModel -SqlInstance $server -Database StackOverflow -RecoveryModel Full
$null = Invoke-DbaQuery -SqlInstance $server -Database master -Query "ALTER DATABASE [StackOverflow] SET READ_COMMITTED_SNAPSHOT ON WITH ROLLBACK IMMEDIATE"
$null = Set-DbaDbQueryStoreOption -SqlInstance $server -Database StackOverflow -State ReadWrite

The attached database comes in at compatibility level 100, so Invoke-DbaDbUpgrade actually does work and leaks. Set-DbaDbRecoveryModel then repairs the connection - not on purpose, it just happens to reach Invoke-DbaQuery with the database the connection is currently stuck on. Reproducing the same sequence against a test database, checking $server.ConnectionContext.ExecuteScalar("SELECT DB_NAME()") after every step:

ok      baseline (compatibility level 100)                   DB_NAME()=master
LEAKED  Invoke-DbaDbUpgrade -NoCheckDb                       DB_NAME()=dbatoolsci_qscontext
ok      Set-DbaDbRecoveryModel                               DB_NAME()=master
ok      Invoke-DbaQuery -Database master (RCSI)              DB_NAME()=master
ok      Set-DbaDbQueryStoreOption -State ReadWrite           DB_NAME()=master

Reorder those lines, drop one of them, or run the same script against a database that does not need upgrading, and the leak moves somewhere else or disappears entirely.

A caller who tries to observe it with Invoke-DbaQuery -SqlInstance $server -Database master -Query "SELECT DB_NAME()" will never see it either: because master differs from the leaked current database, dbatools builds a separate connection and always reports master. The reliable check is $server.ConnectionContext.ExecuteScalar("SELECT DB_NAME()").

Scope

Rough inventory over public/ and private/ (excluding tests):

  • ~97 database-scoped call sites ($db*.Query/Invoke/ExecuteNonQuery/ExecuteWithResults, $server.Databases[...].*) in ~52 files
  • ~29 additional $server.Query($sql, $database) style call sites
  • the heaviest users: Invoke-DbaDbDataMasking (12), Copy-DbaSystemDbUserObject (5), Set-DbaTempDbConfig (4), Invoke-DbaDbUpgrade (4), Copy-DbaDbAssembly (4)

Two concrete instances are filed separately:

Possible fixes

Worth deciding centrally before touching individual commands:

  1. Fix the dbatools wrappers. Database.Query and Database.Invoke are our own ScriptMethods in xml/dbatools.Types.ps1xml. They could run on a copied connection context ($this.Parent.ConnectionContext.Copy().GetDatabaseConnection($this.Name)) or restore the previous current database afterwards. That fixes every call site that goes through them at once, including Server.Query($sql, $db).
  2. Replace direct SMO calls per call site. $db.ExecuteNonQuery(...) / $db.ExecuteWithResults(...) are SMO's own methods and cannot be shadowed, so those call sites have to be rewritten - either to the fixed wrapper, or to a server-level query when the statement does not need the database context at all (ALTER DATABASE ... is the common case).
  3. Do nothing and document it. Not attractive: the connection is the caller's, and a command that silently repoints it is surprising in exactly the scripting scenarios dbatools is used for.

Option 1 plus targeted option 2 work looks like the sensible split.

How to verify a fix

A regression test per fixed command in the shape of:

$server = Connect-DbaInstance -SqlInstance $instance -NonPooledConnection
$null = <command under test> -SqlInstance $server -Database $dbName ...
$server.ConnectionContext.ExecuteScalar("SELECT DB_NAME()") | Should -Be "master"

-NonPooledConnection matters - with a pooled connection the assertion passes for the wrong reason.


This text was created by Claude and reviewed by Andreas Jordan.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions