Skip to content

Progress - Complete the bars of 13 long-running commands in finally blocks (tier 1) - #10773

Open
andreasjordan wants to merge 14 commits into
developmentfrom
progress-leaks-tier1
Open

andreasjordan wants to merge 14 commits into
developmentfrom
progress-leaks-tier1

Conversation

@andreasjordan

@andreasjordan andreasjordan commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Part of RFC #10548: the try/finally leak fixes, tier 1 of 3 (the commands with many Stop-Function calls inside the bar's lifetime). Same pattern as #10762, #10763 and #10764, one commit per command with its test.

Problem

These 13 commands complete their progress bar only on the success path. A bar that is not completed stays on screen for the rest of the session. It stayed when the command threw under -EnableException, when the pipeline was stopped (Ctrl+C) or ended early (Select-Object -First), and in several commands also in plain warning mode, because a Stop-Function -Continue or a continue skipped the completion at the end of the loop body:

Command Warning-mode path that left the bar on screen
Invoke-DbaDbLogShipRecovery a database that is not a log shipping secondary
Install-DbaInstance a setup folder without a setup file, a pending reboot, no remote connection
Update-DbaInstance computer not reachable, no SQL Server found, pending reboot, update not applicable
Invoke-DbaAdvancedInstall the configuration file cannot be copied to the target, the installation fails
Invoke-DbaAdvancedUpdate the extraction or the update fails
Set-DbaNetworkCertificate certificate already configured ("No changes needed"), no suitable certificate, certificate not suitable
New-DbaComputerCertificate the CA does not issue the certificate, the import on a remote computer fails

Change

Each bar's completion moves into a finally around that bar's lifetime. No registry, no Stop-Function integration.

  • Where a bar lives per loop iteration (LogShipRecovery, Install/Update-DbaInstance, Set-DbaNetworkCertificate, New-DbaComputerCertificate), the try sits inside the loop. Where it lives across a loop (DataMasking per table, Mirroring, Start-DbaDbEncryption, Reset-DbaAdmin, Install-DbaMaintenanceSolution, Install-DbaSqlPackage and the two Advanced commands), it sits around it.
  • Start-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.
  • Install-DbaSqlPackage completed its bar by hand in front of each of its 9 Stop-Function calls. Those completions go away.
  • Install-DbaInstance: the old completion was the only Write-Progress -Complete in the tree; it worked only as an abbreviation of -Completed. It goes away with the move into the finally, which uses -Completed.

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

Test

Each command got a new integration context that runs the real command in a runspace of its own, created by the PowerShell API without a host, where Write-Progress puts every record into Streams.Progress. It asserts that the command drew its own bar and that no progress Id is left whose last record is not a completed one.

Command How the test ends the run
Invoke-DbaDbDataMasking pipeline stopped at the first masking record
Invoke-DbaDbMirroring -WhatIf, pipeline stopped at the first record
Invoke-DbaDbLogShipRecovery runs to the end on master, which is never a log shipping secondary (warning path)
Install-DbaInstance runs to the end against a computer under .invalid, where the check for a pending reboot fails (warning path, -WhatIf only as a guard). The first version used the local computer and an empty setup folder; on the CI runner it stopped one check earlier because a reboot was pending, so the second commit for this command moved the test to a computer that cannot be reached on any machine
Update-DbaInstance runs to the end against a computer name under .invalid (warning path, -WhatIf)
Invoke-DbaAdvancedInstall runs to the end against a computer name under .invalid, so the configuration file cannot be copied
Invoke-DbaAdvancedUpdate runs to the end with an installer that does not exist, so the extraction fails
Install-DbaSqlPackage -Force into the test drive, pipeline stopped at the first record (during the download)
Start-DbaDbEncryption Select-Object -First 1: stops after the first of two databases (asserted)
Reset-DbaAdmin the existing reset on InstanceRestart now runs in the runspace and ends with Select-Object -First 1 at the login, so no second restart
Set-DbaNetworkCertificate runs to the end with a thumbprint that does not exist (warning path)
New-DbaComputerCertificate -SelfSigned, Select-Object -First 1 at the certificate
Install-DbaMaintenanceSolution Select-Object -First 1 at the result

Two things changed against the #10764 pattern, both found while writing these tests:

  • The last record decides. Windows PowerShell completes its own "Preparing modules for first use." bar with Id 0 while it loads a module. With the old assertion (any completed record per Id), the old Install-DbaInstance passed on 5.1 although it drew bar 0 again afterwards and left it open. The stop tests also wait for a record of the command itself, not for the first record of any kind. (New-DbaAvailabilityGroup.Tests.ps1 from New-DbaAvailabilityGroup - Complete the progress bar in a finally block #10764 still has the old form; it was red on the old command, so it is left alone here.)
  • The runspace imports the manifest. dbatools.psm1 imported without a command line skips the type data (it checks $MyInvocation.Line -like '*.psm1*'), so in the new runspace SMO databases had no Query() method and DataMasking did nothing.

One bar cannot be seen this way: the -Parallel bar of Start-DbaDbEncryption. The command runs its threads in a runspace pool on $Host, and once a thread has run there, the warning, verbose and progress records of the calling pipeline no longer reach the Streams of an API host. I checked that bar in a real console window instead, reading the ConsoleHost's pending bars by reflection after Start-DbaDbEncryption -Parallel | Select-Object -First 1: the old command leaves "Enabling encryption on ..." (Id 1) behind on 5.1 and 7.6, the new one does not.

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

  • New command: all 13 test files pass on PowerShell 5.1 and 7.6, without warnings. The integration Describe of Reset-DbaAdmin is skipped on pwsh, as before.
  • Old command: in every file exactly the new "Completes its progress bar" test fails, on both editions (Reset-DbaAdmin on 5.1).
  • TestTestfiles.Tests.ps1 and TestTestLayout.ps1 report nothing for the 13 test files.

Left open

  • Install-DbaMaintenanceSolution with -ReplaceExisting already draws its bar in begin, before it downloads the solution. That bar spans begin and process, which the RFC listed as needing its own lifetime strategy, so a Ctrl+C during the download still leaves it. Not changed here.
  • The runspace pool on $Host (also in Read-DbaBackupHeader, so in Restore-DbaDatabase) is a defect of its own for anyone who runs dbatools through the PowerShell API: their warnings and verbose messages vanish after the pool ran. Not part of this PR.

🤖 Generated with Claude Code

andreasjordan and others added 13 commits October 5, 2026 16:22
The bar is drawn per table in the row loop and was completed only after the
loop over the databases. A throw under EnableException, a return from the row
loop or a stopped pipeline left it on screen. The row loop now runs in
try/finally and the finally completes the bar, so the completion after the
database loop goes away.

(do Invoke-DbaDbDataMasking)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed only after the loop over the databases. A throw under
EnableException, a stopped pipeline or Select-Object -First left it on
screen. The loop now runs in try/finally and the finally completes the bar.

(do Invoke-DbaDbMirroring)

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

The bar of each database was completed at the end of its loop iteration. A
database that is not a log shipping secondary warns with Stop-Function
-Continue, which skipped the completion and left the bar on screen even
without EnableException; a throw, a return or a stopped pipeline did the same.
The work on each database now runs in try/finally and the finally completes
the bar.

(do Invoke-DbaDbLogShipRecovery)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar is drawn before the loop over the computers and was completed at the
end of each iteration. Every Stop-Function -Continue in the loop, such as a
setup folder without a setup file, skipped the completion and left the bar on
screen, as did a throw or a stopped pipeline. The search for the setup files
and the loop now run in try/finally and the finally completes the bar. The old
completion used -Complete, which worked only as an abbreviation of -Completed.

(do Install-DbaInstance)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of each computer was completed at the end of its loop iteration. Every
Stop-Function -Continue in the loop, such as a computer that cannot be reached,
no SQL Server found or a pending reboot, skipped the completion and left the
bar on screen, as did a throw or a stopped pipeline. The work on each computer
now runs in try/finally and the finally completes the bar.

(do Update-DbaInstance)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed after the output object only. The returns when the
configuration file cannot be copied or the installation fails, a throw, a
stopped pipeline and Select-Object -First skipped it. The body after the
activity text now runs in try/finally and the finally completes the bar.

(do Invoke-DbaAdvancedInstall)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed after the output of each action only. The returns when
the extraction or the update fails, a throw, a stopped pipeline and
Select-Object -First skipped it. The restart check and the loop over the
actions now run in try/finally and the finally completes the bar.

(do Invoke-DbaAdvancedUpdate)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed by hand in front of each Stop-Function and at the end,
which left it on screen when the pipeline was stopped during the download or
the extraction, or when anything outside Stop-Function threw. The work from the
first progress step on now runs in try/finally and the finally completes the
bar; the ten hand-written completions go away.

(do Install-DbaSqlPackage)

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed at the end of each instance only. The returns after the
service was stopped, the -Continue when the restart fails, a throw, a stopped
pipeline and Select-Object -First at the returned login skipped it. The steps
of each instance now run in try/finally and the finally completes the bar.

(do Reset-DbaAdmin)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of each instance was completed at the end of its loop iteration. The
continue after "No changes needed" and every Stop-Function -Continue, such as
no suitable certificate, skipped it and left the bar on screen even without
EnableException. The work on each instance now runs in try/finally and the
finally completes the bar.

(do Set-DbaNetworkCertificate)

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

The bar of each computer was completed at the end of its loop iteration. Every
Stop-Function -Continue, such as a certificate the CA does not issue, a throw,
a stopped pipeline and Select-Object -First at the returned certificate skipped
it. The work on each computer now runs in try/finally and the finally completes
the bar.

(do New-DbaComputerCertificate)

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

The bar was completed at the end of the process block only. A throw under
EnableException, a stopped pipeline and Select-Object -First at the result
skipped it. The loop over the instances now runs in try/finally and the finally
completes the bar.

(do Install-DbaMaintenanceSolution)

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

The test ran the command against the local computer and expected the warning
about the missing setup file. The CI runner had a reboot pending, so the
command stopped one check earlier with another warning and the test failed.
The test now uses a computer under the top level domain invalid, which never
resolves, so the check for a pending reboot fails on every machine. That is a
Stop-Function -Continue inside the same loop, so the old command still leaves
its bar on screen and the test stays red on it.

(do Install-DbaInstance)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@andreasjordan
andreasjordan marked this pull request as ready for review October 5, 2026 17:21
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