diff --git a/Modules/ZipParallelProcessing.ps1 b/Modules/ZipParallelProcessing.ps1 index 20d35fb..7f3e6c6 100644 --- a/Modules/ZipParallelProcessing.ps1 +++ b/Modules/ZipParallelProcessing.ps1 @@ -1,5 +1,63 @@ # See module-descriptions.md for this module's full description. +function Test-ZipJobValueIsMaterializedCollection { + param($Value) + if ($Value -is [System.Collections.ICollection]) { return $true } + foreach ($i in $Value.GetType().GetInterfaces()) { + if ($i.IsGenericType) { + $def = $i.GetGenericTypeDefinition() + if ($def -eq [System.Collections.Generic.ICollection`1] -or $def -eq [System.Collections.Generic.IReadOnlyCollection`1]) { return $true } + } + } + return $false +} + +function Test-ZipJobValueHasScriptBlock { + param($Value, [System.Collections.Generic.HashSet[object]]$Seen, [int]$Depth) + if ($null -eq $Value) { return $false } + if ($Value -is [scriptblock]) { return $true } + if ($Depth -ge 4 -or $Value -is [string]) { return $false } + $type = $Value.GetType() + $itemsAreProperties = $false + if ($Value -is [System.Collections.DictionaryEntry] -or + ($type.IsGenericType -and $type.GetGenericTypeDefinition() -eq [System.Collections.Generic.KeyValuePair`2])) { + $items = @($Value.Key, $Value.Value) + } elseif ($Value -is [System.Management.Automation.PSCustomObject]) { + if (-not $Seen.Add($Value)) { return $false } + $items = $Value.PSObject.Properties + $itemsAreProperties = $true + } elseif ($Value -is [System.Collections.IEnumerable] -and (Test-ZipJobValueIsMaterializedCollection -Value $Value)) { + if (-not $type.IsValueType -and -not $Seen.Add($Value)) { return $false } + if ($Value -is [System.Collections.IDictionary]) { $items = $Value.Values } else { $items = $Value } + } else { + return $false + } + try { + foreach ($item in $items) { + if ($itemsAreProperties) { $item = $item.Value } + if (Test-ZipJobValueHasScriptBlock -Value $item -Seen $Seen -Depth ($Depth + 1)) { return $true } + } + } catch [System.InvalidOperationException] { + # "Collection was modified": something else is writing to it right + # now, so it can't be read safely. Let it through unchecked rather + # than fail the run over it. + } + return $false +} + +function Get-ZipJobScriptBlockKey { + param([hashtable]$Job, [System.Collections.Generic.HashSet[object]]$Seen) + foreach ($key in $Job.Keys) { + try { + $found = Test-ZipJobValueHasScriptBlock -Value $Job[$key] -Seen $Seen -Depth 0 + } catch { + throw ("Invoke-ZipParallelProcessing: couldn't check job parameter '" + $key + "' for scriptblocks - reading it failed: " + $_) + } + if ($found) { return [string]$key } + } + return $null +} + function Invoke-ZipParallelProcessing { param( [Parameter(Mandatory)] @@ -27,6 +85,16 @@ function Invoke-ZipParallelProcessing { if ($null -ne $JobParams -and $JobParams.Count -eq 0) { return @{ ForceStoppedCount = 0 } } + $scriptBlockJobError = "Invoke-ZipParallelProcessing: job parameter '{0}' is (or contains) a scriptblock. A scriptblock belongs to the runspace that created it, and run inside a worker it can deadlock or crash the process. Pass plain data instead, or pass the code as a string and build it inside the worker with [scriptblock]::Create()." + $checkedJobValues = [System.Collections.Generic.HashSet[object]]::new() + $refusedJobError = $null + if ($null -ne $JobParams) { + foreach ($job in $JobParams) { + $badKey = Get-ZipJobScriptBlockKey -Job $job -Seen $checkedJobValues + if ($null -ne $badKey) { throw ($scriptBlockJobError -f $badKey) } + } + } + $fixedListIndex = 0 $previousLogMutex = $null @@ -68,6 +136,10 @@ function Invoke-ZipParallelProcessing { } } else { $nextJob = & $NextJobProvider + if ($null -ne $nextJob) { + $badKey = Get-ZipJobScriptBlockKey -Job $nextJob -Seen $checkedJobValues + if ($null -ne $badKey) { $refusedJobError = $scriptBlockJobError -f $badKey; $nextJob = $null } + } } if ($null -eq $nextJob) { $scanExhausted = $true; break } @@ -131,6 +203,7 @@ function Invoke-ZipParallelProcessing { Flush-ConsoleQueue -Queue $localConsoleQueue if ($null -ne $PollCallback) { $null = & $PollCallback } + if ($null -ne $refusedJobError) { throw $refusedJobError } } finally { # A worker stuck inside a .NET call can't be stopped, and Stop()/ # Dispose()/Close() would all sit and wait for it, so an abandoned worker may keep running. diff --git a/_Harness/Lint.Harness.ps1 b/_Harness/Lint.Harness.ps1 index 865cb14..d5b795a 100644 --- a/_Harness/Lint.Harness.ps1 +++ b/_Harness/Lint.Harness.ps1 @@ -63,6 +63,15 @@ $script:LintScriptVarAllowed = @( Why = 'Only read after Test-AbortSignal''s own worker early-return (returns [ConsoleCtrl]::AbortRequested, never announces), i.e. main thread only.' } ) +# Rule 9 only: specific (function, host cmdlet) pairs reviewed as safe on a +# runspace-reachable path. Each entry must say why the cmdlet can't +# actually run inside a worker. +$script:LintHostCmdletAllowed = @( + @{ Function = 'Write-LogLineToHost' + Cmdlets = @('Write-Host') + Why = 'Only called from Write-Log past its worker early-return (worker lines go through the console queue) and from Flush-ConsoleQueue (main thread). Listed in Get-LoggingSharedFunctionNames only because Rule 5 walks Write-Log''s callees statically.' } +) + $script:passCount = 0 $script:failCount = 0 @@ -74,6 +83,14 @@ function Test-IsScriptVarAllowed { return $false } +function Test-IsHostCmdletAllowed { + param([string]$Function, [string]$Cmdlet) + foreach ($a in $script:LintHostCmdletAllowed) { + if ($a.Function -eq $Function -and ($a.Cmdlets -contains $Cmdlet)) { return $true } + } + return $false +} + function Test-Assert { param([bool]$Condition, [string]$Message) if ($Condition) { @@ -278,6 +295,7 @@ Write-Host "" Write-Host "=== Lint: -SharedFunctionNames missing a transitively-called function ===" -ForegroundColor Cyan $fnAstByName = @{} +$fnFileByName = @{} $fileAsts = @{} $allFnNames = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) foreach ($name in $fileText.Keys) { @@ -285,6 +303,7 @@ foreach ($name in $fileText.Keys) { $fileAsts[$name] = $ast foreach ($f in $ast.FindAll({ param($n) $n -is [System.Management.Automation.Language.FunctionDefinitionAst] }, $true)) { $fnAstByName[$f.Name] = $f + $fnFileByName[$f.Name] = $name $null = $allFnNames.Add($f.Name) } } @@ -342,23 +361,30 @@ function Get-MissingSharedFunctionNames { return ,@($missing | Select-Object -Unique) } +# The scriptblock literal an assignment's right-hand side holds - a bare +# { ... }, or { ... }.GetNewClosure(), which parses as an +# InvokeMemberExpressionAst wrapping it - or $null. The one place this is +# decided; Rule 5's worker-body lookup below and Rule 7 both call it. +function Get-AssignedScriptBlockLiteral { + param($Right) + if ($Right -isnot [System.Management.Automation.Language.CommandExpressionAst]) { return $null } + $expr = $Right.Expression + if ($expr -is [System.Management.Automation.Language.InvokeMemberExpressionAst] -and $expr.Member.Extent.Text -eq 'GetNewClosure') { $expr = $expr.Expression } + if ($expr -is [System.Management.Automation.Language.ScriptBlockExpressionAst]) { return $expr } + return $null +} + # Every $script:XXX = { ... } scriptblock assignment, so a -WorkerScript/ # -NextJobProvider $script:XXX argument can be resolved to its real body. -# Also unwraps $script:XXX = { ... }.GetNewClosure() - that shape parses -# as an InvokeMemberExpressionAst wrapping the ScriptBlockExpressionAst, -# not a bare ScriptBlockExpressionAst, and this codebase's own -# ResultHandler callbacks already use .GetNewClosure() elsewhere. $scriptBlockVars = @{} +$scriptBlockVarFile = @{} foreach ($name in $fileText.Keys) { foreach ($assign in $fileAsts[$name].FindAll({ param($n) $n -is [System.Management.Automation.Language.AssignmentStatementAst] }, $true)) { - if ($assign.Right -is [System.Management.Automation.Language.CommandExpressionAst] -and - $assign.Left -is [System.Management.Automation.Language.VariableExpressionAst]) { - $rhsExpr = $assign.Right.Expression - if ($rhsExpr -is [System.Management.Automation.Language.InvokeMemberExpressionAst]) { - $rhsExpr = $rhsExpr.Expression - } - if ($rhsExpr -is [System.Management.Automation.Language.ScriptBlockExpressionAst]) { - $scriptBlockVars[$assign.Left.VariablePath.UserPath] = $rhsExpr.ScriptBlock + if ($assign.Left -is [System.Management.Automation.Language.VariableExpressionAst]) { + $literal = Get-AssignedScriptBlockLiteral -Right $assign.Right + if ($null -ne $literal) { + $scriptBlockVars[$assign.Left.VariablePath.UserPath] = $literal.ScriptBlock + $scriptBlockVarFile[$assign.Left.VariablePath.UserPath] = $name } } } @@ -449,6 +475,26 @@ Test-Assert (@($rule5).Count -eq 0) "every -SharedFunctionNames array covers eve Show-Findings -Findings @($rule5) Write-Host "" +# Runs $Finder (param($AstNode, $FunctionName), returning @{ Line; What } +# items) over every runspace-reachable function and worker body, and shapes +# the hits for Show-Findings. $FunctionName is $null for a worker body. +function Get-RunspaceReachableFindings { + param([scriptblock]$Finder) + $out = [System.Collections.Generic.List[object]]::new() + foreach ($fname in $runspaceReachable) { + if (-not $fnAstByName.ContainsKey($fname)) { continue } + foreach ($hit in (& $Finder $fnAstByName[$fname] $fname)) { + $out.Add(@{ File = $fnFileByName[$fname]; Line = $hit.Line; Text = ($fname + " (runspace-reachable) " + $hit.What) }) + } + } + foreach ($workerVar in $runspaceWorkerBodies) { + foreach ($hit in (& $Finder $scriptBlockVars[$workerVar] $null)) { + $out.Add(@{ File = $scriptBlockVarFile[$workerVar]; Line = $hit.Line; Text = ("worker `$" + $workerVar + " " + $hit.What) }) + } + } + return ,$out +} + # ------------------------------------------------------------------------ # Rule 6: $script: variable used on a runspace-reachable path # ------------------------------------------------------------------------ @@ -478,8 +524,7 @@ $scriptVarRefFilter = { foreach ($fname in $runspaceReachable) { if (-not $fnAstByName.ContainsKey($fname)) { continue } $fnAst = $fnAstByName[$fname] - $fnFile = $null - foreach ($name in $fileAsts.Keys) { if ($fileText[$name].Contains($fnAst.Extent.Text)) { $fnFile = $name; break } } + $fnFile = $fnFileByName[$fname] foreach ($ref in $fnAst.FindAll($scriptVarRefFilter, $true)) { $varName = $ref.VariablePath.UserPath -replace '^script:', '' if (Test-IsScriptVarAllowed -Function $fname -Variable $varName) { continue } @@ -488,8 +533,7 @@ foreach ($fname in $runspaceReachable) { } foreach ($workerVar in $runspaceWorkerBodies) { $body = $scriptBlockVars[$workerVar] - $workerFile = $null - foreach ($name in $fileAsts.Keys) { if ($fileText[$name].Contains($body.Extent.Text)) { $workerFile = $name; break } } + $workerFile = $scriptBlockVarFile[$workerVar] foreach ($nested in $body.FindAll({ param($n) $n -is [System.Management.Automation.Language.ScriptBlockExpressionAst] }, $true)) { foreach ($ref in $nested.FindAll($scriptVarRefFilter, $true)) { $varName = $ref.VariablePath.UserPath -replace '^script:', '' @@ -502,118 +546,56 @@ Show-Findings -Findings @($rule6) Write-Host "" # ------------------------------------------------------------------------ -# Rule 7: a scriptblock crossing into a worker runspace +# Rule 7: a scriptblock handed to .NET or stored into shared state on a runspace-reachable path # ------------------------------------------------------------------------ # A scriptblock belongs to the runspace that created it. Run on another -# runspace's thread - as a delegate (Rule 8), or because it was handed -# over as a job parameter or through shared state and called with & - -# every scriptblock PowerShell runs inside it (a Where-Object filter, a -# nested delegate) gets queued back to the owning runspace, and the -# calling thread waits. If the owner is itself waiting on that thread (a -# lock, a Lazy), both hang for good; if the owner is idle, the queued -# block can run against the owner's scope stack and crash the process. -# Workers get plain data only. +# runspace's thread, every scriptblock PowerShell runs inside it (a +# Where-Object filter, a nested delegate) gets queued back to the owning +# runspace, and the calling thread waits. If the owner is itself waiting on +# that thread (a lock, a Lazy), both hang for good; if the owner is idle, +# the queued block can run against the owner's scope stack and crash the +# process. # -# Two shapes, both flagged: -# - a job-parameter hashtable handed to Invoke-ZipParallelProcessing (a -# -JobParams entry, or what -NextJobProvider returns) with a -# scriptblock value -# - runspace-reachable code passing a variable that holds a scriptblock -# to a .NET method or constructor ($dict.TryAdd($k, $sb), -# [System.Lazy[object]]::new($factory)) - .NET receives a delegate, -# or stores it where another runspace can run it. Rule 8 covers the -# literal and cast forms of the same thing. +# Two ways a scriptblock reaches another runspace: +# - as a job parameter. Invoke-ZipParallelProcessing refuses those at +# runtime (Get-ZipJobScriptBlockKey), so every harness or real run +# that dispatches a job already catches it - no static check here. +# - through .NET or shared state: runspace-reachable code passing a +# scriptblock to a .NET method or constructor ($dict.TryAdd($k, $sb), +# [System.Lazy[object]]::new($Factory)), or storing one into a +# collection or object it didn't create itself ($Cache[$k] = $sb, +# $Slot.Factory = { ... }) - a parameter or outer variable, so +# possibly shared with the other engine. This rule flags a variable +# holding a scriptblock (assigned a scriptblock literal, or a +# [scriptblock]-typed parameter) passed to .NET, and any scriptblock +# value stored that way. Rule 8 covers literals and casts passed to .NET. # Add "# lint:ignore" on the line for a deliberate exception. -Write-Host "=== Lint: scriptblock crossing into a worker runspace ===" -ForegroundColor Cyan +Write-Host "=== Lint: scriptblock handed to .NET or stored into shared state on a runspace-reachable path ===" -ForegroundColor Cyan function Get-ScriptBlockVariableNames { param($AstNode) $names = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) foreach ($assign in $AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.AssignmentStatementAst] }, $true)) { if ($assign.Left -is [System.Management.Automation.Language.VariableExpressionAst] -and - $assign.Right -is [System.Management.Automation.Language.CommandExpressionAst]) { - $rhsExpr = $assign.Right.Expression - if ($rhsExpr -is [System.Management.Automation.Language.InvokeMemberExpressionAst]) { $rhsExpr = $rhsExpr.Expression } - if ($rhsExpr -is [System.Management.Automation.Language.ScriptBlockExpressionAst]) { $null = $names.Add($assign.Left.VariablePath.UserPath) } + $null -ne (Get-AssignedScriptBlockLiteral -Right $assign.Right)) { + $null = $names.Add($assign.Left.VariablePath.UserPath) } } - return ,$names -} - -function Test-IsScriptBlockValue { - param($Expr, $ScriptBlockVarNames) - if ($Expr -is [System.Management.Automation.Language.PipelineAst] -and $Expr.PipelineElements.Count -eq 1) { $Expr = $Expr.PipelineElements[0] } - if ($Expr -is [System.Management.Automation.Language.CommandExpressionAst]) { $Expr = $Expr.Expression } - if ($Expr -is [System.Management.Automation.Language.InvokeMemberExpressionAst]) { $Expr = $Expr.Expression } - if ($Expr -is [System.Management.Automation.Language.ScriptBlockExpressionAst]) { return $true } - return ($Expr -is [System.Management.Automation.Language.VariableExpressionAst] -and - $ScriptBlockVarNames.Contains($Expr.VariablePath.UserPath)) -} - -# Job-parameter hashtables for every Invoke-ZipParallelProcessing call -# under $RootAst: hashtables .Add()ed to (or assigned to) the -JobParams -# variable, and hashtables the -NextJobProvider scriptblock returns. -$jobParamScriptBlockFinder = { - param($RootAst) - $found = [System.Collections.Generic.List[object]]::new() - foreach ($call in $RootAst.FindAll({ param($n) $n -is [System.Management.Automation.Language.CommandAst] -and $n.GetCommandName() -eq 'Invoke-ZipParallelProcessing' }, $true)) { - $scope = $call.Parent - while ($scope -and -not ($scope -is [System.Management.Automation.Language.FunctionDefinitionAst])) { $scope = $scope.Parent } - if (-not $scope) { $scope = $RootAst } - $sbVarNames = Get-ScriptBlockVariableNames -AstNode $scope - - $jobParamsVar = $null; $providerVar = $null - for ($i = 0; ($i + 1) -lt $call.CommandElements.Count; $i++) { - $el = $call.CommandElements[$i]; $valEl = $call.CommandElements[$i + 1] - if ($el -isnot [System.Management.Automation.Language.CommandParameterAst] -or - $valEl -isnot [System.Management.Automation.Language.VariableExpressionAst]) { continue } - if ($el.ParameterName -eq 'JobParams') { $jobParamsVar = $valEl.VariablePath.UserPath } - if ($el.ParameterName -eq 'NextJobProvider') { $providerVar = $valEl.VariablePath.UserPath } - } - - $tables = [System.Collections.Generic.List[object]]::new() - if ($jobParamsVar) { - foreach ($m in $scope.FindAll({ param($n) $n -is [System.Management.Automation.Language.InvokeMemberExpressionAst] }, $true)) { - if ($m.Expression -is [System.Management.Automation.Language.VariableExpressionAst] -and - $m.Expression.VariablePath.UserPath -eq $jobParamsVar -and $m.Member.Extent.Text -eq 'Add' -and $m.Arguments) { - foreach ($argument in $m.Arguments) { - foreach ($h in $argument.FindAll({ param($n) $n -is [System.Management.Automation.Language.HashtableAst] }, $true)) { $tables.Add($h) } - } - } - } - } - foreach ($assign in $scope.FindAll({ param($n) $n -is [System.Management.Automation.Language.AssignmentStatementAst] }, $true)) { - if ($assign.Left -isnot [System.Management.Automation.Language.VariableExpressionAst]) { continue } - $assignedName = $assign.Left.VariablePath.UserPath - if ($jobParamsVar -and $assignedName -eq $jobParamsVar) { - foreach ($h in $assign.Right.FindAll({ param($n) $n -is [System.Management.Automation.Language.HashtableAst] }, $true)) { $tables.Add($h) } - } - if ($providerVar -and $assignedName -eq $providerVar) { - foreach ($ret in $assign.Right.FindAll({ param($n) $n -is [System.Management.Automation.Language.ReturnStatementAst] }, $true)) { - if ($null -eq $ret.Pipeline) { continue } - foreach ($h in $ret.Pipeline.FindAll({ param($n) $n -is [System.Management.Automation.Language.HashtableAst] }, $true)) { $tables.Add($h) } - } - } - } - - foreach ($h in $tables) { - foreach ($pair in $h.KeyValuePairs) { - if ($pair.Item2.Extent.StartScriptPosition.Line -match '#\s*lint:ignore') { continue } - if (Test-IsScriptBlockValue -Expr $pair.Item2 -ScriptBlockVarNames $sbVarNames) { - $found.Add(@{ Line = $pair.Item2.Extent.StartLineNumber; What = ("job parameter '" + $pair.Item1.Extent.Text + "' is a scriptblock") }) - } + foreach ($p in $AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.ParameterAst] }, $true)) { + foreach ($attr in $p.Attributes) { + if ($attr -is [System.Management.Automation.Language.TypeConstraintAst] -and + $attr.TypeName.FullName -match '^(System\.Management\.Automation\.)?scriptblock$') { + $null = $names.Add($p.Name.VariablePath.UserPath) } } } - return ,$found + return ,$names } -# A scriptblock-holding variable passed to a .NET method or constructor. $scriptBlockArgFinder = { param($AstNode) $found = [System.Collections.Generic.List[object]]::new() $sbVarNames = Get-ScriptBlockVariableNames -AstNode $AstNode - if ($sbVarNames.Count -eq 0) { return ,$found } foreach ($call in $AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.InvokeMemberExpressionAst] }, $true)) { if ($null -eq $call.Arguments) { continue } $memberName = [string]$call.Member.Extent.Text @@ -625,67 +607,87 @@ $scriptBlockArgFinder = { } } } + # Stores: $x[...] = / $x.Prop = , unless $x + # is a container this code built itself - every assignment to it creates + # a new one (@{}, @(), [Type]::new(), New-Object) and it isn't a + # parameter. An alias of something else ($c = $SharedCache), a parameter + # or a loop variable could be shared, so those count. + $assignments = @($AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.AssignmentStatementAst] }, $true)) + $freshNames = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) + $otherNames = [System.Collections.Generic.HashSet[string]]::new([System.StringComparer]::OrdinalIgnoreCase) + foreach ($p in $AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.ParameterAst] }, $true)) { $null = $otherNames.Add($p.Name.VariablePath.UserPath) } + foreach ($assign in $assignments) { + if ($assign.Left -isnot [System.Management.Automation.Language.VariableExpressionAst]) { continue } + $rhs = $assign.Right + $isFresh = $false + if ($rhs -is [System.Management.Automation.Language.CommandExpressionAst]) { + $expr = $rhs.Expression + if ($expr -is [System.Management.Automation.Language.ConvertExpressionAst]) { $expr = $expr.Child } + $isFresh = ($expr -is [System.Management.Automation.Language.HashtableAst]) -or + ($expr -is [System.Management.Automation.Language.ArrayExpressionAst]) -or + ($expr -is [System.Management.Automation.Language.ArrayLiteralAst]) -or + ($expr -is [System.Management.Automation.Language.InvokeMemberExpressionAst] -and $expr.Static -and $expr.Member.Extent.Text -eq 'new') + } elseif ($rhs -is [System.Management.Automation.Language.PipelineAst] -and $rhs.PipelineElements.Count -eq 1 -and + $rhs.PipelineElements[0] -is [System.Management.Automation.Language.CommandAst] -and $rhs.PipelineElements[0].GetCommandName() -eq 'New-Object') { + $isFresh = $true + } + if ($isFresh) { $null = $freshNames.Add($assign.Left.VariablePath.UserPath) } else { $null = $otherNames.Add($assign.Left.VariablePath.UserPath) } + } + foreach ($assign in $assignments) { + $target = $assign.Left + if ($target -isnot [System.Management.Automation.Language.IndexExpressionAst] -and + $target -isnot [System.Management.Automation.Language.MemberExpressionAst]) { continue } + $value = $null + if ($assign.Right -is [System.Management.Automation.Language.CommandExpressionAst]) { $value = $assign.Right.Expression } + $isScriptBlock = ($null -ne (Get-AssignedScriptBlockLiteral -Right $assign.Right)) -or + ($value -is [System.Management.Automation.Language.VariableExpressionAst] -and $sbVarNames.Contains($value.VariablePath.UserPath)) + if (-not $isScriptBlock) { continue } + $root = $target + while ($root -is [System.Management.Automation.Language.IndexExpressionAst] -or $root -is [System.Management.Automation.Language.MemberExpressionAst]) { + if ($root -is [System.Management.Automation.Language.IndexExpressionAst]) { $root = $root.Target } else { $root = $root.Expression } + } + if ($root -is [System.Management.Automation.Language.VariableExpressionAst] -and + $freshNames.Contains($root.VariablePath.UserPath) -and -not $otherNames.Contains($root.VariablePath.UserPath)) { continue } + if ($assign.Extent.StartScriptPosition.Line -match '#\s*lint:ignore') { continue } + $found.Add(@{ Line = $assign.Extent.StartLineNumber; What = ("stores a scriptblock into " + $target.Extent.Text + ", which it didn't create") }) + } return ,$found } -# Proves both finders fire, since nothing in the real code trips them. +# Proves the finder fires, since nothing in the real code trips it. $rule7SampleText = @' -function Sample-Caller { - $cb = { param($x) $x } - $plain = 'value' - $jobs = [System.Collections.Generic.List[hashtable]]::new() - $jobs.Add(@{ Name = $plain; Callback = $cb; Inline = { 1 }; Closure = { 2 }.GetNewClosure() }) - $provider = { - return @{ Path = $plain; Hook = $cb } - }.GetNewClosure() - $jobs.Add(@{ Skipped = $cb }) # lint:ignore - sample - Invoke-ZipParallelProcessing -WorkerScript $w -JobParams $jobs -NextJobProvider $provider -} function Sample-Worker { - $factory = { 3 }.GetNewClosure() - $lazy = [System.Lazy[object]]::new($factory) - $null = $dict.TryAdd('k', $factory) - $null = $dict.TryAdd('k', $plain) - $e = @(1, 2).Where($factory) + param([scriptblock]$Factory, [string]$Name, $Cache) + $built = { 3 }.GetNewClosure() + $result = { 4 }.Invoke() + $lazy = [System.Lazy[object]]::new($Factory) + $null = $dict.TryAdd('k', $built) + $null = $dict.TryAdd('k', $Name) + $null = $dict.TryAdd('k', $result) + $e = @(1, 2).Where($built) + $null = $dict.TryAdd('k', $built) # lint:ignore - sample + $Cache['k'] = $Factory + $Cache.Hook = { 5 }.GetNewClosure() + $local = @{} + $local['k'] = $built + $Cache['n'] = $Name + $alias = $Cache + $alias['a'] = $built + foreach ($slot in $Cache.Values) { $slot.Factory = $Factory } } '@ $rule7SampleAst = [System.Management.Automation.Language.Parser]::ParseInput($rule7SampleText, [ref]$null, [ref]$null) -$rule7JobSample = & $jobParamScriptBlockFinder $rule7SampleAst -$rule7JobSampleText = @($rule7JobSample | ForEach-Object { $_.What }) -Test-Assert ($rule7JobSampleText -contains "job parameter 'Callback' is a scriptblock") "Rule 7 flags a scriptblock variable in a -JobParams hashtable" -Test-Assert (($rule7JobSampleText -contains "job parameter 'Inline' is a scriptblock") -and ($rule7JobSampleText -contains "job parameter 'Closure' is a scriptblock")) "Rule 7 flags a scriptblock literal (bare or .GetNewClosure()'d) in a -JobParams hashtable" -Test-Assert ($rule7JobSampleText -contains "job parameter 'Hook' is a scriptblock") "Rule 7 flags a scriptblock in a hashtable the -NextJobProvider returns" -Test-Assert ($rule7JobSample.Count -eq 4) ("Rule 7 leaves plain job-parameter values and a lint:ignore'd line alone (flagged " + $rule7JobSample.Count + ", expected 4)") -$rule7ArgSample = & $scriptBlockArgFinder $rule7SampleAst.Find({ param($n) $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $n.Name -eq 'Sample-Worker' }, $true) -$rule7ArgSampleLines = @($rule7ArgSample | ForEach-Object { $_.Line }) -Test-Assert ($rule7ArgSampleLines -contains 14) "Rule 7 flags a scriptblock variable passed to a constructor ([System.Lazy]::new(`$factory))" -Test-Assert ($rule7ArgSampleLines -contains 15) "Rule 7 flags a scriptblock variable stored through a .NET method (TryAdd)" -Test-Assert ($rule7ArgSample.Count -eq 2) ("Rule 7 leaves a plain value and .Where() alone (flagged " + $rule7ArgSample.Count + ", expected 2)") - -$rule7 = @() -foreach ($name in $fileAsts.Keys) { - foreach ($hit in (& $jobParamScriptBlockFinder $fileAsts[$name])) { - $rule7 += @{ File = $name; Line = $hit.Line; Text = $hit.What } - } -} -foreach ($fname in $runspaceReachable) { - if (-not $fnAstByName.ContainsKey($fname)) { continue } - $fnAst = $fnAstByName[$fname] - $fnFile = $null - foreach ($name in $fileAsts.Keys) { if ($fileText[$name].Contains($fnAst.Extent.Text)) { $fnFile = $name; break } } - foreach ($hit in (& $scriptBlockArgFinder $fnAst)) { - $rule7 += @{ File = $fnFile; Line = $hit.Line; Text = ($fname + " (runspace-reachable) " + $hit.What) } - } -} -foreach ($workerVar in $runspaceWorkerBodies) { - $body = $scriptBlockVars[$workerVar] - $workerFile = $null - foreach ($name in $fileAsts.Keys) { if ($fileText[$name].Contains($body.Extent.Text)) { $workerFile = $name; break } } - foreach ($hit in (& $scriptBlockArgFinder $body)) { - $rule7 += @{ File = $workerFile; Line = $hit.Line; Text = ("worker `$" + $workerVar + " " + $hit.What) } - } -} -Test-Assert (@($rule7).Count -eq 0) "no scriptblock is handed to a worker runspace as a job parameter or passed to .NET from runspace-reachable code" +$rule7Sample = & $scriptBlockArgFinder $rule7SampleAst +$rule7SampleLines = @($rule7Sample | ForEach-Object { $_.Line }) +Test-Assert ($rule7SampleLines -contains 5) "Rule 7 flags a [scriptblock] parameter passed to a constructor ([System.Lazy]::new(`$Factory))" +Test-Assert ($rule7SampleLines -contains 6) "Rule 7 flags a .GetNewClosure()'d scriptblock variable stored through a .NET method (TryAdd)" +Test-Assert ($rule7SampleLines -contains 11) "Rule 7 flags a scriptblock stored through an indexer on a container it didn't create (`$Cache['k'] = `$Factory)" +Test-Assert ($rule7SampleLines -contains 12) "Rule 7 flags a scriptblock literal stored into a property of a container it didn't create (`$Cache.Hook = { }.GetNewClosure())" +Test-Assert (($rule7SampleLines -contains 17) -and ($rule7SampleLines -contains 18)) "Rule 7 flags a store through a local alias of a shared container, and through a loop variable" +Test-Assert ($rule7Sample.Count -eq 6) ("Rule 7 leaves a plain parameter, a scriptblock's .Invoke() result, .Where(), a lint:ignore'd line, a container it built itself and a plain stored value alone (flagged " + $rule7Sample.Count + ", expected 6)") + +$rule7 = Get-RunspaceReachableFindings -Finder $scriptBlockArgFinder +Test-Assert (@($rule7).Count -eq 0) "no runspace-reachable function or worker body passes a scriptblock to .NET or stores one into a container it didn't create" Show-Findings -Findings @($rule7) Write-Host "" @@ -755,25 +757,78 @@ Test-Assert ($rule8SampleLines -contains 2) "Rule 8 flags a written delegate cas Test-Assert ($rule8SampleLines -contains 3) "Rule 8 flags a scriptblock passed straight to a .NET method (GetOrAdd)" Test-Assert ($rule8SampleLines -contains 4) "Rule 8 flags a scriptblock passed straight to a constructor ([System.Lazy]::new)" Test-Assert ($rule8SampleFound.Count -eq 3) ("Rule 8 leaves .ForEach()/.Where(), a lint:ignore'd line and a plain call alone (flagged " + $rule8SampleFound.Count + ", expected 3)") -foreach ($fname in $runspaceReachable) { - if (-not $fnAstByName.ContainsKey($fname)) { continue } - $fnAst = $fnAstByName[$fname] - $fnFile = $null - foreach ($name in $fileAsts.Keys) { if ($fileText[$name].Contains($fnAst.Extent.Text)) { $fnFile = $name; break } } - foreach ($hit in (& $delegateCastFinder $fnAst)) { - $rule8 += @{ File = $fnFile; Line = $hit.Line; Text = ($fname + " (runspace-reachable) " + $hit.What) } +$rule8 = Get-RunspaceReachableFindings -Finder $delegateCastFinder +Test-Assert (@($rule8).Count -eq 0) "no runspace-reachable function or worker body turns a scriptblock into a delegate (written cast or implicit)" +Show-Findings -Findings @($rule8) +Write-Host "" + +# ------------------------------------------------------------------------ +# Rule 9: host cmdlet on a runspace-reachable path +# ------------------------------------------------------------------------ +# Worker runspaces share the real console host (the pool is built with +# $Host), so a worker writing straight to it bypasses the console queue the +# main thread drains - its lines interleave with, and scramble, Clean +# Console Output's live display - and Read-Host would prompt from a +# background thread mid-run. Worker output goes through Write-Log (the +# console queue) instead. Other cmdlets are fine in runspace code. +# Deliberate, reviewed exceptions go in $script:LintHostCmdletAllowed with +# a reason. +Write-Host "=== Lint: host cmdlet on a runspace-reachable path ===" -ForegroundColor Cyan + +# The same goes for writing to the console directly: [Console]::... and +# $Host.UI. Those go in the allow-list as '[Console]' and '$Host.UI'. +$hostCmdletNames = @('Write-Host', 'Read-Host', 'Out-Host', 'Clear-Host', 'Get-Credential', 'Out-GridView') +# Aliases (cls, clear, oh, ogv, ...) resolve to the cmdlet they stand for. +$hostCmdletByName = @{} +foreach ($n in $hostCmdletNames) { $hostCmdletByName[$n] = $n } +foreach ($a in @(Get-Alias -Definition $hostCmdletNames -ErrorAction SilentlyContinue)) { $hostCmdletByName[$a.Name] = $a.Definition } +$hostCmdletFinder = { + param($AstNode, $FunctionName) + $found = [System.Collections.Generic.List[object]]::new() + $hits = [System.Collections.Generic.List[object]]::new() + foreach ($c in $AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.CommandAst] }, $true)) { + $cmdName = $c.GetCommandName() + if (-not $cmdName) { continue } + $bareName = $cmdName -replace '^[^\\]+\\', '' # Microsoft.PowerShell.Utility\Write-Host -> Write-Host + if (-not $hostCmdletByName.ContainsKey($bareName)) { continue } + $hits.Add(@{ Ast = $c; Name = $hostCmdletByName[$bareName]; What = ("calls host cmdlet " + $cmdName) }) } -} -foreach ($workerVar in $runspaceWorkerBodies) { - $body = $scriptBlockVars[$workerVar] - $workerFile = $null - foreach ($name in $fileAsts.Keys) { if ($fileText[$name].Contains($body.Extent.Text)) { $workerFile = $name; break } } - foreach ($hit in (& $delegateCastFinder $body)) { - $rule8 += @{ File = $workerFile; Line = $hit.Line; Text = ("worker `$" + $workerVar + " " + $hit.What) } + foreach ($m in $AstNode.FindAll({ param($n) $n -is [System.Management.Automation.Language.MemberExpressionAst] }, $true)) { + if ($m.Static -and $m.Expression -is [System.Management.Automation.Language.TypeExpressionAst] -and + $m.Expression.TypeName.FullName -match '^(System\.)?Console$') { + $hits.Add(@{ Ast = $m; Name = '[Console]'; What = ("uses [Console]::" + $m.Member.Extent.Text) }) + } elseif ($m.Expression -is [System.Management.Automation.Language.VariableExpressionAst] -and + $m.Expression.VariablePath.UserPath -match '^(global:)?Host$' -and $m.Member.Extent.Text -eq 'UI') { + $hits.Add(@{ Ast = $m; Name = '$Host.UI'; What = "uses `$Host.UI" }) + } + } + foreach ($h in $hits) { + if ($FunctionName -and (Test-IsHostCmdletAllowed -Function $FunctionName -Cmdlet $h.Name)) { continue } + if ($h.Ast.Extent.StartScriptPosition.Line -match '#\s*lint:ignore') { continue } + $found.Add(@{ Line = $h.Ast.Extent.StartLineNumber; What = $h.What }) } + return ,$found } -Test-Assert (@($rule8).Count -eq 0) "no runspace-reachable function or worker body turns a scriptblock into a delegate (written cast or implicit)" -Show-Findings -Findings @($rule8) +$rule9SampleText = @' +function Sample-Worker { + Write-Host 'x' + $a = Read-Host + Write-Output 'y' + cls + [Console]::WriteLine('z') + $Host.UI.WriteLine('w') + $width = $Host.Name + Microsoft.PowerShell.Utility\Write-Host 'q' +} +'@ +$rule9SampleAst = [System.Management.Automation.Language.Parser]::ParseInput($rule9SampleText, [ref]$null, [ref]$null) +$rule9Sample = & $hostCmdletFinder $rule9SampleAst 'Sample-Worker' +$rule9SampleLines = @($rule9Sample | ForEach-Object { $_.Line } | Sort-Object) +Test-Assert (($rule9SampleLines -join ',') -eq '2,3,5,6,7,9') ("Rule 9 flags Write-Host, Read-Host, an alias (cls), [Console]::, `$Host.UI and a module-qualified name, and leaves Write-Output and other `$Host members alone (flagged lines " + ($rule9SampleLines -join ',') + ", expected 2,3,5,6,7,9)") + +$rule9 = Get-RunspaceReachableFindings -Finder $hostCmdletFinder +Test-Assert (@($rule9).Count -eq 0) "no runspace-reachable function or worker body calls a host cmdlet" +Show-Findings -Findings @($rule9) Write-Host "" # ============================================================================== diff --git a/_Harness/ZipParallelProcessing.Harness.ps1 b/_Harness/ZipParallelProcessing.Harness.ps1 index a17006b..7755a2d 100644 --- a/_Harness/ZipParallelProcessing.Harness.ps1 +++ b/_Harness/ZipParallelProcessing.Harness.ps1 @@ -533,6 +533,135 @@ Test-Assert ($outcomeG -is [hashtable]) ("the return is exactly one hashtable (g Test-Assert ($outcomeG -is [hashtable] -and $outcomeG.ForceStoppedCount -eq 0) "ForceStoppedCount is still readable on it" Write-Host "" +# ============================================================================== +# Scenario H: a scriptblock job value is refused before any worker runs. +# A scriptblock belongs to the runspace that created it; run inside a worker +# it can deadlock or crash the process. +# ============================================================================== +Write-Host "=== Scenario H: scriptblock job values are refused ===" -ForegroundColor Cyan +Reset-HarnessAbortState +$sharedH = @(Get-LoggingSharedFunctionNames) +function Invoke-ScenarioHJobs { + param($Job, [switch]$UseProvider) + $results = [System.Collections.Generic.List[hashtable]]::new() + $cts = [System.Threading.CancellationTokenSource]::new() + $err = $null + try { + if ($UseProvider) { + $state = @{ Given = $false } + $provider = { if ($state.Given) { return $null }; $state.Given = $true; return $Job }.GetNewClosure() + $null = Invoke-ZipParallelProcessing -WorkerScript $fakeWorkerOk -NextJobProvider $provider -SharedFunctionNames $sharedH ` + -MaxParallel 1 -Cts $cts -ResultHandler { param($r) $results.Add($r) }.GetNewClosure() + } else { + $list = [System.Collections.Generic.List[hashtable]]::new() + $list.Add($Job) + $null = Invoke-ZipParallelProcessing -WorkerScript $fakeWorkerOk -JobParams $list -SharedFunctionNames $sharedH ` + -MaxParallel 1 -Cts $cts -ResultHandler { param($r) $results.Add($r) }.GetNewClosure() + } + } catch { $err = $_.ToString() } + $cts.Dispose() + return @{ Error = $err; Results = $results.Count } +} +$baseH = @{ JobId = 1; DelayMs = 0; DoFullLogging = $FullLogging; LogFile = $logFile; LogErrorFile = $logErrorFile; LogWarnFile = $logWarnFile } +function New-ScenarioHJob { param([hashtable]$Extra) $j = $baseH.Clone(); foreach ($k in $Extra.Keys) { $j[$k] = $Extra[$k] }; return $j } + +$h1 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Callback = { 1 } }) +Test-Assert ($null -ne $h1.Error -and $h1.Error -match "'Callback'" -and $h1.Error -match '\[scriptblock\]::Create') ("a -JobParams scriptblock value throws, naming the key and the workaround (got: " + $h1.Error + ")") +Test-Assert ($h1.Results -eq 0) "no worker ran for the refused -JobParams list" +$h2 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Steps = @('a', { 2 }) }) +Test-Assert ($null -ne $h2.Error -and $h2.Error -match "'Steps'") "a scriptblock inside an array job value is refused" +$h3 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Options = @{ OnDone = { 3 } } }) +Test-Assert ($null -ne $h3.Error -and $h3.Error -match "'Options'") "a scriptblock inside a hashtable job value is refused" +$h4 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Hook = { 4 } }) -UseProvider +Test-Assert ($null -ne $h4.Error -and $h4.Error -match "'Hook'" -and $h4.Results -eq 0) "a scriptblock in a -NextJobProvider job is refused before that job runs" +$h5 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Code = '{ 5 }'; Names = @('x', 'y'); Map = @{ K = 'v' } }) +Test-Assert ($null -eq $h5.Error -and $h5.Results -eq 1) ("plain data, including code passed as a string, still runs (error: " + $h5.Error + ", results: " + $h5.Results + ")") +$h6 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Steps = @(@{ Run = { 6 } }) }) +Test-Assert ($null -ne $h6.Error -and $h6.Error -match "'Steps'") "a scriptblock two levels down (an array of hashtables) is refused" +$h7 = Invoke-ScenarioHJobs -Job (New-ScenarioHJob @{ Config = [pscustomobject]@{ Name = 'x'; OnDone = { 7 } } }) +Test-Assert ($null -ne $h7.Error -and $h7.Error -match "'Config'") "a scriptblock held in a [pscustomobject] property is refused" + +# Test doubles as PowerShell classes (built in memory - no Add-Type compile). +class ZppCountingCollection : System.Collections.ICollection { + [int]$Enumerations + [int] get_Count() { return 2 } + [bool] get_IsSynchronized() { return $false } + [object] get_SyncRoot() { return $this } + [void] CopyTo([System.Array]$array, [int]$index) { } + [System.Collections.IEnumerator] GetEnumerator() { $this.Enumerations++; return @('a', 'b').GetEnumerator() } +} +class ZppThrowingCollection : System.Collections.ICollection { + [bool]$AsModified + ZppThrowingCollection([bool]$asModified) { $this.AsModified = $asModified } + [int] get_Count() { return 1 } + [bool] get_IsSynchronized() { return $false } + [object] get_SyncRoot() { return $this } + [void] CopyTo([System.Array]$array, [int]$index) { } + [System.Collections.IEnumerator] GetEnumerator() { + if ($this.AsModified) { throw [System.InvalidOperationException]::new('Collection was modified; enumeration operation may not execute.') } + throw [System.UnauthorizedAccessException]::new('simulated read failure') + } +} + +# A collection shared by reference across jobs is read once per run, not +# once per job: later jobs may run while workers are writing to it. +$counterH = [ZppCountingCollection]::new() +$seenH = [System.Collections.Generic.HashSet[object]]::new() +$keysH = @(@(foreach ($i in 1..3) { Get-ZipJobScriptBlockKey -Job @{ JobId = $i; Shared = $counterH } -Seen $seenH }) | Where-Object { $null -ne $_ }) +Test-Assert ($keysH.Count -eq 0 -and $counterH.Enumerations -eq 1) ("a collection shared by three jobs is enumerated once, not per job (enumerated " + $counterH.Enumerations + " times)") + +# Same for a shared [pscustomobject]: its properties are read once. +$probeH = @{ Reads = 0 } +$sharedObjH = [pscustomobject]@{ Name = 'shared' } +$sharedObjH | Add-Member -MemberType ScriptProperty -Name Probe -Value { $probeH.Reads++; 'x' }.GetNewClosure() +$seenH2 = [System.Collections.Generic.HashSet[object]]::new() +foreach ($i in 1..3) { $null = Get-ZipJobScriptBlockKey -Job @{ JobId = $i; State = $sharedObjH } -Seen $seenH2 } +Test-Assert ($probeH.Reads -eq 1) ("a [pscustomobject] shared by three jobs is read once, not per job (read " + $probeH.Reads + " times)") + +# A consuming enumerable is left alone - reading it would take the items +# the worker is meant to get. +$queueH = [System.Collections.Concurrent.BlockingCollection[object]]::new() +$queueH.Add('first'); $queueH.Add('second'); $queueH.CompleteAdding() +$null = Get-ZipJobScriptBlockKey -Job @{ JobId = 1; Work = $queueH.GetConsumingEnumerable() } -Seen ([System.Collections.Generic.HashSet[object]]::new()) +Test-Assert ($queueH.Count -eq 2) ("checking a job doesn't consume a GetConsumingEnumerable() value (" + $queueH.Count + " of 2 items left)") + +# Scriptblocks inside struct entries (KeyValuePair) are found. +$pairsH = [System.Collections.Generic.List[System.Collections.Generic.KeyValuePair[string,object]]]::new() +$pairsH.Add([System.Collections.Generic.KeyValuePair[string,object]]::new('plain', 1)) +$pairsH.Add([System.Collections.Generic.KeyValuePair[string,object]]::new('hook', { 10 })) +$kvpKeyH = Get-ZipJobScriptBlockKey -Job @{ JobId = 1; Pairs = $pairsH } -Seen ([System.Collections.Generic.HashSet[object]]::new()) +Test-Assert ($kvpKeyH -eq 'Pairs') ("a scriptblock in a KeyValuePair's value is found (got: " + $kvpKeyH + ")") + +# A collection being modified is let through; any other read failure is +# reported, naming the key, instead of silently passing. +$modifiedKeyH = Get-ZipJobScriptBlockKey -Job @{ JobId = 1; Busy = [ZppThrowingCollection]::new($true) } -Seen ([System.Collections.Generic.HashSet[object]]::new()) +Test-Assert ($null -eq $modifiedKeyH) "a collection that's being modified (InvalidOperationException) is let through" +$readErrH = $null +try { $null = Get-ZipJobScriptBlockKey -Job @{ JobId = 1; Broken = [ZppThrowingCollection]::new($false) } -Seen ([System.Collections.Generic.HashSet[object]]::new()) } catch { $readErrH = $_.ToString() } +Test-Assert ($null -ne $readErrH -and $readErrH -match "'Broken'" -and $readErrH -match 'simulated read failure') ("any other read failure is reported with the key and the cause (got: " + $readErrH + ")") + +# A provider job refused mid-run lets the jobs already running finish +# normally, then throws - it doesn't stop them or cancel the caller's token. +$resultsH9 = [System.Collections.Generic.List[hashtable]]::new() +$ctsH9 = [System.Threading.CancellationTokenSource]::new() +$stateH9 = @{ N = 0 } +$providerH9 = { + $stateH9.N++ + if ($stateH9.N -eq 1) { return (New-ScenarioHJob @{ JobId = 1; DelayMs = 800 }) } + if ($stateH9.N -eq 2) { return (New-ScenarioHJob @{ JobId = 2; Hook = { 9 } }) } + return $null +} +$errH9 = $null +try { + $null = Invoke-ZipParallelProcessing -WorkerScript $fakeWorkerOk -NextJobProvider $providerH9 -SharedFunctionNames $sharedH ` + -MaxParallel 2 -Cts $ctsH9 -ResultHandler { param($r) $resultsH9.Add($r) } +} catch { $errH9 = $_.ToString() } +Test-Assert ($null -ne $errH9 -and $errH9 -match "'Hook'") "a provider job refused mid-run still makes the call throw" +Test-Assert ($resultsH9.Count -eq 1 -and -not $ctsH9.Token.IsCancellationRequested) ("the job already running finished and reported, and the caller's token wasn't cancelled (results " + $resultsH9.Count + ", cancelled " + $ctsH9.Token.IsCancellationRequested + ")") +$ctsH9.Dispose() +Reset-HarnessAbortState +Write-Host "" + # ============================================================================== # Summary # ============================================================================== diff --git a/module-descriptions.md b/module-descriptions.md index ce9e54a..12d6865 100644 --- a/module-descriptions.md +++ b/module-descriptions.md @@ -27,12 +27,13 @@ Every module is a file of function (and type) definitions with no side effects w ## Rules for code that runs in worker runspaces -Zip Folders' workers and Full Pipeline's two engines run in separate PowerShell runspaces, and anything they call must follow two rules. Each module's section says which of its functions run there. +Zip Folders' workers and Full Pipeline's two engines run in separate PowerShell runspaces, and anything they call must follow three rules. Each module's section says which of its functions run there. 1. **Don't read `$script:` variables directly.** A function copied into a runspace can resolve `$script:` to an empty scope (inside a `.GetNewClosure()` block, for example), and the read silently comes back empty. Workers get their state as parameters, or through `Get-WorkerRunspaceValue` (`Logging.ps1`). -2. **Never run a scriptblock in a runspace other than the one that created it.** Workers get plain data, not scriptblocks, as job parameters and through shared caches, and no scriptblock is handed to .NET as a delegate (`[Func[...]]{ }`, a `System.Lazy` factory, or a scriptblock passed to a method such as `ConcurrentDictionary.GetOrAdd`), since .NET can then call it from another engine's thread. A scriptblock running on another runspace's thread queues every scriptblock it calls (a `Where-Object` filter, a nested delegate) back to the runspace it came from and waits. If that runspace is waiting on the first thread, both hang; if it's idle, the process can crash. +2. **Never run a scriptblock in a runspace other than the one that created it.** Workers get plain data, not scriptblocks, as job parameters (`Invoke-ZipParallelProcessing` refuses a scriptblock job value) and through shared caches, and no scriptblock is handed to .NET as a delegate (`[Func[...]]{ }`, a `System.Lazy` factory, or a scriptblock passed to a method such as `ConcurrentDictionary.GetOrAdd`), since .NET can then call it from another engine's thread. A scriptblock running on another runspace's thread queues every scriptblock it calls (a `Where-Object` filter, a nested delegate) back to the runspace it came from and waits. If that runspace is waiting on the first thread, both hang; if it's idle, the process can crash. +3. **Don't write to the console directly.** Workers share the real console host, so `Write-Host`, `Read-Host`, `Clear-Host`, `[Console]::...` or `$Host.UI` in worker code bypasses the console queue and scrambles Clean Console Output's live display. Worker output goes through `Write-Log`, which queues it for the main thread. -Cmdlets are fine in worker code. The code uses .NET calls in many of those places for historical reasons, not because a cmdlet would break. +Other cmdlets are fine in worker code. The code uses .NET calls in many of those places for historical reasons, not because a cmdlet would break. A runspace starts with only the functions it's given, so a function a worker calls must be in its caller's `-SharedFunctionNames` list (built from each module's `Get-...SharedFunctionNames`). `_Harness\Lint.Harness.ps1` checks all of this. @@ -489,12 +490,14 @@ A shared runspace-pool dispatcher. It runs a worker scriptblock once per job in | Function | What it does | |---|---| +| `Get-ZipJobScriptBlockKey` | Returns the first key in a job hashtable whose value is, or holds, a scriptblock (up to four levels down, through collections, key/value pairs and `[pscustomobject]` properties); `$null` if there is none. | | `Invoke-ZipParallelProcessing` | Runs `-WorkerScript` for each job in a runspace pool of at most `-MaxParallel`, feeds every result to `-ResultHandler` on the main thread, and returns `@{ ForceStoppedCount = n }`. | **How it works** - Jobs come from exactly one of two sources: `-JobParams` (a fixed list of parameter hashtables) or `-NextJobProvider` (a scriptblock called on the main thread that returns the next hashtable, or `$null` when finished). Supplying both or neither throws. An empty `-JobParams` list returns immediately. - Each job's hashtable keys are passed to the worker as named parameters. The function adds three more: `CancelToken` (from `-Cts`), `LogMutex` and `ConsoleQueue`. A worker script must declare those parameters. +- Every job is checked before it starts: a value that is, or holds, a scriptblock (up to four levels down, through collections, key/value pairs and `[pscustomobject]` properties) is refused with a message naming the key (see the worker-runspace rules above). Only materialized collections are read (anything implementing `ICollection`, `ICollection` or `IReadOnlyCollection`); a lazy or consuming enumerable such as `GetConsumingEnumerable()` is passed through unread, since reading it would take the items the worker is meant to get. A `-JobParams` list is checked whole before anything runs, so nothing starts. A refused `-NextJobProvider` job stops new jobs from starting; the jobs already running finish and report normally, and then the call throws. Each collection or custom object is read once per call, so one passed by reference to every job is checked with the first job, before workers are writing to it. A collection that reports it is being modified is let through; any other failure to read a value throws, naming the key. To hand a worker code, pass it as a string and build it inside the worker with `[scriptblock]::Create()`. - The pool is built from a fresh `InitialSessionState`. Only the functions named in `-SharedFunctionNames` are copied into it (by reading each function's definition and adding a function entry). A name that doesn't resolve throws a message naming the entry. Nothing else from the calling script exists inside a worker. - Each run creates its own log mutex (`Initialize-LogMutex`) and a `ConcurrentQueue` for console output. The previous `$script:logMutex` is restored, and the new mutex disposed, when the call ends. - The main loop keeps the pool filled up to `-MaxParallel`, then on each pass: checks `Test-AbortSignal`, flushes the console queue (`Flush-ConsoleQueue`), calls `-PollCallback` if given, and harvests completed jobs. It sleeps 100 ms only when it has nothing to start.