Skip to content

Commit 39b5356

Browse files
Do not fail analysis on transient command lookup failures (issue 2205)
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
1 parent c51a892 commit 39b5356

3 files changed

Lines changed: 84 additions & 22 deletions

File tree

‎Engine/CommandInfoCache.cs‎

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
1414
/// </summary>
1515
internal class CommandInfoCache : IDisposable
1616
{
17+
/// <summary>
18+
/// Number of times a command lookup is attempted before giving up.
19+
/// Command lookups can fail transiently because the PowerShell engine is not thread safe,
20+
/// see https://github.com/PowerShell/PowerShell/issues/4003
21+
/// </summary>
22+
private const int MaxLookupAttempts = 3;
23+
1724
private readonly ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>> _commandInfoCache;
1825
private readonly RunspacePool _runspacePool;
1926
private bool disposed = false;
@@ -70,7 +77,19 @@ public CommandInfo GetCommandInfo(string commandName, CommandTypes? commandTypes
7077
return GetCommandInfoInternal(commandName, commandTypes);
7178
}
7279
// 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;
80+
var lazyCommandInfo = _commandInfoCache.GetOrAdd(key, new Lazy<CommandInfo>(() => GetCommandInfoInternal(commandName, commandTypes)));
81+
try
82+
{
83+
return lazyCommandInfo.Value;
84+
}
85+
catch
86+
{
87+
// Lazy<T> caches exceptions forever, which would make every subsequent lookup of this
88+
// command fail for the lifetime of the process. Evict the entry so that the next lookup
89+
// can try again.
90+
_commandInfoCache.TryRemove(key, out _);
91+
throw;
92+
}
7493
}
7594

7695

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

102-
using (var ps = System.Management.Automation.PowerShell.Create())
121+
for (int attempt = 1; ; attempt++)
103122
{
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))
123+
using (var ps = System.Management.Automation.PowerShell.Create())
116124
{
117-
ps.AddParameter("Module", moduleName);
125+
ps.RunspacePool = _runspacePool;
126+
127+
ps.AddCommand("Get-Command")
128+
.AddParameter("Name", actualCmdName)
129+
.AddParameter("ErrorAction", "SilentlyContinue");
130+
131+
if (commandType != null)
132+
{
133+
ps.AddParameter("CommandType", commandType);
134+
}
135+
136+
if (!string.IsNullOrEmpty(moduleName))
137+
{
138+
ps.AddParameter("Module", moduleName);
139+
}
140+
141+
try
142+
{
143+
return ps.Invoke<CommandInfo>()
144+
.FirstOrDefault();
145+
}
146+
// 'Get-Command' is invoked with 'SilentlyContinue', so a CommandNotFoundException can only
147+
// mean that the engine failed to resolve 'Get-Command' itself in the pooled runspace.
148+
// That happens intermittently because the PowerShell engine is not thread safe, see
149+
// https://github.com/PowerShell/PowerShell/issues/4003 and
150+
// https://github.com/PowerShell/PSScriptAnalyzer/issues/2205
151+
// Retrying usually succeeds, but rather than failing the whole analysis when it does not,
152+
// treat the command as unresolvable.
153+
catch (CommandNotFoundException)
154+
{
155+
if (attempt >= MaxLookupAttempts)
156+
{
157+
return null;
158+
}
159+
}
118160
}
119-
120-
return ps.Invoke<CommandInfo>()
121-
.FirstOrDefault();
122161
}
123162
}
124163

‎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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Licensed under the MIT License.
33

44
Describe 'Issue 2205' {
5-
It "reproduces the Linux recursive analysis failure" -Skip:(-not $IsLinux) {
5+
It "does not fail the analysis when a command lookup hits the runspace affinity problem" -Skip:(-not $IsLinux) {
66
$settingsPath = Join-Path $PSScriptRoot 'Issue2205/PSScriptAnalyzerSettings.psd1'
77
$repositoryRoot = Split-Path -Parent (Split-Path -Parent $PSScriptRoot)
88

0 commit comments

Comments
 (0)