Skip to content

Progress - Complete the bars of 9 more commands in finally blocks (tier 2) - #10774

Open
andreasjordan wants to merge 9 commits into
developmentfrom
progress-leaks-tier2
Open

andreasjordan wants to merge 9 commits into
developmentfrom
progress-leaks-tier2

Conversation

@andreasjordan

Copy link
Copy Markdown
Collaborator

Part of RFC #10548: the try/finally leak fixes, tier 2 of 3. Same pattern as #10773 (tier 1), one commit per command with its test. Independent of #10773.

Problem

These 9 commands complete their progress bar only on the success path, so a throw under -EnableException, a stopped pipeline (Ctrl+C) or Select-Object -First left the bar on screen for the rest of the session. Two of them were worse:

  • Expand-DbaDbLogFile never completed its bars at all, not even after a run that worked: the bar of the databases (Id 1) and the bar of the growth (Id 2) stayed after every run. Its only completion completed the bar of the databases when a log backup failed, which removed the parent while the child stayed.
  • Invoke-DbaDiagnosticQuery drew its bar in process and completed it in end, which neither Select-Object -First nor a stopped pipeline reaches.

Change

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

  • Invoke-DbaAdvancedRestore has a parent bar per database (Id 1) and a child bar per backup file (Id 2). The child is now completed first thing in the existing finally of each file's restore, before the output there (a downstream Select-Object -First stops at that output). The loop over the files runs in try/finally, whose finally completes the parent. The success-path completion of the child stays, because a stop-at-mark restore still runs the recovery statement after it.
  • Expand-DbaDbLogFile: the growth loop, the log backup and the loop over the databases each run in try/finally, each finally completes its own bar, children inside their parent. The completion of the parent on a failed backup goes away. The completion of the backup bar after the backup stays, because the shrink follows it.
  • Invoke-DbaDiagnosticQuery: the loop over the instances runs in try/finally. Its counter restarts per instance, so the bar's lifetime is that loop. The end block held only the completion and goes away.
  • Read-DbaBackupHeader, Start-DbaMigration, Sync-DbaAvailabilityGroup, Export-DbaUser, Invoke-DbaDbDataGenerator, Invoke-DbaDbPiiScan: one try/finally around the loop or the steps that draw the bar.

Most of the diff is re-indentation: git diff -w shows 38 lines added and 5 removed for the 9 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
Invoke-DbaAdvancedRestore full and log backup, Select-Object -First 1 stops after the full backup (asserted)
Invoke-DbaDiagnosticQuery one query, Select-Object -First 1 at its result
Start-DbaMigration -WhatIf, pipeline stopped at the first migration step
Sync-DbaAvailabilityGroup the existing job sync between two instances with -WhatIf, pipeline stopped at the first step
Export-DbaUser -Passthru, Select-Object -First 1 at the script
Invoke-DbaDbDataGenerator Select-Object -First 1 at the result of the table
Invoke-DbaDbPiiScan pipeline stopped at the first scan record; a second, unskipped Describe with its own small database, because the existing integration Describe is skipped as a whole
Expand-DbaDbLogFile a plain run that grows the log once more; both bars must be completed
Read-DbaBackupHeader no automated test, see below

Read-DbaBackupHeader 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, so its bar cannot be seen this way (same as the -Parallel bar of Start-DbaDbEncryption in #10773). I checked it in a real console window instead, reading the ConsoleHost's pending bars by reflection after Read-DbaBackupHeader ... | Select-Object -First 1: the old command leaves "Updating" (Id 1) behind on 5.1 and 7.6, the new one does not. Its existing tests pass.

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

  • New command: all 9 test files pass on PowerShell 5.1 and 7.6, without warnings.
  • Old command: in every file with a new test exactly that test fails, on both editions. Expand-DbaDbLogFile leaves both Ids open.
  • TestTestfiles.Tests.ps1 and TestTestLayout.ps1 report nothing for the 8 changed test files.

Left open

  • Sync-DbaAvailabilityGroup closes the dedicated admin connection it opens for the passwords only on the success path, so a throw or a stop between opening and closing keeps the single DAC of the instance busy. Not a progress defect, not changed here.
  • Read-DbaBackupHeader does not close its runspace pool or restore [Runspace]::DefaultRunspace when it is stopped. Same.

馃 Generated with Claude Code

andreasjordan and others added 9 commits October 5, 2026 16:26
The bar of a database (Id 1) and the bar of its backup files (Id 2) were
completed after the loop over the backup files. A throw, a stopped pipeline
and Select-Object -First at the result of a file skipped both. The child bar
is now completed first thing in the finally of each file's restore, before the
output there, and the loop over the files runs in try/finally, whose finally
completes the bar of the database.

(do Invoke-DbaAdvancedRestore)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed after the loop that collects the results of the threads.
A throw, a stopped pipeline and Select-Object -First at the first header row
skipped it. The loop now runs in try/finally and the finally completes the bar.

(do Read-DbaBackupHeader)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was drawn in the process block and completed in the end block, which
neither Select-Object -First nor a stopped pipeline reaches. The loop over the
instances now runs in try/finally and the finally completes the bar; the end
block held only that completion and goes away.

(do Invoke-DbaDiagnosticQuery)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar was completed after the last migration step only. A throw in one of
the steps or a stopped pipeline left it on screen. The steps now run in
try/finally and the finally completes the bar.

(do Start-DbaMigration)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of each availability group was completed after the last sync step only.
A throw in one of the steps or a stopped pipeline left it on screen. The steps
now run in try/finally and the finally completes the bar.

(do Sync-DbaAvailabilityGroup)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of each database was completed after the loop over its users. A throw,
a stopped pipeline and Select-Object -First at the first script or file skipped
it. The loop now runs in try/finally and the finally completes the bar.

(do Export-DbaUser)

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

(do Invoke-DbaDbDataGenerator)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of each instance was completed after the loop over the databases. A
throw or a stopped pipeline, likely during a long scan, left it on screen. The
loop now runs in try/finally and the finally completes the bar.

(do Invoke-DbaDbPiiScan)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bar of the databases (Id 1) and the bar of the growth (Id 2) were never
completed, not even after a run that worked; the only completion completed the
bar of the databases when a log backup failed. The growth loop, the log backup
and the loop over the databases now each run in try/finally, and each finally
completes its own bar. The completion of the parent on a failed backup goes
away. The completion after the backup stays, because the shrink follows it.

(do Expand-DbaDbLogFile)

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