Skip to content

Side findings of the progress leak fixes: runspace pool on $Host, cleanup, output and -WhatIf defects #10776

Description

@andreasjordan

While fixing the progress bar leaks of RFC #10548 (#10773, #10774, #10775) I came across defects that have nothing to do with progress. I left them out of those PRs to keep them focused, and collect them here so they do not get lost. Line numbers are as of development e273a41. "Seen in the lab" means I ran into it; the others come from reading the code.

1. A runspace pool on $Host hides warnings, verbose messages and progress from API hosts

Read-DbaBackupHeader (and with it Restore-DbaDatabase and Get-DbaBackupInformation) and Start-/Stop-DbaDbEncryption -Parallel create their pool with [runspacefactory]::CreateRunspacePool(1, 10, $initialSessionState, $Host):

  • public/Read-DbaBackupHeader.ps1:293
  • public/Start-DbaDbEncryption.ps1:563
  • public/Stop-DbaDbEncryption.ps1:198

Once a thread of that pool has run, the calling pipeline's warning, verbose and progress records no longer reach the Streams of a host created through the PowerShell API ([powershell]::Create(): automation hosts, SDK hosts, notebooks). Output and errors still arrive. In a console the records go to the console UI, so nobody sees the problem there. Seen in the lab, identical on 5.1 and 7.6:

$runspace = [runspacefactory]::CreateRunspace()
$runspace.Open()
$shell = [powershell]::Create()
$shell.Runspace = $runspace
$null = $shell.AddScript({
    $VerbosePreference = "Continue"
    Write-Warning "warning before"
    Write-Verbose "verbose before"
    $pool = [runspacefactory]::CreateRunspacePool(1, 2, [System.Management.Automation.Runspaces.InitialSessionState]::CreateDefault(), $Host)
    $pool.Open()
    $thread = [powershell]::Create()
    $thread.RunspacePool = $pool
    $null = $thread.AddScript({ 1 })
    $null = $thread.EndInvoke($thread.BeginInvoke())
    Write-Warning "warning after"
    Write-Verbose "verbose after"
    Write-Error "error after"
    "output after"
})
$out = $shell.Invoke()
"Output: $out"                                      # output after
"Warning: " + ($shell.Streams.Warning -join " | ")  # warning before
"Verbose: " + ($shell.Streams.Verbose -join " | ")  # verbose before
"Error: " + ($shell.Streams.Error -join " | ")      # error after

For dbatools that means: a script that runs Restore-DbaDatabase through the API loses every warning after the backup headers were read, including the warnings Stop-Function writes when something fails. Candidate fix: create the pool without $Host.

  • Runspace pool on $Host in Read-DbaBackupHeader, Start-DbaDbEncryption, Stop-DbaDbEncryption

2. Cleanup that only runs on the success path

A throw, a stopped pipeline (Ctrl+C) or Select-Object -First skips these:

  • Sync-DbaAvailabilityGroup: the dedicated admin connection opened at line 333 is closed only at line 365. If one of the three copy commands in between throws, the instance's only DAC stays occupied until the process ends.
  • Invoke-DbaDbDataGenerator: $sqlconn (opened at line 226) is closed at line 598 only; the return at lines 323, 386 and 461 and any throw leave it open.
  • Read-DbaBackupHeader: closing the pool and restoring [Runspace]::DefaultRunspace (lines 379-380) are not in a finally.
  • Start-/Stop-DbaDbEncryption -Parallel: the pool is closed after the result loop only (Start lines 621-622, Stop lines 255-256).
  • Invoke-DbaDbDataMasking: a stopped run leaves tempdb.dbo.DeterministicValues and the identity column and index it added to the table (seen in the lab).
  • New-DbaComputerCertificate: a stopped run leaves the request folder in the temp folder.

3. Output and flow control

  • Invoke-DbaDbDataMasking
    • Line 285: $server.Databases['tempdb'].Query($query) without $null =. When an earlier run left DeterministicValues behind, the next run emits a $null before its results (seen in the lab: the existing test "returns the proper output" failed on it).
    • Lines 1191 and 1265: $stringBuilder.ToString() writes the UPDATE statements to the pipeline: line 1191 when the update fails, line 1265 on every run with composites.
    • Line 821: Stop-Function -Message "You cannot use both composites and actions" has no return or -Continue, so without EnableException the table is masked anyway.
    • Lines 1192 and 1268: the Stop-Function -Continue continues the table loop and skips the removal of the identity index and column (lines 1300-1326) for that table, although the comment at line 1315 says it "always runs".
  • Invoke-DbaAdvancedRestore
    • Lines 618-619: Stop-Function -Message "Failed to restore db $database, stopping" -Continue is followed by break. Without EnableException the continue wins, the break is unreachable, and the restore goes on with the next backup file of the database whose restore just failed.
    • Line 547: return "Verify successful" leaves the whole end block, so -VerifyOnly verifies only the first backup file.
  • Invoke-DbaAdvancedInstall
    • Line 285: the return after "Failed to copy file ..." returns no $output object, unlike the other failure paths (lines 322, 346), so Install-DbaInstance gets no result for that computer.
    • Line 234: the message uses $computer, which this function never sets; line 367 uses $ComputerName.
  • Invoke-DbaDbLogShipRecovery, line 131: -Target $server.name before $server is set at line 140.
  • Install-DbaInstance, lines 568-580: when Resolve-DbaNetworkName fails, $fullComputerName stays empty, so Test-PendingReboot fails on the empty name and the warning reads "Failed to get reboot status from " without a computer name. Update-DbaInstance falls back to the name it was given in that case (line 456).

4. -WhatIf does real work

  • New-DbaComputerCertificate: the request folder and request.inf are created outside ShouldProcess (lines 403 and 414), and their removal at line 630 is inside it, so -WhatIf leaves the folder behind.
  • Install-DbaSqlPackage: the download (lines 218-237) runs before the ShouldProcess at line 242, so -WhatIf downloads SqlPackage.

5. Error reporting

  • Save-DbaKbUpdate: a failed download (lines 156 and 161) is not handled by Stop-Function. Invoke-WebRequest reports it as an error that ends only its own statement, so the command neither warns nor throws under -EnableException; the caller gets a plain error record and no output (seen in the lab with a link that does not resolve).

6. Tests


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