Skip to content

Test-ModuleManifest is not thread safe (exceptions thrown from deep inside PowerShell engine) #9889

Closed

Description

PSScriptAnalyzer version of 1.18.0 or older sporadically encounter an exception when calling Test-ModuleManifest concurrently, it seems to be an issue of the PowerShell engine code itself, not the cmdlet
The issue applies to any PowerShell version (5, 6.2, 7.0-preview1)

For context, here is how PSSA calls Test-ModuleManifest in the Singleton Helper class by creating an instance of PowerShell each time (that gets disposed afterwards) but with a re-used runspace pool (previous versions did not re-use the runspace pool and suffered from the same issue as well though).
https://github.com/PowerShell/PSScriptAnalyzer/blob/0bef1c8b8ddf5b570dd0b6c90f0d0257b94eb374/Engine/Helper.cs#L306-L315

using (var ps = System.Management.Automation.PowerShell.Create())
{
    ps.RunspacePool = _runSpacePool;
    ps.AddCommand("Test-ModuleManifest")
        .AddParameter("Path", filePath)
        .AddParameter("WarningAction", ActionPreference.SilentlyContinue);
    try
    {
        psObj = ps.Invoke();
    }
    { ... }
}

The above code gets called from from different threads (each rule has its own thread) that are started as follows here:
https://github.com/PowerShell/PSScriptAnalyzer/blob/a68b13956ab1136af91066d1ce61113574447945/Engine/ScriptAnalyzer.cs#L2075

allowedRules.Select(scriptRule => Task.Factory.StartNew(() => .... );

Steps to reproduce

Extract the files in the below zip to a folder (yes, they are a bit artificial but are a good and simple way of making the race condition more reliably on most machines (my machine is a 4 core i7).
repro.zip

Install-module PSScriptAnalyzer -RequiredVersion 1.18.0
1..100 | Invoke-ScriptAnalyzer -Path $PathToReproPsd1

Expected behavior

No leaked errors

Actual behavior

PowerShell leaks errors that indicate a thread safety problem.
PR #9860 offered a fix but the review concluded it was not the right fix/place.

The stack trace is below and I uploaded a minidump with heap here from the latest master commit bdc8548700118cea6c31420c2f7478785e6d84ac of PowerShell for one of the most common exceptions (CmdletInvocationException with inner exception from System.Collections.ListDictionaryInternal: 'Collection was modified; enumeration operation may not execute.')

   at System.Management.Automation.Runspaces.PipelineBase.Invoke(IEnumerable input) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\pipelinebase.cs:line 416
   at System.Management.Automation.Runspaces.Pipeline.Invoke() in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\Pipeline.cs:line 531
   at System.Management.Automation.PowerShell.Worker.ConstructPipelineAndDoWork(Runspace rs, Boolean performSyncInvoke) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 5619
   at System.Management.Automation.PowerShell.Worker.CreateRunspaceIfNeededAndDoWork(Runspace rsToUse, Boolean isSync) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 5466
   at System.Management.Automation.PowerShell.CoreInvokeHelper[TInput,TOutput](PSDataCollection`1 input, PSDataCollection`1 output, PSInvocationSettings settings) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 4588
   at System.Management.Automation.PowerShell.CoreInvoke[TInput,TOutput](PSDataCollection`1 input, PSDataCollection`1 output, PSInvocationSettings settings) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 4780
   at System.Management.Automation.PowerShell.CoreInvoke[TOutput](IEnumerable input, PSDataCollection`1 output, PSInvocationSettings settings) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 4530
   at System.Management.Automation.PowerShell.Invoke(IEnumerable input, PSInvocationSettings settings) in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 2425
   at System.Management.Automation.PowerShell.Invoke() in C:\Users\chris\PowerShell\src\System.Management.Automation\engine\hostifaces\PowerShell.cs:line 2296
   at Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidUsingDeprecatedManifestFields.AnalyzeScript(Ast ast, String fileName)+MoveNext()

I wonder if the call to return Output.NonBlockingRead(Int32.MaxValue); in pipelinebase.cs (line 416) would need to be blocking?

Environment data

Windows (any version, e.g. 1903), Any version of PowerShell (5.1, 6.2, 7.0-prevew1, latest commit from master)

Activity

  1. added
    Issue-BugIssue has been identified as a bug in the product
    and removed
    Issue-Questionideally support can be provided via other mechanisms, but sometimes folks do open an issue to get a
    on Jun 14, 2019
  2. microsoft-github-policy-service commented on Nov 16, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

  3. microsoft-github-policy-service commented on Nov 16, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

  4. microsoft-github-policy-service commented on Nov 16, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

  5. microsoft-github-policy-service commented on Nov 23, 2023

    @microsoft-github-policy-service
    Contributor

    This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

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

    Issue-BugIssue has been identified as a bug in the productResolution-No ActivityIssue has had no activity for 6 months or moreWG-Cmdletsgeneral cmdlet issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions