Clean up runspace invocation ownership
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
+50
-14
@@ -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'
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+3
-21
@@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+5
-5
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user