diff --git a/Engine/CommandInfoCache.cs b/Engine/CommandInfoCache.cs index af1f9584f..af76f928e 100644 --- a/Engine/CommandInfoCache.cs +++ b/Engine/CommandInfoCache.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Management.Automation; using System.Linq; using System.Management.Automation.Runspaces; @@ -209,6 +210,35 @@ private static bool IsGetCommandResolutionError(ErrorRecord errorRecord) return IsGetCommandResolutionException(errorRecord?.Exception); } + /// + /// Retrieves parameter metadata without allowing other threads to drive the command's runspace. + /// + public Dictionary GetCommandParameters( + string commandName, CommandTypes? commandTypes = null, bool bypassCache = false) + { + // Resolve the Lazy value before taking the lock: its factory may already be + // running on another thread that needs the same lock to finish the lookup. + var commandInfo = GetCommandInfo(commandName, commandTypes, bypassCache); + lock (_runspaceLock) + { + // Dynamic parameter getters execute PowerShell code and mutate runspace state, + // even though they look like ordinary property reads. + return disposed ? null : commandInfo?.Parameters; + } + } + + /// + /// Retrieves parameter sets under the same lock as command lookups and dynamic parameter queries. + /// + public ReadOnlyCollection GetCommandParameterSets(string commandName) + { + var commandInfo = GetCommandInfo(commandName); + lock (_runspaceLock) + { + return disposed ? null : commandInfo?.ParameterSets; + } + } + private static bool IsGetCommandResolutionException(Exception exception) { if (exception is CommandNotFoundException) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index f36d17433..9bdc6a0c8 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -402,15 +402,15 @@ public HashSet GetExportedFunction(Ast ast) IEnumerable cmdAsts = ast.FindAll(item => item is CommandAst && exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true); - CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet); - - // switch parameters - IEnumerable switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where(pm => pm.SwitchParameter) : Enumerable.Empty(); - - if (exportMM == null) + // Export-ModuleMember has no dynamic parameters. Resolve names from its static + // metadata instead of ResolveParameter(), which re-enters the cached command's + // runspace and races with command lookups and metadata queries on other rule threads. + var parameters = GetCommandParameters("export-modulemember", CommandTypes.Cmdlet); + if (parameters == null) { return exportedFunctions; } + IEnumerable switchParams = parameters.Values.Where(pm => pm.SwitchParameter); foreach (CommandAst cmdAst in cmdAsts) { @@ -429,7 +429,20 @@ public HashSet GetExportedFunction(Ast ast) if (ceAst is CommandParameterAst) { var paramAst = ceAst as CommandParameterAst; - var param = exportMM.ResolveParameter(paramAst.ParameterName); + ParameterMetadata param; + if (!parameters.TryGetValue(paramAst.ParameterName, out param)) + { + param = parameters.Values.FirstOrDefault(pm => + pm.Aliases.Contains(paramAst.ParameterName, StringComparer.OrdinalIgnoreCase)); + if (param == null) + { + var matches = parameters.Values.Where(pm => + pm.Name.StartsWith(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase) + || pm.Aliases.Any(alias => alias.StartsWith(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase))) + .Take(2).ToArray(); + param = matches.Length == 1 ? matches[0] : null; + } + } if (param == null) { @@ -671,6 +684,23 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null, return CommandInfoCache.GetCommandInfo(name, commandTypes: commandType, bypassCache: bypassCache); } + /// + /// Retrieves command parameters while serializing access to the cached command's runspace. + /// + public Dictionary GetCommandParameters( + string name, CommandTypes? commandType = null, bool bypassCache = false) + { + return CommandInfoCache.GetCommandParameters(name, commandType, bypassCache); + } + + /// + /// Retrieves command parameter sets while serializing access to the cached command's runspace. + /// + public ReadOnlyCollection GetCommandParameterSets(string name) + { + return CommandInfoCache.GetCommandParameterSets(name); + } + /// /// Returns the get, set and test targetresource dsc function /// diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index ccec27e0b..89e895ea0 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -148,8 +148,8 @@ private bool MandatoryParameterExists(CommandAst cmdAst) var mandatoryParameters = new List(); try { - int noOfParamSets = cmdInfo.ParameterSets.Count; - foreach (ParameterMetadata pm in cmdInfo.Parameters.Values) + int noOfParamSets = Helper.Instance.GetCommandParameterSets(cmdAst.GetCommandName()).Count; + foreach (ParameterMetadata pm in Helper.Instance.GetCommandParameters(cmdAst.GetCommandName()).Values) { int count = 0; @@ -256,4 +256,3 @@ public string GetSourceName() - diff --git a/Rules/UseCorrectCasing.cs b/Rules/UseCorrectCasing.cs index de9e2acd6..a2ef3ad91 100644 --- a/Rules/UseCorrectCasing.cs +++ b/Rules/UseCorrectCasing.cs @@ -123,7 +123,7 @@ public override IEnumerable AnalyzeScript(Ast ast, string file Dictionary availableParameters; try { - availableParameters = commandInfo.Parameters; + availableParameters = Helper.Instance.GetCommandParameters(commandName); } // It's a known issue that objects from PowerShell can have a runspace affinity, // therefore if that happens, we query a fresh object instead of using the cache. @@ -176,7 +176,7 @@ private Dictionary GetParametersFromFreshCommandInfo( { try { - return Helper.Instance.GetCommandInfo(commandName, bypassCache: true)?.Parameters; + return Helper.Instance.GetCommandParameters(commandName, bypassCache: true); } catch (Exception exception) when (exception is InvalidOperationException || exception is NullReferenceException) { diff --git a/Tests/Engine/CommandInfoCacheConcurrency.tests.ps1 b/Tests/Engine/CommandInfoCacheConcurrency.tests.ps1 index 10f9c1047..c74f41813 100644 --- a/Tests/Engine/CommandInfoCacheConcurrency.tests.ps1 +++ b/Tests/Engine/CommandInfoCacheConcurrency.tests.ps1 @@ -12,12 +12,48 @@ Describe "Concurrent command lookups" { # threads. Invoking a PowerShell script block on a thread pool thread would introduce # runspace affinity problems of its own and would not test the command info cache. $analyzerAssembly = [Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper].Assembly.Location - Add-Type -IgnoreWarnings -WarningAction SilentlyContinue -ReferencedAssemblies $analyzerAssembly, ([System.Management.Automation.PSObject].Assembly.Location) -TypeDefinition @' + $references = @($analyzerAssembly, ([System.Management.Automation.PSObject].Assembly.Location)) + if ($PSVersionTable.PSEdition -eq 'Core') { + $references += Join-Path $PSHOME 'ref/System.Collections.dll' + } + Add-Type -IgnoreWarnings -WarningAction SilentlyContinue -ReferencedAssemblies $references -TypeDefinition @' using System.Threading.Tasks; +using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer; public static class ConcurrentCommandLookup { + public static void ResolveExports() + { + Token[] tokens; + ParseError[] errors; + var ast = Parser.ParseInput("Export-ModuleMember -Function Test-Example", out tokens, out errors); + var helper = Helper.Instance; + var tasks = new Task[8]; + for (int i = 0; i < tasks.Length; i++) + { + tasks[i] = Task.Run(() => + { + for (int j = 0; j < 100; j++) + { + helper.GetCommandInfo("Get-Command", bypassCache: true); + var parameters = helper.GetCommandParameters("Get-Item"); + if (!parameters.ContainsKey("Path") || helper.GetCommandParameterSets("Get-Item").Count == 0) + { + throw new System.InvalidOperationException("Command parameter metadata was not resolved."); + } + var exports = helper.GetExportedFunction(ast); + if (!exports.SetEquals(new[] { "Test-Example" })) + { + throw new System.InvalidOperationException("Exported function was not resolved."); + } + } + }); + } + + Task.WaitAll(tasks); + } + public static string[] Lookup(string[] commandNames) { var helper = Helper.Instance; @@ -60,5 +96,10 @@ public static class ConcurrentCommandLookup for ($i = 0; $i -lt $commandNames.Count; $i++) { $results[$i] | Should -BeExactly $commandNames[$i] } + + } + + It "resolves exported functions while command lookups run concurrently" { + [ConcurrentCommandLookup]::ResolveExports() } } diff --git a/Tests/Engine/Helper.tests.ps1 b/Tests/Engine/Helper.tests.ps1 index 3d53e71f1..a6ccd70b2 100644 --- a/Tests/Engine/Helper.tests.ps1 +++ b/Tests/Engine/Helper.tests.ps1 @@ -39,3 +39,28 @@ Describe "Test Directed Graph" { } } } + +Describe "Exported function parameter resolution" { + BeforeAll { + $null = Invoke-ScriptAnalyzer -ScriptDefinition 'Get-Item -Path .' + } + + It "resolves exports from