Skip to content

Database context - SMO Create/Drop and collection enumeration still move the caller's connection#10604

Description

@andreasjordan

Continuation of #10555, which closes with #10603. The wrapper mechanism that issue asked for is merged: the four script methods restore the caller's database (#10579), statements that never needed a database context run on the server connection (#10580), Invoke-DbaQuery re-checks the context on connection reuse (#10564), and every direct SMO ExecuteNonQuery/ExecuteWithResults on a Database object is fixed or deliberately accounted for (#10603, which also adds the shared helper Restore-DatabaseContext).

Two surfaces remain, and they are the large ones. Before any batch starts, there is a decision to make - it is the first section on purpose.

The question to settle first: what is the contract?

Both remaining surfaces are too large to fix casually, and neither can be inventoried by reading code. Two positions, both defensible:

  1. "No dbatools command moves the database context of your connection." Exhaustive coverage: every command below gets the capture/restore treatment plus a regression test. Honest estimate: at the pace of Database context - Add Restore-DatabaseContext and fix eighteen commands聽#10603 (eighteen commands), on the order of ten more pull requests - and the guarantee regresses silently, because nothing stops a new command (or a new collection touch added outside an existing try/finally) from leaking again except that one command's test.
  2. "dbatools guarantees the context wherever it consumes one, and fixes leaks where they bite." The Connect-DbaInstance / Invoke-DbaQuery - Do not close connections of the caller and do not reuse them for the wrong database聽#10564 half is the systemic guarantee; per-command restores are applied by priority: commands called internally by other commands on a shared connection, and commands users demonstrably run against kept or non-pooled connections. The rest is fixed opportunistically as commands are touched.

Whichever way this lands, the pattern should become policy: a short section in tests/CLAUDE.md ("commands touching database level SMO collections capture CurrentDatabase before any work and restore it in a finally; the regression test uses a non-pooled connection") is cheap and makes the fixed state defensible going forward.

Bucket C - SMO's own Create() / Drop() / some Alter()

SMO scripts a USE in front of server level object operations and never puts the connection back. Measured on SQL Server 2019 (details in #10555):

Target context Commands
master New-DbaLogin, Remove-DbaLogin, New-DbaCredential, Remove-DbaCredential, New-DbaServerRole, Remove-DbaServerRole, New-DbaLinkedServer, Remove-DbaLinkedServer, Remove-DbaDatabase, New-DbaEndpoint
msdb New-DbaAgentJob, Remove-DbaAgentJob, Set-DbaAgentJob

No rule predicts membership: Remove-DbaEndpoint does not leak while New-DbaEndpoint does, Set-DbaAgentJob leaks although it is an Alter, Agent objects go to msdb rather than master. So the ~68 public commands that call .Create()/.Drop() on non-database objects are candidates that must each be measured, not a fix list. The unmeasured families: the rest of Agent (schedules, job steps, operators, alerts, proxies, categories), Database Mail, audits and audit specifications, Resource Governor, server triggers, Extended Events sessions, availability groups, and the Copy-Dba* commands that create objects on the destination.

Bucket D - enumerating a database level collection

A property getter, not a call, so no script method can shadow it:

db.Views                      leaked        db.Size (a plain property)   ok
db.Tables                     leaked        db.Refresh()                 ok
db.StoredProcedures           leaked
db.Users                      leaked        Get-DbaDbView                leaked
db.Schemas                    leaked        Get-DbaDbTable               leaked
db.Roles                      leaked        Get-DbaDbUser                leaked
db.FileGroups                 leaked
db.Tables[0].Columns          leaked

A grep for database level collection access hits ~86 files after #10603 - and that is a floor, not a count: pipeline-passed Database objects and rarer collections escape any grep. For scale, there are 66 Get-DbaDb* commands alone, and the three measured all leaked. Read only commands are the worst offenders precisely because reading is supposed to be harmless.

The mechanism to reuse

private/functions/Restore-DatabaseContext.ps1 (#10603) is the fix pattern: read ConnectionContext.CurrentDatabase before any work starts (reading it later records a database an earlier statement already leaked into), restore in a finally, one call per caller connection - commands with a source and destinations restore each. A failing restore warns rather than throws, with interrupting warning preferences neutralised, because housekeeping must never replace the outcome of the command. The regression test pattern is in every #10603 test file: non-pooled caller connection, the command doing real work, DB_NAME() compared before and after.

Two SMO facts worth knowing before writing tests: a populated collection is cached, so $db.Users moves the connection only the first time on a given connection - a test reusing a collection as its leak fixture silently stops proving anything; and a pooled connection that closes between calls reconnects at its default database, hiding any leak.


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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions