Skip to content

Progress - Complete the bars of 13 more commands in finally blocks (tier 3) - #10775

Open
andreasjordan wants to merge 13 commits into
developmentfrom
progress-leaks-tier3
Open

andreasjordan wants to merge 13 commits into
developmentfrom
progress-leaks-tier3

Conversation

@andreasjordan

Copy link
Copy Markdown
Collaborator

Part of RFC #10548: the try/finally leak fixes, tier 3 of 3 (the commands with few or no Stop-Function calls inside the bar's lifetime). Same pattern as #10773 and #10774, one commit per command with its test. Independent of both.

Problem

These 13 functions complete their progress bar only on the success path, so a throw, a stopped pipeline (Ctrl+C) or Select-Object -First left the bar on screen for the rest of the session. Three of them are internal functions nested in a public command: Get-SqlFileStructure in Copy-DbaDatabase, Get-InferredSchema in Import-DbaCsv (the full scan of -DetectColumnTypes) and Test-SqlInstance in Find-DbaInstance.

Change

Each bar's completion moves into a finally around that bar's lifetime.

  • Stop-DbaDbEncryption has two bars, the sequential one (Id 0) and the one that waits for the -Parallel threads (Id 1). Each gets its own finally, as in Start-DbaDbEncryption in Progress - Complete the bars of 13 long-running commands in finally blocks (tier 1) #10773.
  • Sync-DbaLoginPermission already ran its login loop in try/finally to restore the database context; the completion moves into that finally.
  • Install-DbaParquet completed its bar by hand in five places, Save-DbaDiagnosticQueryScript in two. Those completions go away.
  • Save-DbaKbUpdate: each of its two downloads gets its own try/finally.
  • The others (Disable-DbaDbEncryption, Install-DbaSqlWatch, Remove-DbaNetworkCertificate, New-DbatoolsSupportPackage, New-DbaComputerCertificateSigningRequest and the three nested functions): one try/finally around the loop or the steps that draw the bar.

Most of the diff is re-indentation: git diff -w shows 45 lines added and 11 removed for the 13 commands.

Test

Same boundary as in #10773: the real command runs in a runspace of its own, created by the PowerShell API without a host, where Write-Progress puts every record into Streams.Progress. The test asserts that the command drew its bar and that no progress Id is left whose last record is not a completed one.

Command How the test ends the run
Stop-DbaDbEncryption Select-Object -First 1 at the first database, which the earlier contexts left not encrypted
Sync-DbaLoginPermission Select-Object -First 1 at the result of the login
Install-DbaSqlWatch pipeline stopped at the first record, before the dacpac is published (5.1 only, like the existing tests)
Remove-DbaNetworkCertificate new integration Describe on InstanceRestart, Select-Object -First 1 at the result; a certificate that was configured is put back
Save-DbaDiagnosticQueryScript new integration Describe, Select-Object -First 1 at the first downloaded script
Install-DbaParquet -Force into the test drive, pipeline stopped at the first record (while NuGet is resolved)
Save-DbaKbUpdate a download link under .invalid and -ErrorAction Stop, so the failed download throws out of the command; needs neither the catalog nor the internet. Without -ErrorAction Stop the old code completed the bar as well: Invoke-WebRequest reports the failure as an error that ends only its own statement, so the next line still ran
New-DbatoolsSupportPackage new integration Describe, pipeline stopped at the first record while the information is collected
New-DbaComputerCertificateSigningRequest Select-Object -First 1 at the first file; the request is removed from LocalMachine\REQUEST like the others
Import-DbaCsv a 200,000-row file with -DetectColumnTypes, pipeline stopped at the first record of the type detection
Find-DbaInstance the existing scan of InstanceSingle, pipeline stopped at the first record of the computer
Disable-DbaDbEncryption no new test, see below
Copy-DbaDatabase no new test, see below

No automated test for two of them:

  • Disable-DbaDbEncryption draws its bar only while it waits for the decryption, and the decryption of a test database is done within the first second, before the first record. Trace flag 5004 holds the scan (checked in the lab: state 5, 0 percent, even after TRACEOFF until the scan is restarted), but it works for the whole instance, and a run that dies between TRACEON and TRACEOFF would leave every later TDE test waiting. ALTER DATABASE ... SET ENCRYPTION SUSPEND per database does not help: the decryption is done before it can be suspended, and SMO issues the ALTER again, which SQL Server refuses for a database whose encryption is already off. If a test with trace flag 5004 is wanted, I can add it.
  • Copy-DbaDatabase: the loop of Get-SqlFileStructure takes milliseconds per database and throws nothing that a test could provoke, so neither a stop nor a throw can be placed inside it.

The existing tests of both pass.

The -Parallel bar of Stop-DbaDbEncryption runs its threads in a runspace pool on $Host, like Start-DbaDbEncryption in #10773, so the API host does not see its records either. I checked it the same way, in a real console window after Stop-DbaDbEncryption -Parallel | Select-Object -First 1 against one unencrypted test database: the old command leaves "Disabling encryption" (Id 1) behind on 5.1 and 7.6, the new one does not.

Results (lab, InstanceSingle SQL Server 2019, Multi1 2025, Multi2/Restart 2022):

  • New command: the 11 changed test files and the existing tests of Copy-DbaDatabase and Disable-DbaDbEncryption pass on PowerShell 5.1 and 7.6, without warnings. The integration Describe of Install-DbaSqlWatch is skipped on pwsh, as before.
  • Old command: in every file exactly the new progress test fails, on both editions (Install-DbaSqlWatch on 5.1), with one exception: Install-DbaParquet fails on 5.1 only. On 7.x its cleanup of the temp folder (Remove-Item -Recurse) writes a completed record of its own for Id 0, which clears the leftover bar by accident; on 5.1 the bar stays.
  • Once, the existing install context of Install-DbaSqlWatch failed with a deadlock (Msg 1205) inside SqlWatch's own post-deployment script; the three runs after it passed. Not related to this change.
  • TestTestfiles.Tests.ps1 and TestTestLayout.ps1 report nothing for the 11 changed test files.

Left open

  • The sequential Stop-DbaDbEncryption calls Disable-DbaDbEncryption for each encrypted database, and both draw their bar through Write-ProgressHelper with Id 0. The completion of the inner bar therefore also removes the outer one until the next database redraws it. That is the shared-Id question of the RFC, not a leak, so it is not changed here.

🤖 Generated with Claude Code

andreasjordan and others added 13 commits October 5, 2026 18:00
The sequential bar was completed after the loop over the databases, the bar
that waits for the -Parallel threads after the retrieval loop. A throw, a
stopped pipeline and Select-Object -First skipped both. Each loop now runs in
try/finally and its finally completes its own bar.

(do Stop-DbaDbEncryption)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar is drawn while the command waits for the decryption and was completed
after that wait only. A throw or a stopped pipeline during the wait left it on
screen. The wait now runs in try/finally and the finally completes the bar.

(do Disable-DbaDbEncryption)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of each destination was completed after the loop over the logins, at
the end of the try that already restores the database context. A throw, a
stopped pipeline and Select-Object -First at the result of a login skipped it.
The completion moves into the finally of that try.

(do Sync-DbaLoginPermission)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed after the loop over the instances only. A throw, a
stopped pipeline during the long publish of the dacpac and Select-Object -First
at the result skipped it. The loop now runs in try/finally and the finally
completes the bar.

(do Install-DbaSqlWatch)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… block

The bar of each instance was completed at the end of its loop iteration. The
Stop-Function -Continue when the remote call fails, a throw and Select-Object
-First at the result skipped it. The steps from the first bar on now run in
try/finally and the finally completes the bar.

(do Remove-DbaNetworkCertificate)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y block

The bar was completed after the loop over the scripts and by hand in the catch.
A stopped pipeline during a download and Select-Object -First at the first
saved file skipped it. The loop now runs in try/finally and the finally
completes the bar; the completion in the catch goes away.

(do Save-DbaDiagnosticQueryScript)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed by hand in five places. A stopped pipeline while the
NuGet packages were resolved and downloaded, or an exception outside the
existing try, left it on screen. The work from the first progress step on now
runs in try/finally and the finally completes the bar; the five hand-written
completions go away.

(do Install-DbaParquet)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of a download was completed after Invoke-WebRequest only. A failed
download ends just that statement, so the completion still ran, but with
-ErrorAction Stop of the caller the failure throws out of the command, and a
stopped pipeline during the download ends it as well; both left the bar on
screen. Each of the two downloads now runs in try/finally and the finally
completes its bar.

(do Save-DbaKbUpdate)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lock

The bar was completed after the information was collected. A throw or a
stopped pipeline during the collection left it on screen. The collection now
runs in try/finally and the finally completes the bar.

(do New-DbatoolsSupportPackage)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in a finally block

The bar of each computer was completed after the files were returned. A throw,
a stopped pipeline and Select-Object -First at the first file skipped it. The
work on each computer now runs in try/finally and the finally completes the bar.

(do New-DbaComputerCertificateSigningRequest)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a finally block

The bar of the internal Get-SqlFileStructure was completed after its loop over
the databases. A throw inside the loop left it on screen. The loop now runs in
try/finally and the finally completes the bar.

(do Copy-DbaDatabase)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…finally block

The bar of the full scan behind -DetectColumnTypes (Id 2) was completed after
the scan only. A stopped pipeline or a failing scan left it on screen. The scan
now runs in try/finally and the finally completes the bar.

(do Import-DbaCsv)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nally block

The internal Test-SqlInstance completed the bar of a computer after the scan of
that computer. A throw or a stopped pipeline during the scan, which can take
seconds per computer, left it on screen. The scan now runs in try/finally and
the finally completes the bar.

(do Find-DbaInstance)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant