Skip to content

Commit 9734b62

Browse files
jessehouwingjessehouwing
authored
Merge pull request #1 from jessehouwing/copilot/create-unit-test-for-issue-2205
Fix intermittent "The term 'Get-Command' is not recognized" failures during recursive analysis
2 parents 4b0117c + bc604c9 commit 9734b62

4 files changed

Lines changed: 151 additions & 21 deletions

File tree

‎Engine/CommandInfoCache.cs‎

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Concurrent;
6+
using System.Collections.Generic;
67
using System.Management.Automation;
78
using System.Linq;
89
using System.Management.Automation.Runspaces;
@@ -14,6 +15,13 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
1415
/// </summary>
1516
internal class CommandInfoCache : IDisposable
1617
{
18+
/// <summary>
19+
/// Number of times a command lookup is attempted before giving up.
20+
/// Command lookups can fail transiently because the PowerShell engine is not thread safe,
21+
/// see https://github.com/PowerShell/PowerShell/issues/4003
22+
/// </summary>
23+
private const int MaxLookupAttempts = 3;
24+
1725
private readonly ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>> _commandInfoCache;
1826
private readonly RunspacePool _runspacePool;
1927
private bool disposed = false;
@@ -70,7 +78,21 @@ public CommandInfo GetCommandInfo(string commandName, CommandTypes? commandTypes
7078
return GetCommandInfoInternal(commandName, commandTypes);
7179
}
7280
// Atomically either use PowerShell to query a command info object, or fetch it from the cache
73-
return _commandInfoCache.GetOrAdd(key, new Lazy<CommandInfo>(() => GetCommandInfoInternal(commandName, commandTypes))).Value;
81+
var lazyCommandInfo = _commandInfoCache.GetOrAdd(key, new Lazy<CommandInfo>(() => GetCommandInfoInternal(commandName, commandTypes)));
82+
try
83+
{
84+
return lazyCommandInfo.Value;
85+
}
86+
catch
87+
{
88+
// Lazy<T> caches exceptions forever, which would make every subsequent lookup of this
89+
// command fail for the lifetime of the process. Evict the entry so that the next lookup
90+
// can try again. Only remove the faulted instance so that a replacement that another
91+
// thread may already have added is left alone.
92+
((ICollection<KeyValuePair<CommandLookupKey, Lazy<CommandInfo>>>)_commandInfoCache)
93+
.Remove(new KeyValuePair<CommandLookupKey, Lazy<CommandInfo>>(key, lazyCommandInfo));
94+
throw;
95+
}
7496
}
7597

7698

@@ -99,26 +121,46 @@ private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? command
99121
// For more details see https://github.com/PowerShell/PowerShell/issues/9308
100122
actualCmdName = WildcardPattern.Escape(actualCmdName);
101123

102-
using (var ps = System.Management.Automation.PowerShell.Create())
124+
for (int attempt = 1; ; attempt++)
103125
{
104-
ps.RunspacePool = _runspacePool;
105-
106-
ps.AddCommand("Get-Command")
107-
.AddParameter("Name", actualCmdName)
108-
.AddParameter("ErrorAction", "SilentlyContinue");
109-
110-
if (commandType != null)
111-
{
112-
ps.AddParameter("CommandType", commandType);
113-
}
114-
115-
if (!string.IsNullOrEmpty(moduleName))
126+
using (var ps = System.Management.Automation.PowerShell.Create())
116127
{
117-
ps.AddParameter("Module", moduleName);
128+
ps.RunspacePool = _runspacePool;
129+
130+
ps.AddCommand("Get-Command")
131+
.AddParameter("Name", actualCmdName)
132+
.AddParameter("ErrorAction", "SilentlyContinue");
133+
134+
if (commandType != null)
135+
{
136+
ps.AddParameter("CommandType", commandType);
137+
}
138+
139+
if (!string.IsNullOrEmpty(moduleName))
140+
{
141+
ps.AddParameter("Module", moduleName);
142+
}
143+
144+
try
145+
{
146+
return ps.Invoke<CommandInfo>()
147+
.FirstOrDefault();
148+
}
149+
// 'Get-Command' is invoked with 'SilentlyContinue', so a CommandNotFoundException can only
150+
// mean that the engine failed to resolve 'Get-Command' itself in the pooled runspace.
151+
// That happens intermittently because the PowerShell engine is not thread safe, see
152+
// https://github.com/PowerShell/PowerShell/issues/4003 and
153+
// https://github.com/PowerShell/PSScriptAnalyzer/issues/2205
154+
// Retrying usually succeeds, but rather than failing the whole analysis when it does not,
155+
// treat the command as unresolvable.
156+
catch (CommandNotFoundException)
157+
{
158+
if (attempt >= MaxLookupAttempts)
159+
{
160+
return null;
161+
}
162+
}
118163
}
119-
120-
return ps.Invoke<CommandInfo>()
121-
.FirstOrDefault();
122164
}
123165
}
124166

‎Rules/UseCorrectCasing.cs‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,17 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
128128
// It's a known issue that objects from PowerShell can have a runspace affinity,
129129
// therefore if that happens, we query a fresh object instead of using the cache.
130130
// https://github.com/PowerShell/PowerShell/issues/4003
131-
catch (InvalidOperationException)
131+
// The affinity problem surfaces as an InvalidOperationException or as a
132+
// NullReferenceException, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1708
133+
catch (Exception exception) when (exception is InvalidOperationException || exception is NullReferenceException)
132134
{
133-
commandInfo = Helper.Instance.GetCommandInfo(commandName, bypassCache: true);
134-
availableParameters = commandInfo.Parameters;
135+
availableParameters = GetParametersFromFreshCommandInfo(commandName);
136+
}
137+
if (availableParameters is null)
138+
{
139+
// The parameters of this command cannot be determined reliably,
140+
// so skip the parameter casing check instead of failing the analysis.
141+
continue;
135142
}
136143
foreach (var commandParameterAst in commandParameterAsts)
137144
{
@@ -161,6 +168,22 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
161168
}
162169
}
163170

171+
/// <summary>
172+
/// Queries a fresh <see cref="CommandInfo"/> object to work around the runspace affinity problem
173+
/// of the PowerShell engine and returns its parameters, or null if they cannot be determined.
174+
/// </summary>
175+
private Dictionary<string, ParameterMetadata> GetParametersFromFreshCommandInfo(string commandName)
176+
{
177+
try
178+
{
179+
return Helper.Instance.GetCommandInfo(commandName, bypassCache: true)?.Parameters;
180+
}
181+
catch (Exception exception) when (exception is InvalidOperationException || exception is NullReferenceException)
182+
{
183+
return null;
184+
}
185+
}
186+
164187
/// <summary>
165188
/// For a command like "gci -path c:", returns the extent of "gci" in the command
166189
/// </summary>

‎Tests/Rules/Issue2205.tests.ps1‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
Describe 'Issue 2205' {
5+
It "does not fail the analysis when a command lookup hits the runspace affinity problem" -Skip:(-not $IsLinux) {
6+
$settingsPath = Join-Path $PSScriptRoot 'Issue2205/PSScriptAnalyzerSettings.psd1'
7+
# $PSScriptRoot is <repo>/Tests/Rules, so two levels up is the repository root.
8+
$repositoryRoot = (Resolve-Path (Join-Path $PSScriptRoot '..' '..')).Path
9+
10+
Invoke-ScriptAnalyzer -Path $repositoryRoot -Recurse -Settings $settingsPath -ErrorAction Stop | Out-Null
11+
}
12+
}

‎Tests/Rules/Issue2205/PSScriptAnalyzerSettings.psd1‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
@{
2+
Severity = @('Error', 'Warning', 'Information')
3+
IncludeRules = @(
4+
'PSAvoidUsingCmdletAliases', 'PSAvoidDefaultValueForMandatoryParameter',
5+
'PSAvoidDefaultValueSwitchParameter', 'PSAvoidGlobalAliases',
6+
'PSAvoidGlobalFunctions', 'PSAvoidGlobalVars', 'PSAvoidInvokingEmptyMembers',
7+
'PSAvoidNullOrEmptyHelpMessageAttribute', 'PSAvoidShouldContinueWithoutForce',
8+
'PSAvoidUsingComputerNameHardcoded', 'PSAvoidUsingConvertToSecureStringWithPlainText',
9+
'PSAvoidUsingDeprecatedManifestFields', 'PSAvoidUsingEmptyCatchBlock',
10+
'PSAvoidUsingInvokeExpression', 'PSAvoidUsingPlainTextForPassword',
11+
'PSAvoidUsingPositionalParameters', 'PSAvoidUsingUsernameAndPasswordParams',
12+
'PSAvoidUsingWMICmdlet', 'PSAvoidUsingWriteHost', 'PSMisleadingBacktick',
13+
'PSMissingModuleManifestField', 'PSPossibleIncorrectComparisonWithNull',
14+
'PSPossibleIncorrectUsageOfAssignmentOperator', 'PSPossibleIncorrectUsageOfRedirectionOperator',
15+
'PSProvideCommentHelp', 'PSReservedCmdletChar', 'PSReservedParams',
16+
'PSUseApprovedVerbs', 'PSUseBOMForUnicodeEncodedFile', 'PSUseCmdletCorrectly',
17+
'PSUseConsistentIndentation', 'PSUseConsistentWhitespace', 'PSUseCorrectCasing',
18+
'PSUseDeclaredVarsMoreThanAssignments', 'PSUseLiteralInitializerForHashtable',
19+
'PSUseOutputTypeCorrectly', 'PSUsePSCredentialType', 'PSUseSingularNouns',
20+
'PSUseToExportFieldsInManifest', 'PSUseUTF8EncodingForHelpFile'
21+
)
22+
ExcludeRules = @(
23+
'PSAvoidUsingWriteHost', 'PSAvoidUsingPositionalParameters', 'PSUseApprovedVerbs',
24+
'PSProvideCommentHelp', 'PSAvoidGlobalVars', 'PSAvoidGlobalFunctions',
25+
'PSUseSingularNouns', 'PSUseOutputTypeCorrectly'
26+
)
27+
Rules = @{
28+
PSUseConsistentIndentation = @{
29+
Enable = $true
30+
IndentationSize = 4
31+
PipelineIndentation = 'IncreaseIndentationForFirstPipeline'
32+
Kind = 'space'
33+
}
34+
PSUseConsistentWhitespace = @{
35+
Enable = $true
36+
CheckInnerBrace = $true
37+
CheckOpenBrace = $true
38+
CheckOpenParen = $true
39+
CheckOperator = $true
40+
CheckPipe = $true
41+
CheckPipeForRedundantWhitespace = $false
42+
CheckSeparator = $true
43+
CheckParameter = $false
44+
IgnoreAssignmentOperatorInsideHashTable = $true
45+
}
46+
PSUseCompatibleCmdlets = @{ Enable = $false }
47+
PSUseCorrectCasing = @{ Enable = $true }
48+
PSAvoidUsingCmdletAliases = @{ Enable = $true; allowlist = @() }
49+
PSAlignAssignmentStatement = @{ Enable = $false; CheckHashtable = $false }
50+
PSPlaceOpenBrace = @{ Enable = $true; OnSameLine = $true; NewLineAfter = $true; IgnoreOneLineBlock = $true }
51+
PSPlaceCloseBrace = @{ Enable = $true; NewLineAfter = $true; IgnoreOneLineBlock = $true; NoEmptyLineBefore = $false }
52+
}
53+
}

0 commit comments

Comments
 (0)