Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collection was modified; enumeration operation may not execute when running Invoke-ScriptAnalyzer #1516

Closed
mrboring opened this issue May 30, 2020 · 9 comments · Fixed by #1523

Comments

@mrboring
Copy link

Before submitting a bug report:

  • Perform a quick search for existing issues to check if this bug has already been reported

This may be related to:
PSUseToExportFieldsInManifest throw System.InvalidOperationException: Collection was modified; enumeration operation may not execute #902

Steps to reproduce

PSScriptAnalyzer - Collection was modified.zip

  1. Unzip the above into a folder.
  2. Open that folder in PowerShell 7.
  3. Execute: $x = .\Test.ps1 -Run 10 -SAVersion '1.19.0'
  4. The results will contain entries with Collection was modified; enumeration operation may not execute exceptions. If run several times different files will have these exceptions.

Expected behavior

No Collection was modified; enumeration operation may not execute. issues.

If I change to a previous version of the module the issue goes away:

  1. Unzip the above into a folder.
  2. Open that folder in PowerShell 7.
  3. Execute: $x = .\Test.ps1 -Run 10 -SAVersion '1.18.3'
  4. The results will contain NO Collection was modified; enumeration operation may not execute exceptions. If run several times there will never be any such exceptions.

Actual behavior

Several issues are returned. If run multiple times different files show the issues. Sample error message:

Invoke-ScriptAnalyzer: C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Test.ps1:29
Line |
  29 |$issues = Invoke-ScriptAnalyzer -Path $file -Settings $settings `
     |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Collection was modified; enumeration operation may not execute.

If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *

PSMessageDetails      :
Exception             : System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
                           at System.Collections.Generic.Dictionary`2.Enumerator.MoveNext()
                           at System.Management.Automation.CommandInfo.get_Parameters()
                           at Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseCorrectCasing.AnalyzeScript(Ast ast, String fileName)+MoveNext()
                           at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
                           at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
                           at Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<>c__DisplayClass82_1.<AnalyzeSyntaxTree>b__2()
TargetObject          : C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Functions\Private\Get-HFGitStatus.ps1
CategoryInfo          : InvalidOperation: (C:\Data\PowerShell\…Get-HFGitStatus.ps1:String) [Invoke-ScriptAnalyzer], InvalidOperationException
FullyQualifiedErrorId : RULE_ERROR,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand
ErrorDetails          :
InvocationInfo        : System.Management.Automation.InvocationInfo
ScriptStackTrace      : at <ScriptBlock>, C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Test.ps1: line 29
                        at <ScriptBlock>, <No file>: line 1
PipelineIterationInfo : {0, 1}
> Get-Error -InputObject $x[0].ErrorRecord

Exception             :
    Type       : System.InvalidOperationException
    TargetSite :
        Name          : ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion
        DeclaringType : System.ThrowHelper, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
        MemberType    : Method
        Module        : System.Private.CoreLib.dll
    StackTrace :
   at System.Collections.Generic.Dictionary`2.Enumerator.MoveNext()
   at System.Management.Automation.CommandInfo.get_Parameters()
   at Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseCorrectCasing.AnalyzeScript(Ast ast, String fileName)+MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<>c__DisplayClass82_1.<AnalyzeSyntaxTree>b__2()
    Message    : Collection was modified; enumeration operation may not execute.
    Source     : System.Private.CoreLib
    HResult    : -2146233079
TargetObject          : C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Functions\Private\Get-HFGitStatus.ps1
CategoryInfo          : InvalidOperation: (C:\Data\PowerShell\…Get-HFGitStatus.ps1:String) [Invoke-ScriptAnalyzer], InvalidOperationException
FullyQualifiedErrorId : RULE_ERROR,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand
InvocationInfo        :
    MyCommand        : Invoke-ScriptAnalyzer
    ScriptLineNumber : 29
    OffsetInLine     : 19
    HistoryId        : 12
    ScriptName       : C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Test.ps1
    Line             : $issues = Invoke-ScriptAnalyzer -Path $file -Settings $settings `

    PositionMessage  : At C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Test.ps1:29 char:19
                       + …     $issues = Invoke-ScriptAnalyzer -Path $file -Settings $settings `
                       +                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    PSScriptRoot     : C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified
    PSCommandPath    : C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Test.ps1
    InvocationName   : Invoke-ScriptAnalyzer
    CommandOrigin    : Internal
ScriptStackTrace      : at <ScriptBlock>, C:\Data\PowerShell\Miscellaneous\Issues\PSScriptAnalyzer - Collection was modified\Test.ps1: line 29
                        at <ScriptBlock>, <No file>: line 1
PipelineIterationInfo :


Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.0.1
PSEdition                      Core
GitCommitId                    7.0.1
OS                             Microsoft Windows 10.0.18363
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

1.19.0
1.18.3
1.18.2
1.18.1
1.18.0
1.17.1
@bergmeister
Copy link
Collaborator

bergmeister commented Jun 1, 2020

Thank you for the detailed report. I can reproduce (only had to change the hard-coded path to the custom rule in the psd1 to match my local path) and can confirm this happens with 1.19.0 but not 1.18.3. As the origin of this bug is the PSUseCorrectCasing rule, a temporary workaround is to either use 1.18.3 or not enable that rule in the settings file until we have a fix.
Looking at it in the debugger, it happens exactly here when calling the .Parameters property on the CommandInfo object of Test-Path:

var availableParameters = commandInfo.Parameters;

Full stack trace is:

   at System.ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion()
   at System.Collections.Generic.Dictionary`2.Enumerator.MoveNext()
   at System.Management.Automation.CommandInfo.get_Parameters()
   at Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseCorrectCasing.<AnalyzeScript>d__0.MoveNext() in C:\Users\christoph.bergmeiste\git\psscriptanalyzer\Rules\UseCorrectCasing.cs:line 72

About the other issue that you mentioned: It is not directly related to it because the origin of the other issue is reasonably well understood (Test-ModuleManifest is not thread safe due to PowerShell internals) and also its fix, which was to have a lock around it. Where the issue is similar is that there are elements of the PowerShell engine that are not thread safe. I need to spend more time looking into to see if a) a lock around the call would prevent it from happening and b) attach the debugger to PowerShell itself to see if there is something that can be fixed. cc @rjmholt

Update: The exception inside PowerShell itself is coming from here when calling in in the foreach loop: https://github.com/PowerShell/PowerShell/blob/3de9069ca799fd5f67ef3dc44198a5e9833c2a68/src/System.Management.Automation/engine/CommandInfo.cs#L571
Full stack trace:

   at System.ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion()
   at System.Collections.Generic.Dictionary`2.Enumerator.MoveNext()
   at System.Management.Automation.CommandInfo.get_Parameters() in C:\Users\christoph.bergmeiste\git\powershell\src\System.Management.Automation\engine\CommandInfo.cs:line 571

image

@mrboring
Copy link
Author

mrboring commented Jun 2, 2020

@bergmeister Thanks for the update. As you suggested, I have reverted back to 1.18.3 for now.

@mrboring
Copy link
Author

mrboring commented Jun 3, 2020

@bergmeister Just a quick question. If the issue is related to PowerShell, why does it not affect 1.18.3?

@bergmeister
Copy link
Collaborator

bergmeister commented Jun 3, 2020

@bergmeister Just a quick question. If the issue is related to PowerShell, why does it not affect 1.18.3?

Because PSUseCorrectCasing corrects only cmdlet names in 1.18.3 but in 1.19.0 it also corrects parameter names and the issue happens when trying to do the parameter lookup.
We could possibly try to see if we can also prevent it from happening in PSSA by trying to limit concurrency of access to the shared cmdlet cache but we'd first have to find out if that would work because it might also be that it's a PowerShell bug where there is an invalid reference to the original runspace that created the ComandInfo object. Depending on that we might have to add a configuration option to the rule for being able to turn off parameter correction (possibly even by default) if it turns out it's something we cannot fix in PSSA. Because parts of the PowerShell engine itself weren't entirely designed to be thread safe by design (for fast performance), it's kind of expected to see some hickups coming from PowerShell and where possible, I'd ideally like to fix the root cause. WDYT @rjmholt

@mrboring
Copy link
Author

mrboring commented Jun 3, 2020

@bergmeister Thanks for the info.

@bergmeister
Copy link
Collaborator

bergmeister commented Jun 13, 2020

It seems to be that the following issue in PowerShell is the root cause: PowerShell/PowerShell#4003
Since the issue seems to be hard to resolve in PowerShell, we need to think about what we could do in PSSA:
The first simplest and fastest 'solution' would be to add a configuration option so that one can optionally turn of the parameter correction feature off so that you have a better way of suppressing the error rather than disabling the whole rule or pinning an old version of PSSA.
For performance reasons, I do not want to add a lock around the CommandInfoCache, therefore a more pragmatic solution might be to make this rule not use the object from the cache but query a new object every time. Although this will make it slower, it would not have a knock on effect on other rules and this rule is usually rather used in the editor anyway and not enabled by default.
I will discuss this at the next triage with the PowerShell team.

@mrboring
Copy link
Author

@bergmeister Thanks for the update. Please note that I use PSScriptAnalyzer from both VS Code and the commandline. I like the second option as I would rather have the rule there, but also implementing the first option would allow the disabling of the rule for those concerned about any performance issues.

@bergmeister
Copy link
Collaborator

bergmeister commented Jun 13, 2020

I found a better way of implementing option 2 by specifically catching the exception when it happens and then requesting a fresh, new object that does not come from the cache. Watch PR #1523 for that.
Whilst errors were coming from multiple different sources, I found a more minimalistic repro that reliably reproduces as those cmdlet seem to be frequently the source of the errors:

1..100 | ForEach-Object { $null = Invoke-ScriptAnalyzer -ScriptDefinition @'
Get-Content
Test-Path
Get-ChildItem
Get-Content
Test-Path
Get-ChildItem
'@ -Settings @{ 'Rules' = @{ 'PSUseCorrectCasing' = @{ 'Enable' = $true } } } }

@mrboring
Copy link
Author

Very nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment