From 76ce9680dc9ef06862531eaf5e1b0704556d4d9e Mon Sep 17 00:00:00 2001 From: Chris Titus Date: Wed, 1 Jul 2026 22:20:52 -0500 Subject: [PATCH] Clean up runspace invocation ownership --- functions/public/Invoke-WPFRunspace.ps1 | 63 ++++++++++++++++++------ pester/runspace.Tests.ps1 | 64 +++++++++++++++++++------ pester/sanity.Tests.ps1 | 24 ++-------- speed-todo.md | 10 ++-- 4 files changed, 107 insertions(+), 54 deletions(-) diff --git a/functions/public/Invoke-WPFRunspace.ps1 b/functions/public/Invoke-WPFRunspace.ps1 index 03e05038..cf9280d6 100644 --- a/functions/public/Invoke-WPFRunspace.ps1 +++ b/functions/public/Invoke-WPFRunspace.ps1 @@ -30,30 +30,65 @@ function Invoke-WPFRunspace { $ParameterList ) + if (-not ("WinUtilRunspaceCleanup" -as [type])) { + Add-Type @" +using System; +using System.Management.Automation; + +public sealed class WinUtilRunspaceCleanupState +{ + public PowerShell PowerShell { get; set; } + public IAsyncResult Handle { get; set; } +} + +public static class WinUtilRunspaceCleanup +{ + public static void Cleanup(object state, bool timedOut) + { + var cleanupState = state as WinUtilRunspaceCleanupState; + if (cleanupState == null || cleanupState.PowerShell == null || cleanupState.Handle == null) + { + return; + } + + try + { + cleanupState.PowerShell.EndInvoke(cleanupState.Handle); + } + catch + { + } + finally + { + cleanupState.PowerShell.Dispose(); + } + } +} +"@ + } + # Create a PowerShell instance - $script:powershell = [powershell]::Create() + $powershell = [powershell]::Create() # Add Scriptblock and Arguments to runspace - [void]$script:powershell.AddScript($ScriptBlock) - [void]$script:powershell.AddArgument($ArgumentList) + [void]$powershell.AddScript($ScriptBlock) + [void]$powershell.AddArgument($ArgumentList) foreach ($parameter in $ParameterList) { - [void]$script:powershell.AddParameter($parameter[0], $parameter[1]) + [void]$powershell.AddParameter($parameter[0], $parameter[1]) } - $script:powershell.RunspacePool = $sync.runspace + $powershell.RunspacePool = $sync.runspace # Execute the RunspacePool - $script:handle = $script:powershell.BeginInvoke() + $handle = $powershell.BeginInvoke() + + $cleanupState = [WinUtilRunspaceCleanupState]::new() + $cleanupState.PowerShell = $powershell + $cleanupState.Handle = $handle + $cleanupCallback = [System.Threading.WaitOrTimerCallback][WinUtilRunspaceCleanup]::Cleanup + [System.Threading.ThreadPool]::RegisterWaitForSingleObject($handle.AsyncWaitHandle, $cleanupCallback, $cleanupState, -1, $true) | Out-Null - # Clean up the RunspacePool threads when they are complete, and invoke the garbage collector to clean up the memory - if ($script:handle.IsCompleted) { - $script:powershell.EndInvoke($script:handle) - $script:powershell.Dispose() - $sync.runspace.Dispose() - $sync.runspace.Close() - [System.GC]::Collect() - } # Return the handle return $handle } diff --git a/pester/runspace.Tests.ps1 b/pester/runspace.Tests.ps1 index 9cb42e85..7acfe066 100644 --- a/pester/runspace.Tests.ps1 +++ b/pester/runspace.Tests.ps1 @@ -21,18 +21,12 @@ BeforeAll { } function script:Clear-WinUtilRunspaceTestContext { - if ($script:powershell) { - $script:powershell.Dispose() - } - if ($script:sync -and $script:sync.runspace) { $script:sync.runspace.Close() $script:sync.runspace.Dispose() } Remove-Variable -Name sync -Scope Script -ErrorAction SilentlyContinue - Remove-Variable -Name powershell -Scope Script -ErrorAction SilentlyContinue - Remove-Variable -Name handle -Scope Script -ErrorAction SilentlyContinue } function script:Assert-WinUtilAsyncHandle { @@ -54,28 +48,34 @@ Describe "Invoke-WPFRunspace behavior" { } It "returns a single async handle with no argument list" { + $script:sync.Result = $null + $handle = Invoke-WPFRunspace -ScriptBlock { Start-Sleep -Milliseconds 100 - "no-args|$($sync.Marker)" + $sync.Result = "no-args|$($sync.Marker)" } Assert-WinUtilAsyncHandle -Handle $handle - @($script:powershell.EndInvoke($handle))[0] | Should -Be "no-args|shared" + $script:sync.Result | Should -Be "no-args|shared" } It "passes one named parameter" { + $script:sync.Result = $null + $handle = Invoke-WPFRunspace -ParameterList @(,("Name", "value")) -ScriptBlock { param([string]$Name) Start-Sleep -Milliseconds 100 - "Name=$Name" + $sync.Result = "Name=$Name" } Assert-WinUtilAsyncHandle -Handle $handle - @($script:powershell.EndInvoke($handle))[0] | Should -Be "Name=value" + $script:sync.Result | Should -Be "Name=value" } It "passes multiple named parameters" { + $script:sync.Result = $null + $handle = Invoke-WPFRunspace -ParameterList @( ("First", "alpha"), ("Second", "beta") @@ -86,21 +86,57 @@ Describe "Invoke-WPFRunspace behavior" { ) Start-Sleep -Milliseconds 100 - "$First|$Second|$($sync.Marker)" + $sync.Result = "$First|$Second|$($sync.Marker)" } Assert-WinUtilAsyncHandle -Handle $handle - @($script:powershell.EndInvoke($handle))[0] | Should -Be "alpha|beta|shared" + $script:sync.Result | Should -Be "alpha|beta|shared" } - It "surfaces scriptblock failures through the owning PowerShell instance" { + It "keeps the shared runspace pool usable after scriptblock failures" { $handle = Invoke-WPFRunspace -ScriptBlock { Start-Sleep -Milliseconds 100 throw "runspace failure" } Assert-WinUtilAsyncHandle -Handle $handle - { $script:powershell.EndInvoke($handle) } | Should -Throw -ExpectedMessage "*runspace failure*" + + $script:sync.Result = $null + $secondHandle = Invoke-WPFRunspace -ScriptBlock { + $sync.Result = "after-failure" + } + + Assert-WinUtilAsyncHandle -Handle $secondHandle + $script:sync.Result | Should -Be "after-failure" + } + + It "runs multiple queued invocations without shared PowerShell state" { + $script:sync.FirstResult = $null + $script:sync.SecondResult = $null + + $firstHandle = Invoke-WPFRunspace -ParameterList @(,("Value", "first")) -ScriptBlock { + param([string]$Value) + + Start-Sleep -Milliseconds 150 + $sync.FirstResult = $Value + } + $secondHandle = Invoke-WPFRunspace -ParameterList @(,("Value", "second")) -ScriptBlock { + param([string]$Value) + + $sync.SecondResult = $Value + } + + Assert-WinUtilAsyncHandle -Handle $firstHandle + Assert-WinUtilAsyncHandle -Handle $secondHandle + $script:sync.FirstResult | Should -Be "first" + $script:sync.SecondResult | Should -Be "second" + } + + It "does not use script-scoped PowerShell or handle state" { + $runspaceScript = Get-Content -Path (Join-Path $script:repoRoot "functions\public\Invoke-WPFRunspace.ps1") -Raw + + $runspaceScript | Should -Not -Match '\$script:powershell' + $runspaceScript | Should -Not -Match '\$script:handle' } } diff --git a/pester/sanity.Tests.ps1 b/pester/sanity.Tests.ps1 index 91c62aa8..2799a2cb 100644 --- a/pester/sanity.Tests.ps1 +++ b/pester/sanity.Tests.ps1 @@ -216,44 +216,26 @@ Describe "Runspace sanity" { $script:sync.runspace = [runspacefactory]::CreateRunspacePool(1, 2, $initialSessionState, $Host) $script:sync.runspace.Open() - $ended = $false try { + $script:sync.Result = $null $handle = Invoke-WPFRunspace -ArgumentList "argument" -ParameterList @(,("NamedValue", "parameter")) -ScriptBlock { param($ArgumentValue, [string]$NamedValue) Start-Sleep -Milliseconds 200 - "$ArgumentValue|$NamedValue|$($sync.SmokeValue)" + $sync.Result = "$ArgumentValue|$NamedValue|$($sync.SmokeValue)" } ($handle -is [System.IAsyncResult]) | Should -BeTrue ($handle -is [array]) | Should -BeFalse $handle.AsyncWaitHandle.WaitOne(5000) | Should -BeTrue - - $result = $script:powershell.EndInvoke($handle) - $ended = $true - - @($result)[0] | Should -Be "argument|parameter|shared" + $script:sync.Result | Should -Be "argument|parameter|shared" } finally { - if (-not $ended -and $handle -and $handle.IsCompleted -and $script:powershell) { - try { - $script:powershell.EndInvoke($handle) | Out-Null - } catch { - # The assertion failure is more useful than cleanup errors here. - } - } - - if ($script:powershell) { - $script:powershell.Dispose() - } - if ($script:sync -and $script:sync.runspace) { $script:sync.runspace.Close() $script:sync.runspace.Dispose() } Remove-Variable -Name sync -Scope Script -ErrorAction SilentlyContinue - Remove-Variable -Name powershell -Scope Script -ErrorAction SilentlyContinue - Remove-Variable -Name handle -Scope Script -ErrorAction SilentlyContinue } } } diff --git a/speed-todo.md b/speed-todo.md index c4a91460..946423ae 100644 --- a/speed-todo.md +++ b/speed-todo.md @@ -36,11 +36,11 @@ WinUtil currently does too much work before the first GUI paint. Work from the t ## P4 - Runspace Cleanup -- [ ] Refactor `Invoke-WPFRunspace` to avoid shared `$script:powershell` and `$script:handle` state for concurrent callers. -- [ ] Keep the shared runspace pool alive for the app lifetime instead of disposing it from individual queued calls. -- [ ] Add explicit completion cleanup for each PowerShell instance after `EndInvoke`. -- [ ] Add focused Pester coverage for multiple queued runspace calls, failures, and cleanup. -- [ ] Do not parallelize winget/choco package installs by default; package manager locking and prompts make that unsafe. +- [x] Refactor `Invoke-WPFRunspace` to avoid shared `$script:powershell` and `$script:handle` state for concurrent callers. +- [x] Keep the shared runspace pool alive for the app lifetime instead of disposing it from individual queued calls. +- [x] Add explicit completion cleanup for each PowerShell instance after `EndInvoke`. +- [x] Add focused Pester coverage for multiple queued runspace calls, failures, and cleanup. +- [x] Do not parallelize winget/choco package installs by default; package manager locking and prompts make that unsafe. ## P5 - Defer Runspace Pool Startup