Skip to content

Export-DbaCsv - Close the database-scoped connection it opens per call - #10634

Merged
potatoqualitee merged 2 commits into
developmentfrom
fix-exportdbacsv-connection-leak
Sep 1, 2026
Merged

Export-DbaCsv - Close the database-scoped connection it opens per call#10634
potatoqualitee merged 2 commits into
developmentfrom
fix-exportdbacsv-connection-leak

Conversation

@andreasjordan

Copy link
Copy Markdown
Collaborator

Summary

Second fix from the sleeping-connection inventory (first: #10633). Every Export-DbaCsv -SqlInstance call leaked one connection to the instance, held open until the process exits. The command's own test file left 18 sleeping sessions behind in an isolated runner measurement; the first -CheckSleepingConnections full lab run flagged it at +17.

Mechanism

The command calls Connect-DbaInstance -Database $Database, which returns a server object whose connection was opened for this call (the database-scoped clone path), streams the data reader off its raw SqlConnectionObject - and never closes the connection. The throwaway server object keeps the session checked out of the pool.

Fix

The established #10554 ownership pattern: pass IsNewConnectionReference [ref]$isNewConnection to Connect-DbaInstance and close via Disconnect-DbaInstance only when it reports the connection as newly opened. The disconnect sits in a finally so it also runs when the export fails (Stop-Function -Continue leaves the try block) and when -WhatIf skips the export - two paths where the sibling pattern in Invoke-DbaQuery places the disconnect after the catch and would miss it.

Tests

New context counts sleeping dbatools sessions through a server object opened once (a per-call counting command would open connections of its own), with a warm-up call before the baseline, asserting three further calls add nothing. Verified red on the unfixed command ("Expected 14, but got 17") and green on the fix (16 passed, 0 failed via the lab harness on SQL03\SQL2019). Runner measurement of the whole file: 18 sleeping sessions before, 2 after.

created by Claude and reviewed by Andreas Jordan

馃 Generated with Claude Code

Every call with -SqlInstance leaked one connection: Connect-DbaInstance
is called with -Database, which returns a server whose connection was
opened for this call, and the command never closed it. One sleeping
session per call, held until the process exits. The test file left 18
sessions behind on its own; the first -CheckSleepingConnections full
run flagged it as +17.

The fix is the established #10554 pattern: ask Connect-DbaInstance via
IsNewConnectionReference whether it opened the connection and close it
with Disconnect-DbaInstance only then. The disconnect sits in a finally
so it also runs when the export fails and the catch continues with the
next instance, and when -WhatIf skips the export altogether.

The new regression test counts sleeping dbatools sessions through a
server object opened once, takes a warm-up call so the shared pooled
connection exists before the baseline, and asserts three more calls add
nothing. Verified red on the unfixed command (expected 14, got 17) and
green on the fix (16 passed, 0 failed); the runner measurement of the
whole file drops from 18 to 2.

(do Export-DbaCsv)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new finally still leaks the command-owned database connection under Export-DbaCsv -WhatIf. The connection is opened before the outer ShouldProcess; then $WhatIfPreference propagates into Disconnect-DbaInstance, whose own ShouldProcess skips .Disconnect(). This directly contradicts the PR's claim that the -WhatIf path is closed. Please force cleanup semantics on the internal ownership call (for example Disconnect-DbaInstance -WhatIf:$false -Confirm:$false -Verbose:$false) and add a repeated -WhatIf connection-count regression.

Review follow-up: Disconnect-DbaInstance supports ShouldProcess, so the propagated WhatIf preference skipped the disconnect in the finally. The ownership cleanup now passes -WhatIf:$false -Confirm:$false - it is bookkeeping for a connection this command opened, not part of the user's operation. Probed on the lab: the skip is real, but no session leaks on this stack because SMO connects lazily and -WhatIf never runs a query on the clone - the defect is latent, so the new repeated -WhatIf counting regression guards the invariant without a red-on-old.

(do Export-DbaCsv)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@andreasjordan

Copy link
Copy Markdown
Collaborator Author

Fixed in 26cb375 - the ownership disconnect now passes -WhatIf:$false -Confirm:$false, with a comment stating why: it is bookkeeping for a connection this command opened, not part of the user's operation, and Disconnect-DbaInstance supports ShouldProcess.

One finding from verifying the mechanism on the lab (Windows PowerShell 5.1, instrumented run): the ShouldProcess skip is exactly as described - isNew=True, the WhatIf preference propagates, and "What if: Disconnecting SQL Connection" fires instead of the disconnect. But no session leaks on this stack, because the defect is latent: SMO connects lazily, and under -WhatIf no query ever runs on the database-scoped clone, so ConnectionContext.IsOpen stays False and there is no server session to fail to close (the probe also showed the one-time shared base connection masquerading as a leak in a fresh process, which is what a naive repro sees). The skip does leave the connectionhash entry in place, so the fix is right as it stands - it just cannot be proven red-on-old here.

The requested repeated -WhatIf counting regression is added anyway: three -WhatIf calls after the baseline must add no sleeping session. It guards the invariant against any future path that opens the clone before ShouldProcess (an eager connect in another configuration would be caught), and it is green on both the old and the new code on this stack - stated plainly, as with the other latent sites in this workstream (#10648, #10652). Full file 17 passed / 1 documented skip via the lab harness, warning-free, lab left clean.

created by Claude and reviewed by Andreas Jordan

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated head resolves the prior blocking feedback; no material issues remain.

@potatoqualitee
potatoqualitee merged commit f808f3c into development Sep 1, 2026
22 checks passed
@potatoqualitee
potatoqualitee deleted the fix-exportdbacsv-connection-leak branch September 1, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants