From 826ae77aae4e556a407721baec47e943caa15b56 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Wed, 10 Jul 2019 16:10:11 -0700 Subject: [PATCH 01/27] Some refactoring to accomodate parallel processing --- .../engine/InternalCommands.cs | 775 ++++++++++-------- 1 file changed, 446 insertions(+), 329 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index a5ba98e7985..c4cffb0df1c 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -59,11 +59,18 @@ public object GetValue(PSObject inputObject, string propertyName) RemotingCapability = RemotingCapability.None)] public sealed class ForEachObjectCommand : PSCmdlet { + #region Private Members + + private const string ParallelParameterSet = "ParallelParameterSet"; + + #endregion + /// /// This parameter specifies the current pipeline object. /// [Parameter(ValueFromPipeline = true, ParameterSetName = "ScriptBlockSet")] [Parameter(ValueFromPipeline = true, ParameterSetName = "PropertyAndMethodSet")] + [Parameter(ValueFromPipeline = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] public PSObject InputObject { set { _inputObject = value; } @@ -195,6 +202,65 @@ public object[] ArgumentList #endregion PropertyAndMethodSet + #region ParallelParameterSet + + /// + /// Flag to indicate that foreach iterations should be run in parallel instead of sequentially. + /// + [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + public SwitchParameter Parallel + { + get; + set; + } + + /// + /// Script block to run for each pipeline object + /// + [Parameter(Position = 0, Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + public ScriptBlock ScriptBlock + { + get; + set; + } + + /// + /// Specifies the maximum number of concurrently running scriptblocks on separate threads. + /// The default number is 5. + /// + [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + [ValidateRange(1, Int32.MaxValue)] + public int ThrottleLimit + { + get; + set; + } = 5; + + /// + /// Specifies a timeout time in seconds, after which the parallel running scripts will be stopped + /// The default value is 0, indicating no timeout. + /// + [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + [ValidateRange(0, Int32.MaxValue)] + public int TimeoutSeconds + { + get; + set; + } + + /// + /// Flag that returns a job object immediately for the parallel operation, instead of returning after + /// all foreach processing is completed. + /// + [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + public SwitchParameter AsJob + { + get; + set; + } + + #endregion + /// /// Execute the begin scriptblock at the start of processing. /// @@ -203,69 +269,87 @@ public object[] ArgumentList /// See Pipeline.Invoke. protected override void BeginProcessing() { - Dbg.Assert(ParameterSetName == "ScriptBlockSet" || ParameterSetName == "PropertyAndMethodSet", "ParameterSetName is neither 'ScriptBlockSet' nor 'PropertyAndMethodSet'"); + switch (ParameterSetName) + { + case "ScriptBlockSet": + InitScriptBlockParameterSet(); + break; - if (ParameterSetName != "ScriptBlockSet") return; + case ForEachObjectCommand.ParallelParameterSet: + InitParallelParameterSet(); + break; + } + } - // Win8: 176403: ScriptCmdlets sets the global WhatIf and Confirm preferences - // This effects the new W8 foreach-object cmdlet with -whatif and -confirm - // implemented. -whatif and -confirm needed only for PropertyAndMethodSet - // parameter set. So erring out in cases where these are used with ScriptBlockSet. - // Not using MshCommandRuntime, as those variables will be affected by ScriptCmdlet - // infrastructure (wherein ScriptCmdlet modifies the global preferences). - Dictionary psBoundParameters = this.MyInvocation.BoundParameters; - if (psBoundParameters != null) + /// + /// Execute the processing script blocks on the current pipeline object + /// which is passed as it's only parameter. + /// + /// Could not parse script. + /// See Pipeline.Invoke. + /// See Pipeline.Invoke. + protected override void ProcessRecord() + { + switch (ParameterSetName) { - SwitchParameter whatIf = false; - SwitchParameter confirm = false; + case "ScriptBlockSet": + ProcessScriptBlockParameterSet(); + break; - object argument; - if (psBoundParameters.TryGetValue("whatif", out argument)) - { - whatIf = (SwitchParameter)argument; - } + case "PropertyAndMethodSet": + ProcessPropertyAndMethodParameterSet(); + break; - if (psBoundParameters.TryGetValue("confirm", out argument)) - { - confirm = (SwitchParameter)argument; - } + case ForEachObjectCommand.ParallelParameterSet: + ProcessParallelParameterSet(); + break; + } + } - if (whatIf || confirm) - { - string message = InternalCommandStrings.NoShouldProcessForScriptBlockSet; + /// + /// Execute the end scriptblock when the pipeline is complete. + /// + /// Could not parse script. + /// See Pipeline.Invoke. + /// See Pipeline.Invoke. + protected override void EndProcessing() + { + switch (ParameterSetName) + { + case "ScriptBlockSet": + EndBlockParameterSet(); + break; - ErrorRecord errorRecord = new ErrorRecord( - new InvalidOperationException(message), - "NoShouldProcessForScriptBlockSet", - ErrorCategory.InvalidOperation, - null); - ThrowTerminatingError(errorRecord); - } + case ForEachObjectCommand.ParallelParameterSet: + EndParallelParameterSet(); + break; } + } - // Calculate the start and end indexes for the processRecord script blocks - _end = _scripts.Count; - _start = _scripts.Count > 1 ? 1 : 0; + #region Private Methods - // and set the end script if it wasn't explicitly set with a named parameter. - if (!_setEndScript) - { - if (_scripts.Count > 2) - { - _end = _scripts.Count - 1; - _endScript = _scripts[_end]; - } - } + private void InitParallelParameterSet() + { + // TODO: + } - // only process the start script if there is more than one script... - if (_end < 2) - return; + private void ProcessParallelParameterSet() + { + // TODO: + } - if (_scripts[0] == null) + private void EndParallelParameterSet() + { + // TODO: + } + + private void EndBlockParameterSet() + { + if (_endScript == null) return; var emptyArray = Array.Empty(); - _scripts[0].InvokeUsingCmdlet( + _endScript.InvokeUsingCmdlet( contextCmdlet: this, useLocalScope: false, errorHandlingBehavior: ScriptBlock.ErrorHandlingBehavior.WriteToCurrentErrorPipe, @@ -275,164 +359,204 @@ protected override void BeginProcessing() args: emptyArray); } - /// - /// Execute the processing script blocks on the current pipeline object - /// which is passed as it's only parameter. - /// - /// Could not parse script. - /// See Pipeline.Invoke. - /// See Pipeline.Invoke. - protected override void ProcessRecord() + private void ProcessPropertyAndMethodParameterSet() { - Dbg.Assert(ParameterSetName == "ScriptBlockSet" || ParameterSetName == "PropertyAndMethodSet", "ParameterSetName is neither 'ScriptBlockSet' nor 'PropertyAndMethodSet'"); + _targetString = string.Format(CultureInfo.InvariantCulture, InternalCommandStrings.ForEachObjectTarget, GetStringRepresentation(InputObject)); - switch (ParameterSetName) + if (LanguagePrimitives.IsNull(InputObject)) { - case "ScriptBlockSet": - for (int i = _start; i < _end; i++) + if (_arguments != null && _arguments.Length > 0) + { + WriteError(GenerateNameParameterError("InputObject", ParserStrings.InvokeMethodOnNull, + "InvokeMethodOnNull", _inputObject)); + } + else + { + // should process + string propertyAction = string.Format(CultureInfo.InvariantCulture, + InternalCommandStrings.ForEachObjectPropertyAction, _propertyOrMethodName); + + if (ShouldProcess(_targetString, propertyAction)) { - // Only execute scripts that aren't null. This isn't treated as an error - // because it allows you to parameterize a command - for example you might allow - // for actions before and after the main processing script. They could be null - // by default and therefore ignored then filled in later... - if (_scripts[i] != null) + if (Context.IsStrictVersion(2)) { - _scripts[i].InvokeUsingCmdlet( - contextCmdlet: this, - useLocalScope: false, - errorHandlingBehavior: ScriptBlock.ErrorHandlingBehavior.WriteToCurrentErrorPipe, - dollarUnder: InputObject, - input: new object[] { InputObject }, - scriptThis: AutomationNull.Value, - args: Array.Empty()); + WriteError(GenerateNameParameterError("InputObject", InternalCommandStrings.InputObjectIsNull, + "InputObjectIsNull", _inputObject)); + } + else + { + // we write null out because: + // PS C:\> $null | ForEach-object {$_.aa} | ForEach-Object {$_ + 3} + // 3 + // so we also want + // PS C:\> $null | ForEach-object aa | ForEach-Object {$_ + 3} + // 3 + // But if we don't write anything to the pipeline when _inputObject is null, + // the result 3 will not be generated. + WriteObject(null); } } + } - break; - case "PropertyAndMethodSet": + return; + } + + ErrorRecord errorRecord = null; - _targetString = string.Format(CultureInfo.InvariantCulture, InternalCommandStrings.ForEachObjectTarget, GetStringRepresentation(InputObject)); + // if args exist, this is explicitly a method invocation + if (_arguments != null && _arguments.Length > 0) + { + MethodCallWithArguments(); + } + // no arg provided + else + { + // if inputObject is of IDictionary, get the value + if (GetValueFromIDictionaryInput()) { return; } - if (LanguagePrimitives.IsNull(InputObject)) + PSMemberInfo member = null; + if (WildcardPattern.ContainsWildcardCharacters(_propertyOrMethodName)) + { + // get the matched member(s) + ReadOnlyPSMemberInfoCollection members = + _inputObject.Members.Match(_propertyOrMethodName, PSMemberTypes.All); + Dbg.Assert(members != null, "The return value of Members.Match should never be null"); + + if (members.Count > 1) { - if (_arguments != null && _arguments.Length > 0) - { - WriteError(GenerateNameParameterError("InputObject", ParserStrings.InvokeMethodOnNull, - "InvokeMethodOnNull", _inputObject)); - } - else + // write error record: property method ambiguous + StringBuilder possibleMatches = new StringBuilder(); + foreach (PSMemberInfo item in members) { - // should process - string propertyAction = string.Format(CultureInfo.InvariantCulture, - InternalCommandStrings.ForEachObjectPropertyAction, _propertyOrMethodName); - - if (ShouldProcess(_targetString, propertyAction)) - { - if (Context.IsStrictVersion(2)) - { - WriteError(GenerateNameParameterError("InputObject", InternalCommandStrings.InputObjectIsNull, - "InputObjectIsNull", _inputObject)); - } - else - { - // we write null out because: - // PS C:\> $null | ForEach-object {$_.aa} | ForEach-Object {$_ + 3} - // 3 - // so we also want - // PS C:\> $null | ForEach-object aa | ForEach-Object {$_ + 3} - // 3 - // But if we don't write anything to the pipeline when _inputObject is null, - // the result 3 will not be generated. - WriteObject(null); - } - } + possibleMatches.AppendFormat(CultureInfo.InvariantCulture, " {0}", item.Name); } + WriteError(GenerateNameParameterError("Name", InternalCommandStrings.AmbiguousPropertyOrMethodName, + "AmbiguousPropertyOrMethodName", _inputObject, + _propertyOrMethodName, possibleMatches)); return; } - ErrorRecord errorRecord = null; - - // if args exist, this is explicitly a method invocation - if (_arguments != null && _arguments.Length > 0) + if (members.Count == 1) { - MethodCallWithArguments(); + member = members[0]; } - // no arg provided - else + } + else + { + member = _inputObject.Members[_propertyOrMethodName]; + } + + // member is a method + if (member is PSMethodInfo) + { + // first we check if the member is a ParameterizedProperty + PSParameterizedProperty targetParameterizedProperty = member as PSParameterizedProperty; + if (targetParameterizedProperty != null) { - // if inputObject is of IDictionary, get the value - if (GetValueFromIDictionaryInput()) { return; } + // should process + string propertyAction = string.Format(CultureInfo.InvariantCulture, + InternalCommandStrings.ForEachObjectPropertyAction, targetParameterizedProperty.Name); - PSMemberInfo member = null; - if (WildcardPattern.ContainsWildcardCharacters(_propertyOrMethodName)) + // ParameterizedProperty always take parameters, so we output the member.Value directly + if (ShouldProcess(_targetString, propertyAction)) { - // get the matched member(s) - ReadOnlyPSMemberInfoCollection members = - _inputObject.Members.Match(_propertyOrMethodName, PSMemberTypes.All); - Dbg.Assert(members != null, "The return value of Members.Match should never be null"); + WriteObject(member.Value); + } - if (members.Count > 1) - { - // write error record: property method ambiguous - StringBuilder possibleMatches = new StringBuilder(); - foreach (PSMemberInfo item in members) - { - possibleMatches.AppendFormat(CultureInfo.InvariantCulture, " {0}", item.Name); - } + return; + } - WriteError(GenerateNameParameterError("Name", InternalCommandStrings.AmbiguousPropertyOrMethodName, - "AmbiguousPropertyOrMethodName", _inputObject, - _propertyOrMethodName, possibleMatches)); - return; - } + PSMethodInfo targetMethod = member as PSMethodInfo; + Dbg.Assert(targetMethod != null, "targetMethod should not be null here."); + try + { + // should process + string methodAction = string.Format(CultureInfo.InvariantCulture, + InternalCommandStrings.ForEachObjectMethodActionWithoutArguments, targetMethod.Name); - if (members.Count == 1) + if (ShouldProcess(_targetString, methodAction)) + { + if (!BlockMethodInLanguageMode(InputObject)) { - member = members[0]; + object result = targetMethod.Invoke(Array.Empty()); + WriteToPipelineWithUnrolling(result); } } + } + catch (PipelineStoppedException) + { + // PipelineStoppedException can be caused by select-object + throw; + } + catch (Exception ex) + { + MethodException mex = ex as MethodException; + if (mex != null && mex.ErrorRecord != null && mex.ErrorRecord.FullyQualifiedErrorId == "MethodCountCouldNotFindBest") + { + WriteObject(targetMethod.Value); + } else { - member = _inputObject.Members[_propertyOrMethodName]; + WriteError(new ErrorRecord(ex, "MethodInvocationError", ErrorCategory.InvalidOperation, _inputObject)); } - - // member is a method - if (member is PSMethodInfo) + } + } + else + { + string resolvedPropertyName = null; + bool isBlindDynamicAccess = false; + if (member == null) + { + if ((_inputObject.BaseObject is IDynamicMetaObjectProvider) && + !WildcardPattern.ContainsWildcardCharacters(_propertyOrMethodName)) { - // first we check if the member is a ParameterizedProperty - PSParameterizedProperty targetParameterizedProperty = member as PSParameterizedProperty; - if (targetParameterizedProperty != null) - { - // should process - string propertyAction = string.Format(CultureInfo.InvariantCulture, - InternalCommandStrings.ForEachObjectPropertyAction, targetParameterizedProperty.Name); - - // ParameterizedProperty always take parameters, so we output the member.Value directly - if (ShouldProcess(_targetString, propertyAction)) - { - WriteObject(member.Value); - } + // Let's just try a dynamic property access. Note that if it + // comes to depending on dynamic access, we are assuming it is a + // property; we don't have ETS info to tell us up front if it + // even exists or not, let alone if it is a method or something + // else. + // + // Note that this is "truly blind"--the name did not show up in + // GetDynamicMemberNames(), else it would show up as a dynamic + // member. + + resolvedPropertyName = _propertyOrMethodName; + isBlindDynamicAccess = true; + } + else + { + errorRecord = GenerateNameParameterError("Name", InternalCommandStrings.PropertyOrMethodNotFound, + "PropertyOrMethodNotFound", _inputObject, + _propertyOrMethodName); + } + } + else + { + // member is [presumably] a property (note that it could be a + // dynamic property, if it shows up in GetDynamicMemberNames()) + resolvedPropertyName = member.Name; + } - return; - } + if (!string.IsNullOrEmpty(resolvedPropertyName)) + { + // should process + string propertyAction = string.Format(CultureInfo.InvariantCulture, + InternalCommandStrings.ForEachObjectPropertyAction, resolvedPropertyName); - PSMethodInfo targetMethod = member as PSMethodInfo; - Dbg.Assert(targetMethod != null, "targetMethod should not be null here."); + if (ShouldProcess(_targetString, propertyAction)) + { try { - // should process - string methodAction = string.Format(CultureInfo.InvariantCulture, - InternalCommandStrings.ForEachObjectMethodActionWithoutArguments, targetMethod.Name); - - if (ShouldProcess(_targetString, methodAction)) - { - if (!BlockMethodInLanguageMode(InputObject)) - { - object result = targetMethod.Invoke(Array.Empty()); - WriteToPipelineWithUnrolling(result); - } - } + WriteToPipelineWithUnrolling(_propGetter.GetValue(InputObject, resolvedPropertyName)); + } + catch (TerminateException) // The debugger is terminating the execution + { + throw; + } + catch (MethodException) + { + throw; } catch (PipelineStoppedException) { @@ -441,157 +565,172 @@ protected override void ProcessRecord() } catch (Exception ex) { - MethodException mex = ex as MethodException; - if (mex != null && mex.ErrorRecord != null && mex.ErrorRecord.FullyQualifiedErrorId == "MethodCountCouldNotFindBest") + // For normal property accesses, we do not generate an error + // here. The problem for truly blind dynamic accesses (the + // member did not show up in GetDynamicMemberNames) is that + // we can't tell the difference between "it failed because + // the property does not exist" (let's call this case 1) and + // "it failed because accessing it actually threw some + // exception" (let's call that case 2). + // + // PowerShell behavior for normal (non-dynamic) properties + // is different for these two cases: case 1 gets an error + // (which is possible because the ETS tells us up front if + // the property exists or not), and case 2 does not. (For + // normal properties, this catch block /is/ case 2.) + // + // For IDMOPs, we have the chance to attempt a "blind" + // access, but the cost is that we must have the same + // response to both cases (because we cannot distinguish + // between the two). So we have to make a choice: we can + // either swallow ALL errors (including "The property + // 'Blarg' does not exist"), or expose them all. + // + // Here, for truly blind dynamic access, we choose to + // preserve the behavior of showing "The property 'Blarg' + // does not exist" (case 1) errors than to suppress + // "FooException thrown when accessing Bloop property" (case + // 2) errors. + + if (isBlindDynamicAccess) { - WriteObject(targetMethod.Value); + errorRecord = new ErrorRecord(ex, + "DynamicPropertyAccessFailed_" + _propertyOrMethodName, + ErrorCategory.InvalidOperation, + InputObject); } else { - WriteError(new ErrorRecord(ex, "MethodInvocationError", ErrorCategory.InvalidOperation, _inputObject)); + // When the property is not gettable or it throws an exception. + // e.g. when trying to access an assembly's location property, since dynamic assemblies are not backed up by a file, + // an exception will be thrown when accessing its location property. In this case, return null. + WriteObject(null); } } } - else - { - string resolvedPropertyName = null; - bool isBlindDynamicAccess = false; - if (member == null) - { - if ((_inputObject.BaseObject is IDynamicMetaObjectProvider) && - !WildcardPattern.ContainsWildcardCharacters(_propertyOrMethodName)) - { - // Let's just try a dynamic property access. Note that if it - // comes to depending on dynamic access, we are assuming it is a - // property; we don't have ETS info to tell us up front if it - // even exists or not, let alone if it is a method or something - // else. - // - // Note that this is "truly blind"--the name did not show up in - // GetDynamicMemberNames(), else it would show up as a dynamic - // member. - - resolvedPropertyName = _propertyOrMethodName; - isBlindDynamicAccess = true; - } - else - { - errorRecord = GenerateNameParameterError("Name", InternalCommandStrings.PropertyOrMethodNotFound, - "PropertyOrMethodNotFound", _inputObject, - _propertyOrMethodName); - } - } - else - { - // member is [presumably] a property (note that it could be a - // dynamic property, if it shows up in GetDynamicMemberNames()) - resolvedPropertyName = member.Name; - } + } + } + } - if (!string.IsNullOrEmpty(resolvedPropertyName)) - { - // should process - string propertyAction = string.Format(CultureInfo.InvariantCulture, - InternalCommandStrings.ForEachObjectPropertyAction, resolvedPropertyName); + if (errorRecord != null) + { + string propertyAction = string.Format(CultureInfo.InvariantCulture, + InternalCommandStrings.ForEachObjectPropertyAction, _propertyOrMethodName); - if (ShouldProcess(_targetString, propertyAction)) - { - try - { - WriteToPipelineWithUnrolling(_propGetter.GetValue(InputObject, resolvedPropertyName)); - } - catch (TerminateException) // The debugger is terminating the execution - { - throw; - } - catch (MethodException) - { - throw; - } - catch (PipelineStoppedException) - { - // PipelineStoppedException can be caused by select-object - throw; - } - catch (Exception ex) - { - // For normal property accesses, we do not generate an error - // here. The problem for truly blind dynamic accesses (the - // member did not show up in GetDynamicMemberNames) is that - // we can't tell the difference between "it failed because - // the property does not exist" (let's call this case 1) and - // "it failed because accessing it actually threw some - // exception" (let's call that case 2). - // - // PowerShell behavior for normal (non-dynamic) properties - // is different for these two cases: case 1 gets an error - // (which is possible because the ETS tells us up front if - // the property exists or not), and case 2 does not. (For - // normal properties, this catch block /is/ case 2.) - // - // For IDMOPs, we have the chance to attempt a "blind" - // access, but the cost is that we must have the same - // response to both cases (because we cannot distinguish - // between the two). So we have to make a choice: we can - // either swallow ALL errors (including "The property - // 'Blarg' does not exist"), or expose them all. - // - // Here, for truly blind dynamic access, we choose to - // preserve the behavior of showing "The property 'Blarg' - // does not exist" (case 1) errors than to suppress - // "FooException thrown when accessing Bloop property" (case - // 2) errors. - - if (isBlindDynamicAccess) - { - errorRecord = new ErrorRecord(ex, - "DynamicPropertyAccessFailed_" + _propertyOrMethodName, - ErrorCategory.InvalidOperation, - InputObject); - } - else - { - // When the property is not gettable or it throws an exception. - // e.g. when trying to access an assembly's location property, since dynamic assemblies are not backed up by a file, - // an exception will be thrown when accessing its location property. In this case, return null. - WriteObject(null); - } - } - } - } - } + if (ShouldProcess(_targetString, propertyAction)) + { + if (Context.IsStrictVersion(2)) + { + WriteError(errorRecord); } - - if (errorRecord != null) + else { - string propertyAction = string.Format(CultureInfo.InvariantCulture, - InternalCommandStrings.ForEachObjectPropertyAction, _propertyOrMethodName); - - if (ShouldProcess(_targetString, propertyAction)) - { - if (Context.IsStrictVersion(2)) - { - WriteError(errorRecord); - } - else - { - // we write null out because: - // PS C:\> "string" | ForEach-Object {$_.aa} | ForEach-Object {$_ + 3} - // 3 - // so we also want - // PS C:\> "string" | ForEach-Object aa | ForEach-Object {$_ + 3} - // 3 - // But if we don't write anything to the pipeline when no member is found, - // the result 3 will not be generated. - WriteObject(null); - } - } + // we write null out because: + // PS C:\> "string" | ForEach-Object {$_.aa} | ForEach-Object {$_ + 3} + // 3 + // so we also want + // PS C:\> "string" | ForEach-Object aa | ForEach-Object {$_ + 3} + // 3 + // But if we don't write anything to the pipeline when no member is found, + // the result 3 will not be generated. + WriteObject(null); } + } + } + } - break; + private void ProcessScriptBlockParameterSet() + { + for (int i = _start; i < _end; i++) + { + // Only execute scripts that aren't null. This isn't treated as an error + // because it allows you to parameterize a command - for example you might allow + // for actions before and after the main processing script. They could be null + // by default and therefore ignored then filled in later... + if (_scripts[i] != null) + { + _scripts[i].InvokeUsingCmdlet( + contextCmdlet: this, + useLocalScope: false, + errorHandlingBehavior: ScriptBlock.ErrorHandlingBehavior.WriteToCurrentErrorPipe, + dollarUnder: InputObject, + input: new object[] { InputObject }, + scriptThis: AutomationNull.Value, + args: Array.Empty()); + } } } + private void InitScriptBlockParameterSet() + { + // Win8: 176403: ScriptCmdlets sets the global WhatIf and Confirm preferences + // This effects the new W8 foreach-object cmdlet with -whatif and -confirm + // implemented. -whatif and -confirm needed only for PropertyAndMethodSet + // parameter set. So erring out in cases where these are used with ScriptBlockSet. + // Not using MshCommandRuntime, as those variables will be affected by ScriptCmdlet + // infrastructure (wherein ScriptCmdlet modifies the global preferences). + Dictionary psBoundParameters = this.MyInvocation.BoundParameters; + if (psBoundParameters != null) + { + SwitchParameter whatIf = false; + SwitchParameter confirm = false; + + object argument; + if (psBoundParameters.TryGetValue("whatif", out argument)) + { + whatIf = (SwitchParameter)argument; + } + + if (psBoundParameters.TryGetValue("confirm", out argument)) + { + confirm = (SwitchParameter)argument; + } + + if (whatIf || confirm) + { + string message = InternalCommandStrings.NoShouldProcessForScriptBlockSet; + + ErrorRecord errorRecord = new ErrorRecord( + new InvalidOperationException(message), + "NoShouldProcessForScriptBlockSet", + ErrorCategory.InvalidOperation, + null); + ThrowTerminatingError(errorRecord); + } + } + + // Calculate the start and end indexes for the processRecord script blocks + _end = _scripts.Count; + _start = _scripts.Count > 1 ? 1 : 0; + + // and set the end script if it wasn't explicitly set with a named parameter. + if (!_setEndScript) + { + if (_scripts.Count > 2) + { + _end = _scripts.Count - 1; + _endScript = _scripts[_end]; + } + } + + // only process the start script if there is more than one script... + if (_end < 2) + return; + + if (_scripts[0] == null) + return; + + var emptyArray = Array.Empty(); + _scripts[0].InvokeUsingCmdlet( + contextCmdlet: this, + useLocalScope: false, + errorHandlingBehavior: ScriptBlock.ErrorHandlingBehavior.WriteToCurrentErrorPipe, + dollarUnder: AutomationNull.Value, + input: emptyArray, + scriptThis: AutomationNull.Value, + args: emptyArray); + } + /// /// Do method invocation with arguments. /// @@ -795,6 +934,8 @@ private bool BlockMethodInLanguageMode(Object inputObject) return false; } + #endregion + /// /// Generate the appropriate error record. /// @@ -830,30 +971,6 @@ internal static ErrorRecord GenerateNameParameterError(string paraName, string r return errorRecord; } - - /// - /// Execute the end scriptblock when the pipeline is complete. - /// - /// Could not parse script. - /// See Pipeline.Invoke. - /// See Pipeline.Invoke. - protected override void EndProcessing() - { - if (ParameterSetName != "ScriptBlockSet") return; - - if (_endScript == null) - return; - - var emptyArray = Array.Empty(); - _endScript.InvokeUsingCmdlet( - contextCmdlet: this, - useLocalScope: false, - errorHandlingBehavior: ScriptBlock.ErrorHandlingBehavior.WriteToCurrentErrorPipe, - dollarUnder: AutomationNull.Value, - input: emptyArray, - scriptThis: AutomationNull.Value, - args: emptyArray); - } } /// From 8bd9fd5e4f4e34b922e273fc4043dee7f4f7633f Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Fri, 12 Jul 2019 12:55:44 -0700 Subject: [PATCH 02/27] Incremental work 1 --- .../engine/InternalCommands.cs | 43 ++ .../engine/hostifaces/PSTask.cs | 479 ++++++++++++++++++ 2 files changed, 522 insertions(+) create mode 100644 src/System.Management.Automation/engine/hostifaces/PSTask.cs diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index c4cffb0df1c..2382cac001d 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -218,6 +218,7 @@ public SwitchParameter Parallel /// Script block to run for each pipeline object /// [Parameter(Position = 0, Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + [ValidateNotNull()] public ScriptBlock ScriptBlock { get; @@ -261,6 +262,8 @@ public SwitchParameter AsJob #endregion + #region Overrides + /// /// Execute the begin scriptblock at the start of processing. /// @@ -326,21 +329,61 @@ protected override void EndProcessing() } } + /// + /// Handle pipeline stop signal + /// + protected override void StopProcessing() + { + switch (ParameterSetName) + { + case ForEachObjectCommand.ParallelParameterSet: + StopParallelProcessing(); + break; + } + } + + #endregion + #region Private Methods private void InitParallelParameterSet() { // TODO: + // Initialize PSTaskPool + // Process script block Ast using variable expressions + // Create usingValuesMap + // or should variables be passed in as ISS entries or usingValueMap arguments? } private void ProcessParallelParameterSet() { // TODO: + // Create PSTask for each pipeline input + // Hook up data streams object collect/writer and host (pass in cmdlet based object stream writer) + // Need to figure out how to pass in dollarUnderbar variable + // Currently no way to pass in via PowerShell object, need to find a way to do this + // PowerShell -> LocalPipe -> Pipeline processing -> Scriptblock processing -> Invoke scriptblock, pass in dollarUnderbar (ScriptCommandProcessor.cs) + // Create usingValuesMap parameter as needed + // + // Sync: Hand each pipeline input to PSTaskPool, block as necessary + // Catch any exception and Write as ErrorRecord object to error data stream + // Async: Create PSThreadChildJob wrapper for and add parent job, and to job queue } private void EndParallelParameterSet() { // TODO: + // Sync: Make sure all stream objects are written (see InvokeCommandCommand) + // Async: Write parent job object + } + + private void StopParallelProcessing() + { + // TODO: + // Sync: Stop processing input (PSTaskPool accepts no more input) + // Send stop to all running PSTasks in PSTaskPool + // Wait for all tasks to stop + // Async: NoOp } private void EndBlockParameterSet() diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs new file mode 100644 index 00000000000..06a95123656 --- /dev/null +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -0,0 +1,479 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Management.Automation; +using System.Management.Automation.Host; +using System.Management.Automation.Remoting.Internal; +using System.Management.Automation.Runspaces; +using System.Management.Automation.Security; +using System.Threading; + +using Dbg = System.Management.Automation.Diagnostics; + +namespace System.Management.Automation.PSTasks +{ + #region PSTask + + /// + /// Class to encapsulate running a PowerShell script concurrently in a cmdlet context + /// + internal sealed class PSTask : IDisposable + { + #region Members + + private readonly ScriptBlock _scriptBlockToRun; + private readonly Dictionary _usingValuesMap; + private readonly object _dollarUnderbar; + private readonly PSTaskDataStreamWriter _dataStreamWriter; + private readonly Job2 _job; + private readonly PSCmdlet _cmdlet; + + private Runspace _runspace; + private PowerShell _powershell; + private PSDataCollection _output; + + private const string VERBATIM_ARGUMENT = "--%"; + + #endregion + + #region Events + + /// + /// Event that fires when the task running state changes + /// + public event EventHandler StateChanged; + + #endregion + + #region Properties + + PSInvocationState State + { + get + { + PowerShell ps = _powershell; + if (ps != null) + { + return ps.InvocationStateInfo.State; + } + + return PSInvocationState.NotStarted; + } + } + + #endregion + + #region Constructor + + private PSTask() { } + + /// + /// Constructor for data streaming + /// + public PSTask( + ScriptBlock scriptBlock, + Dictionary usingValuesMap, + object dollarUnderbar, + PSTaskDataStreamWriter dataStreamWriter, + PSCmdlet psCmdlet) + { + _scriptBlockToRun = scriptBlock; + _usingValuesMap = usingValuesMap; + _dollarUnderbar = dollarUnderbar; + _dataStreamWriter = dataStreamWriter; + _cmdlet = psCmdlet; + } + + /// + /// Constructor for jobs + /// + public PSTask( + ScriptBlock scriptBlock, + Dictionary usingValuesMap, + object dollarUnderbar, + Job2 job, + PSCmdlet psCmdlet) + { + _scriptBlockToRun = scriptBlock; + _usingValuesMap = usingValuesMap; + _dollarUnderbar = dollarUnderbar; + _job = job; + _cmdlet = psCmdlet; + } + + #endregion + + #region IDisposable + + public void Dispose() + { + _runspace.Dispose(); + _powershell.Dispose(); + _output.Dispose(); + } + + #endregion + + #region Public Methods + + /// + /// Start task + /// + public void Start() + { + if (_powershell != null) + { + // TODO: Localize message + throw new PSInvalidOperationException("This task has already been started. A task can be started only once."); + } + + // Create and open Runspace for this task to run in + var iss = InitialSessionState.CreateDefault2(); + iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) + ? PSLanguageMode.ConstrainedLanguage : PSLanguageMode.FullLanguage; + // TODO: Fix dollarUnderbar variable so that it gets passed correctly to the script block + // It needs to be passed to a compiled script block's localsTuple (will likely need ps.Invoke override): + // scriptBlock.Compile(); + // _localsTuple = scriptBlock.MakeLocalsTuple(_runOptimizedCode); + // _localsTuple.SetAutomaticVariable(AutomaticVariable.Underbar, dollarUnderbar, _context); + // See: ScriptCommandProcessor.cs + iss.Variables.Add( + new SessionStateVariableEntry("DollarUnderbar", _dollarUnderbar, string.Empty) + ); + _runspace = RunspaceFactory.CreateRunspace(_cmdlet.Host, iss); + _runspace.Open(); + + // Create the PowerShell command pipeline for the provided script block + // The script will run on the provided Runspace in a new thread by default + _powershell = PowerShell.Create(); + _powershell.Runspace = _runspace; + + // Initialize PowerShell object data streams and event handlers + _output = new PSDataCollection(); + if (_dataStreamWriter != null) + { + InitializePowerShellforDataStreaming(); + } + else + { + InitializePowerShellforJobs(); + } + + // State change handler + _powershell.InvocationStateChanged += new EventHandler(HandleStateChanged); + + // Start the script running in a new thread + _powershell.BeginInvoke(null, _output); + } + + /// + /// Stop running task + /// + public void Stop() + { + _powershell.Stop(); + } + + #endregion + + #region Private Methods + + private void InitializePowerShellforDataStreaming() + { + Dbg.Assert(_dataStreamWriter != null, "Data stream writer cannot be null"); + + // Writer data stream handlers + _output.DataAdded += (sender, args) => + { + foreach (var item in _output.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Output, item) + ); + } + }; + + _powershell.Streams.Error.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Error.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Error, item) + ); + } + }; + + _powershell.Streams.Warning.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Warning.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.WarningRecord, item) + ); + } + }; + + _powershell.Streams.Verbose.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Verbose.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Verbose, item.Message) + ); + } + }; + + _powershell.Streams.Debug.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Debug.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Debug, item.Message) + ); + } + }; + + _powershell.Streams.Information.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Debug.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Information, item) + ); + } + }; + } + + private void InitializePowerShellforJobs() + { + Dbg.Assert(_job != null, "Data stream writer cannot be null"); + + // Job data stream handlers + _output.DataAdded += (sender, args) => + { + foreach (var item in _output.ReadAll()) + { + _job.Output.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Output, item) + ); + } + }; + + _powershell.Streams.Error.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Error.ReadAll()) + { + _job.Error.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Error, item) + ); + } + }; + + _powershell.Streams.Warning.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Warning.ReadAll()) + { + _job.Warning.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.WarningRecord, item) + ); + } + }; + + _powershell.Streams.Verbose.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Verbose.ReadAll()) + { + _job.Verbose.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Verbose, item.Message) + ); + } + }; + + _powershell.Streams.Debug.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Debug.ReadAll()) + { + _job.Debug.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Debug, item.Message) + ); + } + }; + + _powershell.Streams.Information.DataAdded += (sender, args) => + { + foreach (var item in _powershell.Streams.Information.ReadAll()) + { + _job.Information.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Information, item) + ); + } + }; + } + + private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) + { + // Treat any terminating exception as a non-terminating error record + var newStateInfo = stateChangeInfo.InvocationStateInfo; + if (newStateInfo.Reason != null) + { + var errorRecord = new ErrorRecord( + newStateInfo.Reason, + "PSTaskException", + ErrorCategory.InvalidOperation, + this); + + if (_dataStreamWriter != null) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Error, errorRecord) + ); + } + else + { + // TODO: Jobs + } + } + + StateChanged.Invoke(this, stateChangeInfo); + } + + #endregion + } + + #endregion + + #region PSTaskDataStreamWriter + + /// + /// Class that handles writing data stream objects to a cmdlet + /// + internal sealed class PSTaskDataStreamWriter : IDisposable + { + #region Members + + private readonly PSCmdlet _cmdlet; + private readonly PSDataCollection _dataStream; + private readonly int _cmdletThreadId; + + #endregion + + #region Constructor + + private PSTaskDataStreamWriter() { } + + /// + /// Constructor + /// + public PSTaskDataStreamWriter(PSCmdlet psCmdlet) + { + _cmdlet = psCmdlet; + _cmdletThreadId = Thread.CurrentThread.ManagedThreadId; + _dataStream = new PSDataCollection(); + } + + #endregion + + #region Public Methods + + /// + /// Add data stream object to the writer + /// + public void Add(PSStreamObject streamObject) + { + _dataStream.Add(streamObject); + } + + /// + /// Write all objects in data stream collection to the cmdlet data stream + /// + public void WriteImmediate() + { + CheckCmdletThread(); + + foreach (var item in _dataStream.ReadAll()) + { + item.WriteStreamObject(_cmdlet, true); + } + } + + /// + /// Waits for data stream objects to be added to the collection, and writes them + /// to the cmdlet data stream. + /// + public void WaitAndWrite() + { + CheckCmdletThread(); + + while (true) + { + _dataStream.WaitHandle.WaitOne(); + + if (!_dataStream.IsOpen) + { + break; + } + + // Data ready to write + WriteImmediate(); + } + } + + /// + /// Closes the stream writer + /// + public void Close() + { + CheckCmdletThread(); + + _dataStream.Complete(); + } + + #endregion + + #region Private Methods + + private void CheckCmdletThread() + { + if (Thread.CurrentThread.ManagedThreadId != _cmdletThreadId) + { + // TODO: Localize message + throw new PSInvalidOperationException("The method cannot be called from this thread. This method can only be called from the same cmdlet thread that created this object instance."); + } + } + + #endregion + + #region IDisposable + + /// + /// Disposes the stream writer + /// + public void Dispose() + { + _dataStream.Dispose(); + } + + #endregion + } + + #endregion + + #region PSTaskPool + + internal sealed class PSTaskPool + { + // TODO: + } + + #endregion +} From 88719f3bbfa7c4372518109c4de16e0d60070284 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 15 Jul 2019 15:40:20 -0700 Subject: [PATCH 03/27] Work in progress --- .../engine/InternalCommands.cs | 89 ++++-- .../engine/hostifaces/PSTask.cs | 288 ++++++++++++++++-- 2 files changed, 320 insertions(+), 57 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index 2382cac001d..bf0533f5a4f 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -6,10 +6,12 @@ using System.Collections.Generic; using System.Dynamic; using System.Globalization; +using System.Linq; using System.Linq.Expressions; using System.Management.Automation; using System.Management.Automation.Internal; using System.Management.Automation.Language; +using System.Management.Automation.PSTasks; using System.Runtime.CompilerServices; using System.Text; using Dbg = System.Management.Automation.Diagnostics; @@ -346,44 +348,83 @@ protected override void StopProcessing() #region Private Methods + private PSTaskPool _taskPool; + private PSTaskDataStreamWriter _taskDataStreamWriter; + private Dictionary _usingValuesMap; + private void InitParallelParameterSet() { - // TODO: - // Initialize PSTaskPool - // Process script block Ast using variable expressions - // Create usingValuesMap - // or should variables be passed in as ISS entries or usingValueMap arguments? + bool allowUsingExpression = (this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage); + _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(ScriptBlock, allowUsingExpression, this.Context, null); + + if (AsJob) + { + // TODO: Create parent job + // _parentThreadJob (TaskJob?) = ... + // _taskPool = new PSTaskPool(ThrottleLimit, dataStreamWriter); + } + else + { + _taskDataStreamWriter = new PSTaskDataStreamWriter(this); + _taskPool = new PSTaskPool(ThrottleLimit, _taskDataStreamWriter); + _taskPool.TaskComplete += new EventHandler(HandleTaskComplete); + // TODO: Add timeout timer as needed. + } + } + + private void HandleTaskComplete(object sender, PSTaskCompleteEventArgs args) + { + args.Task.Dispose(); } private void ProcessParallelParameterSet() { - // TODO: - // Create PSTask for each pipeline input - // Hook up data streams object collect/writer and host (pass in cmdlet based object stream writer) - // Need to figure out how to pass in dollarUnderbar variable - // Currently no way to pass in via PowerShell object, need to find a way to do this - // PowerShell -> LocalPipe -> Pipeline processing -> Scriptblock processing -> Invoke scriptblock, pass in dollarUnderbar (ScriptCommandProcessor.cs) - // Create usingValuesMap parameter as needed - // - // Sync: Hand each pipeline input to PSTaskPool, block as necessary - // Catch any exception and Write as ErrorRecord object to error data stream - // Async: Create PSThreadChildJob wrapper for and add parent job, and to job queue + var task = new System.Management.Automation.PSTasks.PSTask( + ScriptBlock, + _usingValuesMap, + InputObject, + _taskDataStreamWriter, + this); + + if (AsJob) + { + // TODO: Create child job and add to parent + } + else + { + // Write any streaming data + _taskDataStreamWriter.WriteImmediate(); + + // Add task to task pool. + // Block if the pool is full and wait until task can be added. + _taskPool.Add(task); + } } private void EndParallelParameterSet() { - // TODO: - // Sync: Make sure all stream objects are written (see InvokeCommandCommand) - // Async: Write parent job object + if (AsJob) + { + // TODO: Write parent job to output + } + else + { + _taskDataStreamWriter.WriteImmediate(); + + _taskPool.Close(); + _taskDataStreamWriter.WaitAndWrite(); + + _taskDataStreamWriter.Dispose(); + _taskPool.Dispose(); + } } private void StopParallelProcessing() { - // TODO: - // Sync: Stop processing input (PSTaskPool accepts no more input) - // Send stop to all running PSTasks in PSTaskPool - // Wait for all tasks to stop - // Async: NoOp + if (!AsJob) + { + _taskPool.StopAll(); + } } private void EndBlockParameterSet() diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 06a95123656..201cf49f803 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -1,12 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System; -using System.Collections.Concurrent; using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Management.Automation; -using System.Management.Automation.Host; using System.Management.Automation.Remoting.Internal; using System.Management.Automation.Runspaces; using System.Management.Automation.Security; @@ -31,6 +26,7 @@ internal sealed class PSTask : IDisposable private readonly PSTaskDataStreamWriter _dataStreamWriter; private readonly Job2 _job; private readonly PSCmdlet _cmdlet; + private readonly int _id; private Runspace _runspace; private PowerShell _powershell; @@ -38,6 +34,8 @@ internal sealed class PSTask : IDisposable private const string VERBATIM_ARGUMENT = "--%"; + private static int s_taskId = 0; + #endregion #region Events @@ -51,7 +49,10 @@ internal sealed class PSTask : IDisposable #region Properties - PSInvocationState State + /// + /// Current running state of the task + /// + public PSInvocationState State { get { @@ -65,11 +66,22 @@ PSInvocationState State } } + /// + /// Task Id + /// + public int Id + { + get { return _id; } + } + #endregion #region Constructor - private PSTask() { } + private PSTask() + { + _id = Interlocked.Increment(ref s_taskId); + } /// /// Constructor for data streaming @@ -79,7 +91,7 @@ public PSTask( Dictionary usingValuesMap, object dollarUnderbar, PSTaskDataStreamWriter dataStreamWriter, - PSCmdlet psCmdlet) + PSCmdlet psCmdlet) : this() { _scriptBlockToRun = scriptBlock; _usingValuesMap = usingValuesMap; @@ -96,7 +108,7 @@ public PSTask( Dictionary usingValuesMap, object dollarUnderbar, Job2 job, - PSCmdlet psCmdlet) + PSCmdlet psCmdlet) : this() { _scriptBlockToRun = scriptBlock; _usingValuesMap = usingValuesMap; @@ -141,6 +153,8 @@ public void Start() // _localsTuple = scriptBlock.MakeLocalsTuple(_runOptimizedCode); // _localsTuple.SetAutomaticVariable(AutomaticVariable.Underbar, dollarUnderbar, _context); // See: ScriptCommandProcessor.cs + // TODO: Also update argument binding using variable look up to detect ScriptBlock using variable + // and recreate ScriptBlock instance using Ast iss.Variables.Add( new SessionStateVariableEntry("DollarUnderbar", _dollarUnderbar, string.Empty) ); @@ -167,15 +181,16 @@ public void Start() _powershell.InvocationStateChanged += new EventHandler(HandleStateChanged); // Start the script running in a new thread + _powershell.AddScript(_scriptBlockToRun.ToString()); _powershell.BeginInvoke(null, _output); } /// - /// Stop running task + /// Signals the running task to stop /// - public void Stop() + public void SignalStop() { - _powershell.Stop(); + _powershell.BeginStop(null, null); } #endregion @@ -322,25 +337,24 @@ private void InitializePowerShellforJobs() private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) { - // Treat any terminating exception as a non-terminating error record - var newStateInfo = stateChangeInfo.InvocationStateInfo; - if (newStateInfo.Reason != null) + if (_dataStreamWriter != null) { - var errorRecord = new ErrorRecord( - newStateInfo.Reason, - "PSTaskException", - ErrorCategory.InvalidOperation, - this); - - if (_dataStreamWriter != null) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Error, errorRecord) - ); - } - else + // Treat any terminating exception as a non-terminating error record + var newStateInfo = stateChangeInfo.InvocationStateInfo; + if (newStateInfo.Reason != null) { - // TODO: Jobs + var errorRecord = new ErrorRecord( + newStateInfo.Reason, + "PSTaskException", + ErrorCategory.InvalidOperation, + this); + + if (_dataStreamWriter != null) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Error, errorRecord) + ); + } } } @@ -433,8 +447,6 @@ public void WaitAndWrite() /// public void Close() { - CheckCmdletThread(); - _dataStream.Complete(); } @@ -470,9 +482,219 @@ public void Dispose() #region PSTaskPool - internal sealed class PSTaskPool + /// + /// Event arguments for TaskComplete event + /// + internal sealed class PSTaskCompleteEventArgs : EventArgs + { + public PSTask Task + { + get; + private set; + } + + private PSTaskCompleteEventArgs() { } + + public PSTaskCompleteEventArgs(PSTask task) + { + Task = task; + } + } + + /// + /// Pool for running PSTasks, with limit of total number of running tasks at a time + /// + internal sealed class PSTaskPool : IDisposable { - // TODO: + #region Members + + private readonly int _sizeLimit; + private bool _isOpen; + private readonly object _syncObject; + private readonly ManualResetEvent _addAvailable; + private readonly ManualResetEvent _stopAll; + private readonly PSTaskDataStreamWriter _dataStreamWriter; + private readonly Dictionary _taskPool; + + #endregion + + #region Constructor + + private PSTaskPool() { } + + /// + /// Constructor + /// + public PSTaskPool(int size, PSTaskDataStreamWriter dataStreamWriter) + { + + _sizeLimit = size; + _dataStreamWriter = dataStreamWriter; + _isOpen = true; + _syncObject = new object(); + _addAvailable = new ManualResetEvent(true); + _stopAll = new ManualResetEvent(false); + _taskPool = new Dictionary(size); + } + + #endregion + + #region Events + + /// + /// Event that fires when a running task completes + /// + public event EventHandler TaskComplete; + + #endregion + + #region Public Properties + + /// + /// Returns true if pool is currently open for accepting tasks + /// + public bool IsOpen + { + get { return _isOpen; } + } + + #endregion + + #region IDisposable + + /// + /// Dispose task pool + /// + public void Dispose() + { + _addAvailable.Dispose(); + _stopAll.Dispose(); + } + + #endregion + + #region Public Methods + + /// + /// Method to add a task to the pool + /// If the pool is full, then this method blocks until room is available + /// This method is not multi-thread safe and assumes only one thread waits and adds tasks + /// + public bool Add(PSTask task) + { + if (! _isOpen) + { + return false; + } + + // Block until either room is available or a stop is commanded + var index = WaitHandle.WaitAny(new WaitHandle[] { + _addAvailable, // index 0 + _stopAll // index 1 + }); + + if (index == 1) + { + return false; + } + + task.StateChanged += new EventHandler(HandleTaskStateChanged); + + lock (_syncObject) + { + _taskPool.Add(task.Id, task); + if (_taskPool.Count == _sizeLimit) + { + _addAvailable.Reset(); + } + } + + task.Start(); + + return true; + } + + /// + /// Signals all running tasks to stop and closes pool for any new tasks + /// + public void StopAll() + { + // Accept no more input + Close(); + _stopAll.Set(); + + // Send stop signal to any running tasks + lock (_syncObject) + { + foreach (var task in _taskPool.Values) + { + task.SignalStop(); + } + } + } + + /// + /// Closes the pool and prevents any new tasks from being added + /// + public void Close() + { + _isOpen = false; + } + + #endregion + + #region Private Methods + + private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEventArgs args) + { + var task = sender as PSTask; + Dbg.Assert(task != null, "State changed sender must always be PSTask"); + var stateInfo = args.InvocationStateInfo; + switch (stateInfo.State) + { + // Look for completed state and remove + case PSInvocationState.Completed: + case PSInvocationState.Stopped: + case PSInvocationState.Failed: + lock (_syncObject) + { + _taskPool.Remove(task.Id); + if (_taskPool.Count == (_sizeLimit - 1)) + { + _addAvailable.Set(); + } + } + task.StateChanged -= new EventHandler(HandleTaskStateChanged); + try + { + TaskComplete.Invoke( + this, + new PSTaskCompleteEventArgs(task) + ); + } + catch + { + Dbg.Assert(false, "Exceptions should not be thrown on event thread"); + } + CheckForComplete(); + break; + } + } + + private void CheckForComplete() + { + lock (_syncObject) + { + if (!_isOpen && _taskPool.Count == 0) + { + _dataStreamWriter.Close(); + + // TODO: Callback? + } + } + } + + #endregion } #endregion From 12c577f45f14bcd1d70b0da4b238c13011b1a70b Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Wed, 17 Jul 2019 15:36:55 -0700 Subject: [PATCH 04/27] Completed initial sync implementation --- .../engine/InternalCommands.cs | 52 ++- .../engine/ScriptCommandProcessor.cs | 12 + .../engine/hostifaces/Command.cs | 10 +- .../engine/hostifaces/PSTask.cs | 321 ++++++++++-------- .../resources/InternalCommandStrings.resx | 3 + 5 files changed, 235 insertions(+), 163 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index bf0533f5a4f..9324d8f2765 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -59,7 +59,7 @@ public object GetValue(PSObject inputObject, string propertyName) [Cmdlet("ForEach", "Object", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.Low, DefaultParameterSetName = "ScriptBlockSet", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=113300", RemotingCapability = RemotingCapability.None)] - public sealed class ForEachObjectCommand : PSCmdlet + public sealed class ForEachObjectCommand : PSCmdlet, IDisposable { #region Private Members @@ -244,7 +244,7 @@ public int ThrottleLimit /// The default value is 0, indicating no timeout. /// [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - [ValidateRange(0, Int32.MaxValue)] + [ValidateRange(0, (Int32.MaxValue/1000))] public int TimeoutSeconds { get; @@ -346,11 +346,38 @@ protected override void StopProcessing() #endregion + #region IDisposable + + /// + /// Dispose + /// + public void Dispose() + { + // Ensure all parallel task objects are disposed + if (_taskTimer != null) + { + _taskTimer.Dispose(); + } + if (_taskDataStreamWriter != null) + { + _taskDataStreamWriter.Dispose(); + } + if (_taskPool != null) + { + _taskPool.Dispose(); + } + } + + #endregion + #region Private Methods + #region PSTasks + private PSTaskPool _taskPool; private PSTaskDataStreamWriter _taskDataStreamWriter; private Dictionary _usingValuesMap; + private System.Threading.Timer _taskTimer; private void InitParallelParameterSet() { @@ -367,16 +394,18 @@ private void InitParallelParameterSet() { _taskDataStreamWriter = new PSTaskDataStreamWriter(this); _taskPool = new PSTaskPool(ThrottleLimit, _taskDataStreamWriter); - _taskPool.TaskComplete += new EventHandler(HandleTaskComplete); - // TODO: Add timeout timer as needed. + if (TimeoutSeconds != 0) + { + // TODO: Throw a 'timeout' exception? Already get pipeline stopped exception. + _taskTimer = new System.Threading.Timer( + (_) => _taskPool.StopAll(), + null, + (TimeoutSeconds * 1000), + System.Threading.Timeout.Infinite); + } } } - private void HandleTaskComplete(object sender, PSTaskCompleteEventArgs args) - { - args.Task.Dispose(); - } - private void ProcessParallelParameterSet() { var task = new System.Management.Automation.PSTasks.PSTask( @@ -413,9 +442,6 @@ private void EndParallelParameterSet() _taskPool.Close(); _taskDataStreamWriter.WaitAndWrite(); - - _taskDataStreamWriter.Dispose(); - _taskPool.Dispose(); } } @@ -427,6 +453,8 @@ private void StopParallelProcessing() } } + #endregion + private void EndBlockParameterSet() { if (_endScript == null) diff --git a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs index 5e51f779230..f3a40d7fedb 100644 --- a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs +++ b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs @@ -235,8 +235,16 @@ internal sealed class DlrScriptCommandProcessor : ScriptCommandProcessorBase private MutableTuple _localsTuple; private bool _runOptimizedCode; private bool _argsBound; + private object _dollarUnderbar; private FunctionContext _functionContext; + internal DlrScriptCommandProcessor(ScriptBlock scriptBlock, ExecutionContext context, bool useNewScope, CommandOrigin origin, SessionStateInternal sessionState, object dollarUnderbar) + : base(scriptBlock, context, useNewScope, origin, sessionState) + { + Init(); + _dollarUnderbar = dollarUnderbar; + } + internal DlrScriptCommandProcessor(ScriptBlock scriptBlock, ExecutionContext context, bool useNewScope, CommandOrigin origin, SessionStateInternal sessionState) : base(scriptBlock, context, useNewScope, origin, sessionState) { @@ -513,6 +521,10 @@ private void RunClause(Action clause, object dollarUnderbar, ob { _localsTuple.SetAutomaticVariable(AutomaticVariable.Underbar, dollarUnderbar, _context); } + else if (_dollarUnderbar != AutomationNull.Value) + { + _localsTuple.SetAutomaticVariable(AutomaticVariable.Underbar, _dollarUnderbar, _context); + } if (inputToProcess != AutomationNull.Value) { diff --git a/src/System.Management.Automation/engine/hostifaces/Command.cs b/src/System.Management.Automation/engine/hostifaces/Command.cs index cfc8c8dbe77..afb06dd5a2e 100644 --- a/src/System.Management.Automation/engine/hostifaces/Command.cs +++ b/src/System.Management.Automation/engine/hostifaces/Command.cs @@ -173,6 +173,13 @@ internal bool? UseLocalScopeNullable get { return _useLocalScope; } } + /// + /// DollarUnderbar ($_) value to be used with script command. + /// This is used by foreach-object -parallel where each piped input ($_) is associated + /// with a parallel running script block. + /// + internal object DollarUnderbar { get; set; } + /// /// Checks if the current command marks the end of a statement (see PowerShell.AddStatement()) /// @@ -495,7 +502,8 @@ CommandOrigin origin commandProcessorBase = new DlrScriptCommandProcessor(scriptBlock, executionContext, _useLocalScope ?? false, origin, - executionContext.EngineSessionState); + executionContext.EngineSessionState, + DollarUnderbar); } } else diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 201cf49f803..f6d36379fc4 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -14,7 +14,7 @@ namespace System.Management.Automation.PSTasks #region PSTask /// - /// Class to encapsulate running a PowerShell script concurrently in a cmdlet context + /// Class to encapsulate running a PowerShell script concurrently in a cmdlet or job context /// internal sealed class PSTask : IDisposable { @@ -25,7 +25,6 @@ internal sealed class PSTask : IDisposable private readonly object _dollarUnderbar; private readonly PSTaskDataStreamWriter _dataStreamWriter; private readonly Job2 _job; - private readonly PSCmdlet _cmdlet; private readonly int _id; private Runspace _runspace; @@ -97,7 +96,6 @@ public PSTask( _usingValuesMap = usingValuesMap; _dollarUnderbar = dollarUnderbar; _dataStreamWriter = dataStreamWriter; - _cmdlet = psCmdlet; } /// @@ -114,7 +112,6 @@ public PSTask( _usingValuesMap = usingValuesMap; _dollarUnderbar = dollarUnderbar; _job = job; - _cmdlet = psCmdlet; } #endregion @@ -139,26 +136,15 @@ public void Start() { if (_powershell != null) { - // TODO: Localize message - throw new PSInvalidOperationException("This task has already been started. A task can be started only once."); + Dbg.Assert(false, "A PSTask can be started only once."); + return; } // Create and open Runspace for this task to run in var iss = InitialSessionState.CreateDefault2(); iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) ? PSLanguageMode.ConstrainedLanguage : PSLanguageMode.FullLanguage; - // TODO: Fix dollarUnderbar variable so that it gets passed correctly to the script block - // It needs to be passed to a compiled script block's localsTuple (will likely need ps.Invoke override): - // scriptBlock.Compile(); - // _localsTuple = scriptBlock.MakeLocalsTuple(_runOptimizedCode); - // _localsTuple.SetAutomaticVariable(AutomaticVariable.Underbar, dollarUnderbar, _context); - // See: ScriptCommandProcessor.cs - // TODO: Also update argument binding using variable look up to detect ScriptBlock using variable - // and recreate ScriptBlock instance using Ast - iss.Variables.Add( - new SessionStateVariableEntry("DollarUnderbar", _dollarUnderbar, string.Empty) - ); - _runspace = RunspaceFactory.CreateRunspace(_cmdlet.Host, iss); + _runspace = RunspaceFactory.CreateRunspace(iss); _runspace.Open(); // Create the PowerShell command pipeline for the provided script block @@ -178,10 +164,15 @@ public void Start() } // State change handler - _powershell.InvocationStateChanged += new EventHandler(HandleStateChanged); + _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(sender, args); // Start the script running in a new thread _powershell.AddScript(_scriptBlockToRun.ToString()); + _powershell.Commands.Commands[0].DollarUnderbar = _dollarUnderbar; + if (_usingValuesMap != null && _usingValuesMap.Count > 0) + { + _powershell.AddParameter(VERBATIM_ARGUMENT, _usingValuesMap); + } _powershell.BeginInvoke(null, _output); } @@ -202,139 +193,163 @@ private void InitializePowerShellforDataStreaming() Dbg.Assert(_dataStreamWriter != null, "Data stream writer cannot be null"); // Writer data stream handlers - _output.DataAdded += (sender, args) => - { - foreach (var item in _output.ReadAll()) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Output, item) - ); - } - }; + _output.DataAdded += (sender, args) => HandleOutputData(sender, args); + _powershell.Streams.Error.DataAdded += (sender, args) => HandleErrorData(sender, args); + _powershell.Streams.Warning.DataAdded += (sender, args) => HandleWarningData(sender, args); + _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleVerboseData(sender, args); + _powershell.Streams.Debug.DataAdded += (sender, args) => HandleDebugData(sender, args); + _powershell.Streams.Information.DataAdded += (sender, args) => HandleInformationData(sender, args); + } + + private void InitializePowerShellforJobs() + { + Dbg.Assert(_job != null, "Job object cannot be null"); + + // Job data stream handlers + _output.DataAdded += (sender, args) => HandleJobOutputData(sender, args); + _powershell.Streams.Error.DataAdded += (sender, args) => HandleJobErrorData(sender, args); + _powershell.Streams.Warning.DataAdded += (sender, args) => HandleJobWarningData(sender, args); + _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleJobVerboseData(sender, args); + _powershell.Streams.Debug.DataAdded += (sender, args) => HandleJobDebugData(sender, args); + _powershell.Streams.Information.DataAdded += (sender, args) => HandleJobInformationData(sender, args); + } + + #region Event handlers + + #region Writer data stream handlers - _powershell.Streams.Error.DataAdded += (sender, args) => + private void HandleOutputData(object sender, DataAddedEventArgs args) + { + foreach (var item in _output.ReadAll()) { - foreach (var item in _powershell.Streams.Error.ReadAll()) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Error, item) - ); - } - }; + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Output, item) + ); + } + } - _powershell.Streams.Warning.DataAdded += (sender, args) => + private void HandleErrorData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Error.ReadAll()) { - foreach (var item in _powershell.Streams.Warning.ReadAll()) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.WarningRecord, item) - ); - } - }; + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Error, item) + ); + } + } - _powershell.Streams.Verbose.DataAdded += (sender, args) => + private void HandleWarningData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Warning.ReadAll()) { - foreach (var item in _powershell.Streams.Verbose.ReadAll()) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Verbose, item.Message) - ); - } - }; + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Warning, item.Message) + ); + } + } - _powershell.Streams.Debug.DataAdded += (sender, args) => + private void HandleVerboseData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Verbose.ReadAll()) { - foreach (var item in _powershell.Streams.Debug.ReadAll()) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Debug, item.Message) - ); - } - }; + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Verbose, item.Message) + ); + } + } - _powershell.Streams.Information.DataAdded += (sender, args) => + private void HandleDebugData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Debug.ReadAll()) { - foreach (var item in _powershell.Streams.Debug.ReadAll()) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Information, item) - ); - } - }; + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Debug, item.Message) + ); + } } - private void InitializePowerShellforJobs() + private void HandleInformationData(object sender, DataAddedEventArgs args) { - Dbg.Assert(_job != null, "Data stream writer cannot be null"); + foreach (var item in _powershell.Streams.Information.ReadAll()) + { + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Information, item) + ); + } + } - // Job data stream handlers - _output.DataAdded += (sender, args) => + #endregion + + #region Job data stream handlers + + private void HandleJobOutputData(object sender, DataAddedEventArgs args) + { + foreach (var item in _output.ReadAll()) { - foreach (var item in _output.ReadAll()) - { - _job.Output.Add(item); - _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Output, item) - ); - } - }; + _job.Output.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Output, item) + ); + } + } - _powershell.Streams.Error.DataAdded += (sender, args) => + private void HandleJobErrorData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Error.ReadAll()) { - foreach (var item in _powershell.Streams.Error.ReadAll()) - { - _job.Error.Add(item); - _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Error, item) - ); - } - }; + _job.Error.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Error, item) + ); + } + } - _powershell.Streams.Warning.DataAdded += (sender, args) => + private void HandleJobWarningData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Warning.ReadAll()) { - foreach (var item in _powershell.Streams.Warning.ReadAll()) - { - _job.Warning.Add(item); - _job.Results.Add( - new PSStreamObject(PSStreamObjectType.WarningRecord, item) - ); - } - }; + _job.Warning.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Warning, item.Message) + ); + } + } - _powershell.Streams.Verbose.DataAdded += (sender, args) => + private void HandleJobVerboseData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Verbose.ReadAll()) { - foreach (var item in _powershell.Streams.Verbose.ReadAll()) - { - _job.Verbose.Add(item); - _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Verbose, item.Message) - ); - } - }; + _job.Verbose.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Verbose, item.Message) + ); + } + } - _powershell.Streams.Debug.DataAdded += (sender, args) => + private void HandleJobDebugData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Debug.ReadAll()) { - foreach (var item in _powershell.Streams.Debug.ReadAll()) - { - _job.Debug.Add(item); - _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Debug, item.Message) - ); - } - }; + _job.Debug.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Debug, item.Message) + ); + } + } - _powershell.Streams.Information.DataAdded += (sender, args) => + private void HandleJobInformationData(object sender, DataAddedEventArgs args) + { + foreach (var item in _powershell.Streams.Information.ReadAll()) { - foreach (var item in _powershell.Streams.Information.ReadAll()) - { - _job.Information.Add(item); - _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Information, item) - ); - } - }; + _job.Information.Add(item); + _job.Results.Add( + new PSStreamObject(PSStreamObjectType.Information, item) + ); + } } + #endregion + private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) { if (_dataStreamWriter != null) @@ -349,12 +364,9 @@ private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs ErrorCategory.InvalidOperation, this); - if (_dataStreamWriter != null) - { - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Error, errorRecord) - ); - } + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Error, errorRecord) + ); } } @@ -362,6 +374,8 @@ private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs } #endregion + + #endregion } #endregion @@ -423,6 +437,7 @@ public void WriteImmediate() /// /// Waits for data stream objects to be added to the collection, and writes them /// to the cmdlet data stream. + /// This method returns only after the writer has been closed. /// public void WaitAndWrite() { @@ -431,14 +446,13 @@ public void WaitAndWrite() while (true) { _dataStream.WaitHandle.WaitOne(); + WriteImmediate(); if (!_dataStream.IsOpen) { + WriteImmediate(); break; } - - // Data ready to write - WriteImmediate(); } } @@ -458,8 +472,7 @@ private void CheckCmdletThread() { if (Thread.CurrentThread.ManagedThreadId != _cmdletThreadId) { - // TODO: Localize message - throw new PSInvalidOperationException("The method cannot be called from this thread. This method can only be called from the same cmdlet thread that created this object instance."); + throw new PSInvalidOperationException(InternalCommandStrings.PSTaskStreamWriterWrongThread); } } @@ -482,6 +495,8 @@ public void Dispose() #region PSTaskPool + #region PSTaskCompleteEventArgs + /// /// Event arguments for TaskComplete event /// @@ -501,8 +516,10 @@ public PSTaskCompleteEventArgs(PSTask task) } } + #endregion + /// - /// Pool for running PSTasks, with limit of total number of running tasks at a time + /// Pool for running PSTasks, with limit of total number of running tasks at a time. /// internal sealed class PSTaskPool : IDisposable { @@ -527,7 +544,6 @@ private PSTaskPool() { } /// public PSTaskPool(int size, PSTaskDataStreamWriter dataStreamWriter) { - _sizeLimit = size; _dataStreamWriter = dataStreamWriter; _isOpen = true; @@ -576,9 +592,9 @@ public void Dispose() #region Public Methods /// - /// Method to add a task to the pool - /// If the pool is full, then this method blocks until room is available - /// This method is not multi-thread safe and assumes only one thread waits and adds tasks + /// Method to add a task to the pool. + /// If the pool is full, then this method blocks until room is available. + /// This method is not multi-thread safe and assumes only one thread waits and adds tasks. /// public bool Add(PSTask task) { @@ -598,18 +614,23 @@ public bool Add(PSTask task) return false; } - task.StateChanged += new EventHandler(HandleTaskStateChanged); + task.StateChanged += (sender, args) => HandleTaskStateChanged(sender, args); lock (_syncObject) { + if (! _isOpen) + { + return false; + } + _taskPool.Add(task.Id, task); if (_taskPool.Count == _sizeLimit) { _addAvailable.Reset(); } - } - task.Start(); + task.Start(); + } return true; } @@ -623,12 +644,12 @@ public void StopAll() Close(); _stopAll.Set(); - // Send stop signal to any running tasks + // Stop all running tasks lock (_syncObject) { foreach (var task in _taskPool.Values) { - task.SignalStop(); + task.Dispose(); } } } @@ -639,6 +660,7 @@ public void StopAll() public void Close() { _isOpen = false; + CheckForComplete(); } #endregion @@ -664,10 +686,10 @@ private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEvent _addAvailable.Set(); } } - task.StateChanged -= new EventHandler(HandleTaskStateChanged); + task.StateChanged -= (sender, args) => HandleTaskStateChanged(sender, args); try { - TaskComplete.Invoke( + TaskComplete.SafeInvoke( this, new PSTaskCompleteEventArgs(task) ); @@ -676,6 +698,7 @@ private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEvent { Dbg.Assert(false, "Exceptions should not be thrown on event thread"); } + task.Dispose(); CheckForComplete(); break; } @@ -688,8 +711,6 @@ private void CheckForComplete() if (!_isOpen && _taskPool.Count == 0) { _dataStreamWriter.Close(); - - // TODO: Callback? } } } diff --git a/src/System.Management.Automation/resources/InternalCommandStrings.resx b/src/System.Management.Automation/resources/InternalCommandStrings.resx index 666b2e190b4..b1202b0ede1 100644 --- a/src/System.Management.Automation/resources/InternalCommandStrings.resx +++ b/src/System.Management.Automation/resources/InternalCommandStrings.resx @@ -165,4 +165,7 @@ The specified operator requires both the -Property and -Value parameters. Provide values for both parameters, and then try the command again. + + This method cannot be run on the current thread. It can only be called on the cmdlet thread. + From 546fccead5c6a93658f455ec76cb47ab7f7fe9b2 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Thu, 18 Jul 2019 13:22:18 -0700 Subject: [PATCH 05/27] Added basic tests for synchronous mode --- .../engine/InternalCommands.cs | 1 - .../engine/hostifaces/PSTask.cs | 3 + .../Foreach-Object-Parallel.Tests.ps1 | 77 +++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index 9324d8f2765..d79468d8bd2 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -396,7 +396,6 @@ private void InitParallelParameterSet() _taskPool = new PSTaskPool(ThrottleLimit, _taskDataStreamWriter); if (TimeoutSeconds != 0) { - // TODO: Throw a 'timeout' exception? Already get pipeline stopped exception. _taskTimer = new System.Threading.Timer( (_) => _taskPool.StopAll(), null, diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index f6d36379fc4..2156bd341e0 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System.Collections.Generic; +using System.Globalization; using System.Management.Automation.Remoting.Internal; using System.Management.Automation.Runspaces; using System.Management.Automation.Security; @@ -34,6 +35,7 @@ internal sealed class PSTask : IDisposable private const string VERBATIM_ARGUMENT = "--%"; private static int s_taskId = 0; + private const string RunspaceName = "PSTask"; #endregion @@ -145,6 +147,7 @@ public void Start() iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) ? PSLanguageMode.ConstrainedLanguage : PSLanguageMode.FullLanguage; _runspace = RunspaceFactory.CreateRunspace(iss); + _runspace.Name = string.Format(CultureInfo.InvariantCulture, "{0}:{1}", RunspaceName, s_taskId); _runspace.Open(); // Create the PowerShell command pipeline for the provided script block diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 new file mode 100644 index 00000000000..1149c3b638e --- /dev/null +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -0,0 +1,77 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { + + It "Verifies dollar underbar variable" { + + $expected = 1..10 + $result = $expected | ForEach-Object -Parallel -ScriptBlock { $_ } + $result.Count | Should -BeExactly $expected.Count + $result | Should -Contain 1 + $result | Should -Contain 10 + } + + It 'Verifies using variables' { + + $var = "Hello" + $varArray = "Hello","There" + $result = 1..1 | ForEach-Object -Parallel -ScriptBlock { $using:var; $using:varArray[1] } + $result.Count | Should -BeExactly 2 + $result[0] | Should -BeExactly $var + $result[1] | Should -BeExactly $varArray[1] + } + + It 'Verifies non-terminating error streaming' { + + $expectedError = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Error "Error!" } 2>&1 + $expectedError.ToString() | Should -BeExactly 'Error!' + $expectedError.FullyQualifiedErrorId | Should -BeExactly 'Microsoft.PowerShell.Commands.WriteErrorException' + } + + It 'Verifies terminating error streaming' { + + $result = 1..1 | ForEach-Object -Parallel -ScriptBlock { throw 'Terminating Error!'; "Hello" } 2>&1 + $result.Count | Should -BeExactly 1 + $result.ToString() | Should -BeExactly 'Terminating Error!' + $result.FullyQualifiedErrorId | Should -BeExactly 'PSTaskException' + } + + It 'Verifies warning data streaming' { + + $expectedWarning = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Warning "Warning!" } 3>&1 + $expectedWarning.Message | Should -BeExactly 'Warning!' + } + + It 'Verifies verbose data streaming' { + + $expectedVerbose = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Verbose "Verbose!" -Verbose } -Verbose 4>&1 + $expectedVerbose.Message | Should -BeExactly 'Verbose!' + } + + It 'Verifies debug data streaming' { + + $expectedDebug = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Debug "Debug!" -Debug } -Debug 5>&1 + $expectedDebug.Message | Should -BeExactly 'Debug!' + } + + It 'Verifies information data streaming' { + + $expectedInformation = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Information "Information!" } 6>&1 + $expectedInformation.MessageData | Should -BeExactly 'Information!' + } +} + +Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { + + It 'Verifies timeout and throttle parameters' { + + # With ThrottleLimit set to 1, the two 60 second long script blocks will run sequentially, + # until the timeout in 5 seconds. + $results = 1..2 | ForEach-Object -Parallel { "Output $_"; Start-Sleep -Seconds 60 } -TimeoutSeconds 5 -ThrottleLimit 1 2>&1 + $results.Count | Should -BeExactly 2 + $results[0] | Should -BeExactly 'Output 1' + $results[1].FullyQualifiedErrorId | Should -BeExactly 'PSTaskException' + $results[1].Exception | Should -BeOfType [System.Management.Automation.PipelineStoppedException] + } +} From 264520b53b6478b4c39c14342968ab5cca96eee1 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 22 Jul 2019 08:57:14 -0700 Subject: [PATCH 06/27] Add script block errors and more tests --- .../engine/InternalCommands.cs | 31 +++++++++++++++++++ .../resources/InternalCommandStrings.resx | 6 ++++ .../ConstrainedLanguageRestriction.Tests.ps1 | 20 ++++++++++++ .../Foreach-Object-Parallel.Tests.ps1 | 15 +++++++++ 4 files changed, 72 insertions(+) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index d79468d8bd2..3834322c0fb 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -383,6 +383,22 @@ private void InitParallelParameterSet() { bool allowUsingExpression = (this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage); _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(ScriptBlock, allowUsingExpression, this.Context, null); + + // Validate using values map + foreach (var item in _usingValuesMap.Values) + { + if (item is ScriptBlock) + { + this.ThrowTerminatingError( + new ErrorRecord( + new PSArgumentException(InternalCommandStrings.ParallelUsingVariableCannotBeScriptBlock), + "ParallelUsingVariableCannotBeScriptBlock", + ErrorCategory.InvalidType, + this + ) + ); + } + } if (AsJob) { @@ -407,6 +423,21 @@ private void InitParallelParameterSet() private void ProcessParallelParameterSet() { + // Validate piped InputObject + if (_inputObject.BaseObject is ScriptBlock) + { + this.WriteError( + new ErrorRecord( + new PSArgumentException(InternalCommandStrings.ParallelPipedInputObjectCannotBeScriptBlock), + "ParallelPipedInputObjectCannotBeScriptBlock", + ErrorCategory.InvalidType, + this + ) + ); + + return; + } + var task = new System.Management.Automation.PSTasks.PSTask( ScriptBlock, _usingValuesMap, diff --git a/src/System.Management.Automation/resources/InternalCommandStrings.resx b/src/System.Management.Automation/resources/InternalCommandStrings.resx index b1202b0ede1..669f8e8e14f 100644 --- a/src/System.Management.Automation/resources/InternalCommandStrings.resx +++ b/src/System.Management.Automation/resources/InternalCommandStrings.resx @@ -168,4 +168,10 @@ This method cannot be run on the current thread. It can only be called on the cmdlet thread. + + A ForEach-Object -Parallel using variable cannot be a script block. Script blocks have undefined behavior when used in parallel. + + + A ForEach-Object -Parallel piped input object cannot be a script block. Script blocks have undefined behavior when used in parallel. + diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 index 19ef2df2cef..91e0952c8b0 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 @@ -878,6 +878,26 @@ try } } + Describe "ForEach-Object -Parallel Constrained Language Tests" -Tags 'Feature','RequireAdminOnWindows' { + + It 'Foreach-Object -Parallel must run in ConstrainedLanguage mode under system lock down' { + + try + { + $ExecutionContext.SessionState.LanguageMode = "ConstrainedLanguage" + Invoke-LanguageModeTestingSupportCmdlet -SetLockdownMode + + $results = 1..1 | ForEach-Object -Parallel -ScriptBlock { $ExecutionContext.SessionState.LanguageMode } + } + finally + { + Invoke-LanguageModeTestingSupportCmdlet -RevertLockdownMode -EnableFullLanguageMode + } + + $results | Should -BeExactly "ConstrainedLanguage" + } + } + Describe "Dot sourced script block functions from trusted script files should not run FullLanguage in ConstrainedLanguage context" -Tags 'Feature','RequireAdminOnWindows' { BeforeAll { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 1149c3b638e..6a35edaee2b 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -3,6 +3,11 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { + BeforeAll { + + $sb = { "Hello!" } + } + It "Verifies dollar underbar variable" { $expected = 1..10 @@ -60,6 +65,16 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { $expectedInformation = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Information "Information!" } 6>&1 $expectedInformation.MessageData | Should -BeExactly 'Information!' } + + It 'Verifies error for using script block variable' { + + { 1..1 | ForEach-Object -Parallel -ScriptBlock { $using:sb } } | Should -Throw -ErrorId 'ParallelUsingVariableCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } + + It 'Verifies error for script block piped variable' { + + { $sb | ForEach-Object -Parallel -ScriptBlock { "Hello" } -ErrorAction Stop } | Should -Throw -ErrorId 'ParallelPipedInputObjectCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } } Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { From d49e9de9180a8e5ebec16957c0a84f91b451d357 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 22 Jul 2019 10:49:34 -0700 Subject: [PATCH 07/27] Add as experimental feature --- .../engine/ExperimentalFeature/ExperimentalFeature.cs | 3 +++ src/System.Management.Automation/engine/InternalCommands.cs | 5 +++++ test/tools/TestMetadata.json | 3 ++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs index 57dae18d01f..7bf4678116d 100644 --- a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs +++ b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs @@ -108,6 +108,9 @@ static ExperimentalFeature() new ExperimentalFeature( name: "PSCommandNotFoundSuggestion", description: "Recommend potential commands based on fuzzy search on a CommandNotFoundException"), + new ExperimentalFeature( + name: "PSForEachObjectParallel", + description: "New parameter set for ForEach-Object to run script blocks in parallel") }; EngineExperimentalFeatures = new ReadOnlyCollection(engineFeatures); diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index 3834322c0fb..fa06183ded0 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -209,6 +209,7 @@ public object[] ArgumentList /// /// Flag to indicate that foreach iterations should be run in parallel instead of sequentially. /// + [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] public SwitchParameter Parallel { @@ -219,6 +220,7 @@ public SwitchParameter Parallel /// /// Script block to run for each pipeline object /// + [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(Position = 0, Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] [ValidateNotNull()] public ScriptBlock ScriptBlock @@ -231,6 +233,7 @@ public ScriptBlock ScriptBlock /// Specifies the maximum number of concurrently running scriptblocks on separate threads. /// The default number is 5. /// + [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] [ValidateRange(1, Int32.MaxValue)] public int ThrottleLimit @@ -243,6 +246,7 @@ public int ThrottleLimit /// Specifies a timeout time in seconds, after which the parallel running scripts will be stopped /// The default value is 0, indicating no timeout. /// + [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] [ValidateRange(0, (Int32.MaxValue/1000))] public int TimeoutSeconds @@ -255,6 +259,7 @@ public int TimeoutSeconds /// Flag that returns a job object immediately for the parallel operation, instead of returning after /// all foreach processing is completed. /// + [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] public SwitchParameter AsJob { diff --git a/test/tools/TestMetadata.json b/test/tools/TestMetadata.json index bc172948978..59c68cff185 100644 --- a/test/tools/TestMetadata.json +++ b/test/tools/TestMetadata.json @@ -1,6 +1,7 @@ { "ExperimentalFeatures": { "Microsoft.PowerShell.Utility.PSDebugRunspaceWithBreakpoints": ["test/powershell/Modules/Microsoft.PowerShell.Utility/New-PSBreakpoint.Tests.ps1"], - "ExpTest.FeatureOne": [ "test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1" ] + "ExpTest.FeatureOne": [ "test/powershell/engine/ExperimentalFeature/ExperimentalFeature.Basic.Tests.ps1" ], + "PSForEachObjectParallel": [ "test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1", "test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1" ] } } From 4be015fac2ebf3bd89f7e42fe6c2499f00eebc6b Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Tue, 23 Jul 2019 15:24:34 -0700 Subject: [PATCH 08/27] Add AsJob implementation --- .../engine/InternalCommands.cs | 52 ++- .../engine/hostifaces/PSTask.cs | 348 +++++++++++++++--- .../resources/InternalCommandStrings.resx | 3 + 3 files changed, 341 insertions(+), 62 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index fa06183ded0..e951f378f3e 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -383,6 +383,7 @@ public void Dispose() private PSTaskDataStreamWriter _taskDataStreamWriter; private Dictionary _usingValuesMap; private System.Threading.Timer _taskTimer; + private PSTaskJob _taskJob; private void InitParallelParameterSet() { @@ -399,22 +400,34 @@ private void InitParallelParameterSet() new PSArgumentException(InternalCommandStrings.ParallelUsingVariableCannotBeScriptBlock), "ParallelUsingVariableCannotBeScriptBlock", ErrorCategory.InvalidType, - this - ) + this) ); } } if (AsJob) { - // TODO: Create parent job - // _parentThreadJob (TaskJob?) = ... - // _taskPool = new PSTaskPool(ThrottleLimit, dataStreamWriter); + if (MyInvocation.BoundParameters.ContainsKey(nameof(TimeoutSeconds))) + { + this.ThrowTerminatingError( + new ErrorRecord( + new PSArgumentException(InternalCommandStrings.ParallelCannotUseTimeoutWithJob), + "ParallelCannotUseTimeoutWithJob", + ErrorCategory.InvalidOperation, + this) + ); + } + + _taskJob = new PSTaskJob(ThrottleLimit); } else { _taskDataStreamWriter = new PSTaskDataStreamWriter(this); - _taskPool = new PSTaskPool(ThrottleLimit, _taskDataStreamWriter); + _taskPool = new PSTaskPool(ThrottleLimit); + _taskPool.PoolComplete += (sender, args) => + { + _taskDataStreamWriter.Close(); + }; if (TimeoutSeconds != 0) { _taskTimer = new System.Threading.Timer( @@ -436,29 +449,32 @@ private void ProcessParallelParameterSet() new PSArgumentException(InternalCommandStrings.ParallelPipedInputObjectCannotBeScriptBlock), "ParallelPipedInputObjectCannotBeScriptBlock", ErrorCategory.InvalidType, - this - ) + this) ); return; } - var task = new System.Management.Automation.PSTasks.PSTask( - ScriptBlock, - _usingValuesMap, - InputObject, - _taskDataStreamWriter, - this); - if (AsJob) { - // TODO: Create child job and add to parent + var taskChildJob = new PSTaskChildJob( + ScriptBlock, + _usingValuesMap, + InputObject); + + _taskJob.AddJob(taskChildJob); } else { // Write any streaming data _taskDataStreamWriter.WriteImmediate(); + var task = new System.Management.Automation.PSTasks.PSTask( + ScriptBlock, + _usingValuesMap, + InputObject, + _taskDataStreamWriter); + // Add task to task pool. // Block if the pool is full and wait until task can be added. _taskPool.Add(task); @@ -469,7 +485,9 @@ private void EndParallelParameterSet() { if (AsJob) { - // TODO: Write parent job to output + _taskJob.Start(); + JobRepository.Add(_taskJob); + WriteObject(_taskJob); } else { diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 2156bd341e0..aca1af27bd5 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -25,7 +25,7 @@ internal sealed class PSTask : IDisposable private readonly Dictionary _usingValuesMap; private readonly object _dollarUnderbar; private readonly PSTaskDataStreamWriter _dataStreamWriter; - private readonly Job2 _job; + private readonly Job _job; private readonly int _id; private Runspace _runspace; @@ -91,8 +91,7 @@ public PSTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, object dollarUnderbar, - PSTaskDataStreamWriter dataStreamWriter, - PSCmdlet psCmdlet) : this() + PSTaskDataStreamWriter dataStreamWriter) : this() { _scriptBlockToRun = scriptBlock; _usingValuesMap = usingValuesMap; @@ -107,8 +106,7 @@ public PSTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, object dollarUnderbar, - Job2 job, - PSCmdlet psCmdlet) : this() + Job job) : this() { _scriptBlockToRun = scriptBlock; _usingValuesMap = usingValuesMap; @@ -184,7 +182,10 @@ public void Start() /// public void SignalStop() { - _powershell.BeginStop(null, null); + if (_powershell != null) + { + _powershell.BeginStop(null, null); + } } #endregion @@ -498,29 +499,6 @@ public void Dispose() #region PSTaskPool - #region PSTaskCompleteEventArgs - - /// - /// Event arguments for TaskComplete event - /// - internal sealed class PSTaskCompleteEventArgs : EventArgs - { - public PSTask Task - { - get; - private set; - } - - private PSTaskCompleteEventArgs() { } - - public PSTaskCompleteEventArgs(PSTask task) - { - Task = task; - } - } - - #endregion - /// /// Pool for running PSTasks, with limit of total number of running tasks at a time. /// @@ -533,7 +511,6 @@ internal sealed class PSTaskPool : IDisposable private readonly object _syncObject; private readonly ManualResetEvent _addAvailable; private readonly ManualResetEvent _stopAll; - private readonly PSTaskDataStreamWriter _dataStreamWriter; private readonly Dictionary _taskPool; #endregion @@ -545,10 +522,9 @@ private PSTaskPool() { } /// /// Constructor /// - public PSTaskPool(int size, PSTaskDataStreamWriter dataStreamWriter) + public PSTaskPool(int size) { _sizeLimit = size; - _dataStreamWriter = dataStreamWriter; _isOpen = true; _syncObject = new object(); _addAvailable = new ManualResetEvent(true); @@ -561,9 +537,9 @@ public PSTaskPool(int size, PSTaskDataStreamWriter dataStreamWriter) #region Events /// - /// Event that fires when a running task completes + /// Event that fires when pool is closed and drained of all tasks /// - public event EventHandler TaskComplete; + public event EventHandler PoolComplete; #endregion @@ -638,6 +614,14 @@ public bool Add(PSTask task) return true; } + /// + /// Add child job task to task pool + /// + public bool Add(PSTaskChildJob childJob) + { + return Add(childJob.Task); + } + /// /// Signals all running tasks to stop and closes pool for any new tasks /// @@ -690,31 +674,305 @@ private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEvent } } task.StateChanged -= (sender, args) => HandleTaskStateChanged(sender, args); + task.Dispose(); + CheckForComplete(); + break; + } + } + + private void CheckForComplete() + { + lock (_syncObject) + { + if (!_isOpen && _taskPool.Count == 0) + { try { - TaskComplete.SafeInvoke( - this, - new PSTaskCompleteEventArgs(task) + PoolComplete.SafeInvoke( + this, + new EventArgs() ); } - catch + catch { Dbg.Assert(false, "Exceptions should not be thrown on event thread"); } - task.Dispose(); - CheckForComplete(); - break; + } } } - private void CheckForComplete() + #endregion + } + + #endregion + + #region PSTaskJobs + + /// + /// Job for running ForEach-Object parallel task child jobs asynchronously + /// + internal sealed class PSTaskJob : Job + { + #region Members + + private readonly PSTaskPool _taskPool; + private bool _isOpen; + private bool _stopSignaled; + + #endregion + + #region Constructor + + private PSTaskJob() { } + + public PSTaskJob(int throttleLimit) : base(string.Empty, string.Empty) { - lock (_syncObject) + _taskPool = new PSTaskPool(throttleLimit); + _isOpen = true; + PSJobTypeName = nameof(PSTaskJob); + + _taskPool.PoolComplete += (sender, args) => { - if (!_isOpen && _taskPool.Count == 0) + try + { + if (_stopSignaled) + { + SetJobState(JobState.Stopped, new PipelineStoppedException()); + return; + } + + // Final state will be 'Complete', only if all child jobs completed successfully. + JobState finalState = JobState.Completed; + foreach (var childJob in ChildJobs) + { + if (childJob.JobStateInfo.State != JobState.Completed) + { + finalState = JobState.Failed; + break; + } + } + SetJobState(finalState); + } + catch (ObjectDisposedException) { } + }; + } + + #endregion + + #region Overrides + + /// + /// Location + /// + public override string Location + { + get { return "PowerShell"; } + } + + /// + /// HasMoreData + /// + public override bool HasMoreData + { + get + { + foreach (var childJob in ChildJobs) { - _dataStreamWriter.Close(); + if (childJob.HasMoreData) + { + return true; + } } + + return false; + } + } + + /// + /// StatusMessage + /// + public override string StatusMessage + { + get { return string.Empty; } + } + + /// + /// StopJob + /// + public override void StopJob() + { + _stopSignaled = true; + SetJobState(JobState.Stopping); + + _taskPool.StopAll(); + SetJobState(JobState.Stopped); + } + + /// + /// Dispose + /// + protected override void Dispose(bool disposing) + { + if (disposing) + { + _taskPool.Dispose(); + } + + base.Dispose(disposing); + } + + #endregion + + #region Public Methods + + /// + /// Add a child job to the collection. + /// + public bool AddJob(PSTaskChildJob childJob) + { + if (! _isOpen) + { + return false; + } + + ChildJobs.Add(childJob); + return true; + } + + /// + /// Closes this parent job to adding more child jobs and starts + /// the child jobs running with the provided throttle limit. + /// + public void Start() + { + _isOpen = false; + SetJobState(JobState.Running); + + // Submit jobs to the task pool, blocking when throttle limit is reached. + // This thread will end once all jobs reach a finished state by either running + // to completion, terminating with error, or stopped. + System.Threading.ThreadPool.QueueUserWorkItem( + (state) => + { + foreach (var childJob in ChildJobs) + { + _taskPool.Add((PSTaskChildJob) childJob); + } + _taskPool.Close(); + } + ); + } + + #endregion + } + + + /// + /// Task child job that wraps asynchronously running tasks + /// + internal sealed class PSTaskChildJob : Job + { + #region Members + + private readonly PSTask _task; + + #endregion + + #region Constructor + + private PSTaskChildJob() { } + + public PSTaskChildJob( + ScriptBlock scriptBlock, + Dictionary usingValuesMap, + object dollarUnderbar) + : base(scriptBlock.ToString(), string.Empty) + + { + PSJobTypeName = nameof(PSTaskChildJob); + _task = new PSTask(scriptBlock, usingValuesMap, dollarUnderbar, this); + _task.StateChanged += (sender, args) => HandleTaskStateChange(sender, args); + } + + #endregion + + #region Properties + + internal PSTask Task + { + get { return _task; } + } + + #endregion + + #region Overrides + + /// + /// Location + /// + public override string Location + { + get { return "PowerShell"; } + } + + /// + /// HasMoreData + /// + public override bool HasMoreData + { + get + { + return (this.Output.Count > 0 || + this.Error.Count > 0 || + this.Progress.Count > 0 || + this.Verbose.Count > 0 || + this.Debug.Count > 0 || + this.Warning.Count > 0 || + this.Information.Count > 0); + } + } + + /// + /// StatusMessage + /// + public override string StatusMessage + { + get { return string.Empty; } + } + + /// + /// StopJob + /// + public override void StopJob() + { + _task.SignalStop(); + } + + #endregion + + #region Private Methods + + private void HandleTaskStateChange(object sender, PSInvocationStateChangedEventArgs args) + { + var stateInfo = args.InvocationStateInfo; + + switch (stateInfo.State) + { + case PSInvocationState.Running: + SetJobState(JobState.Running); + break; + + case PSInvocationState.Stopped: + SetJobState(JobState.Stopped, stateInfo.Reason); + break; + + case PSInvocationState.Failed: + SetJobState(JobState.Failed, stateInfo.Reason); + break; + + case PSInvocationState.Completed: + SetJobState(JobState.Completed, stateInfo.Reason); + break; } } diff --git a/src/System.Management.Automation/resources/InternalCommandStrings.resx b/src/System.Management.Automation/resources/InternalCommandStrings.resx index 669f8e8e14f..5b84636f77a 100644 --- a/src/System.Management.Automation/resources/InternalCommandStrings.resx +++ b/src/System.Management.Automation/resources/InternalCommandStrings.resx @@ -174,4 +174,7 @@ A ForEach-Object -Parallel piped input object cannot be a script block. Script blocks have undefined behavior when used in parallel. + + The 'TimeoutSeconds' parameter cannot be used with the 'AsJob' parameter. + From 16140c991d53d228a0a207db6a1c8ededb8d5f3e Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Wed, 24 Jul 2019 13:58:03 -0700 Subject: [PATCH 09/27] Add AsJob tests --- .../Foreach-Object-Parallel.Tests.ps1 | 193 +++++++++++++++++- 1 file changed, 186 insertions(+), 7 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 6a35edaee2b..9ad0d050e8c 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -27,13 +27,6 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { $result[1] | Should -BeExactly $varArray[1] } - It 'Verifies non-terminating error streaming' { - - $expectedError = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Error "Error!" } 2>&1 - $expectedError.ToString() | Should -BeExactly 'Error!' - $expectedError.FullyQualifiedErrorId | Should -BeExactly 'Microsoft.PowerShell.Commands.WriteErrorException' - } - It 'Verifies terminating error streaming' { $result = 1..1 | ForEach-Object -Parallel -ScriptBlock { throw 'Terminating Error!'; "Hello" } 2>&1 @@ -42,6 +35,29 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { $result.FullyQualifiedErrorId | Should -BeExactly 'PSTaskException' } + It 'Verifies terminating error in multiple iterations' { + + $results = 1..2 | ForEach-Object -Parallel -ScriptBlock { + if ($_ -eq 1) { + throw 'Terminating Error!' + "Hello!" + } + else { + "Goodbye!" + } + } 2>&1 + + $results | Should -Not -Contain "Hello!" + $results | Should -Contain "Goodbye!" + } + + It 'Verifies non-terminating error streaming' { + + $expectedError = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Error "Error!" } 2>&1 + $expectedError.ToString() | Should -BeExactly 'Error!' + $expectedError.FullyQualifiedErrorId | Should -BeExactly 'Microsoft.PowerShell.Commands.WriteErrorException' + } + It 'Verifies warning data streaming' { $expectedWarning = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Warning "Warning!" } 3>&1 @@ -77,8 +93,171 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { } } +Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { + + It 'Verifies TimeoutSeconds parameter is excluded from AsJob' { + + { 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello" } -TimeoutSeconds 60 } | Should -Throw -ErrorId 'ParallelCannotUseTimeoutWithJob,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } + + It 'Verifies ForEach-Object -Parallel jobs appear in job repository' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello" } + Get-Job | Should -Contain $job + $job | Wait-Job | Remove-Job + } + + It 'Verifies dollar underbar variable' { + + $expected = 1..10 + $job = $expected | ForEach-Object -Parallel -AsJob -ScriptBlock { $_ } + $result = $job | Wait-Job | Receive-Job + $job | Remove-Job + $result.Count | Should -BeExactly $expected.Count + $result | Should -Contain 1 + $result | Should -Contain 10 + } + + It 'Verifies using variables' { + + $Var1 = "Hello" + $Var2 = "Goodbye" + $Var3 = 105 + $Var4 = "One","Two","Three" + $job = 1..1 | Foreach-Object -Parallel -AsJob -ScriptBlock { + Write-Output $using:Var1 + Write-Output $using:Var2 + Write-Output $using:Var3 + Write-Output @(,$using:Var4) + Write-Output $using:Var4[1] + } + $results = $job | Wait-Job | Receive-Job + $job | Remove-Job + + $results[0] | Should -BeExactly $Var1 + $results[1] | Should -BeExactly $Var2 + $results[2] | Should -BeExactly $Var3 + $results[3] | Should -BeExactly $Var4 + $results[4] | Should -BeExactly $Var4[1] + } + + It 'Verifies terminating error in single iteration' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { throw "Terminating Error!"; "Hello" } + $results = $job | Wait-Job | Receive-Job 2>$null + $results.Count | Should -BeExactly 0 + $job.State | Should -BeExactly 'Failed' + $job.ChildJobs[0].JobStateInfo.State | Should -BeExactly 'Failed' + $job.ChildJobs[0].JobStateInfo.Reason.Message | Should -BeExactly 'Terminating Error!' + $job | Remove-Job + } + + It 'Verifies terminating error in double iteration' { + + $job = 1..2 | ForEach-Object -Parallel -AsJob -ScriptBlock { + if ($_ -eq 1) { + throw "Terminating Error!" + "Goodbye!" + } + else { + "Hello!" + } + } + $results = $job | Wait-Job | Receive-Job 2>$null + $results | Should -Contain 'Hello!' + $results | Should -Not -Contain 'Goodbye!' + $job.JobStateInfo.State | Should -BeExactly 'Failed' + $job.ChildJobs[0].JobStateInfo.State | Should -BeExactly 'Failed' + $job.ChildJobs[0].JobStateInfo.Reason.Message | Should -BeExactly 'Terminating Error!' + $job.ChildJobs[1].JobStateInfo.State | Should -BeExactly 'Completed' + $job | Remove-Job + } + + It 'Verifies non-terminating error' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Error "Error:$_" } + $results = $job | Wait-Job | Receive-Job 2>&1 + $job | Remove-Job + $results.ToString() | Should -BeExactly "Error:1" + } + + It 'Verifies warning data' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Warning "Warning:$_" } + $results = $job | Wait-Job | Receive-Job 3>&1 + $job | Remove-Job + $results.Message | Should -BeExactly "Warning:1" + } + + It 'Verifies verbose data' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Verbose "Verbose:$_" -Verbose } + $results = $job | Wait-Job | Receive-Job -Verbose 4>&1 + $job | Remove-Job + $results.Message | Should -BeExactly "Verbose:1" + } + + It 'Verifies debug data' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Debug "Debug:$_" -Debug } + $results = $job | Wait-Job | Receive-Job -Debug 5>&1 + $job | Remove-Job + $results.Message | Should -BeExactly "Debug:1" + } + + It 'Verifies information data' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Information "Information:$_" } + $results = $job | Wait-Job | Receive-Job 6>&1 + $job | Remove-Job + $results.MessageData | Should -BeExactly "Information:1" + } +} + Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { + It 'Verifies job queuing and throttle limit' { + + # Run four job tasks, two in parallel at a time. + $job = 1..4 | ForEach-Object -Parallel -ScriptBlock { Start-Sleep 60 } -AsJob -ThrottleLimit 2 + + # Wait for child job 2 to begin running for up to ten seconds + $count = 0 + while (($job.ChildJobs[1].JobStateInfo.State -ne 'Running') -and (++$count -lt 40)) + { + Start-Sleep -Milliseconds 250 + } + if ($job.ChildJobs[1].JobStateInfo.State -ne 'Running') + { + throw "ForEach-Object -Parallel child job 2 did not start" + } + + # Two job tasks should be running and two waiting to run + $job.ChildJobs[0].JobStateInfo.State | Should -BeExactly 'Running' + $job.ChildJobs[1].JobStateInfo.State | Should -BeExactly 'Running' + $job.ChildJobs[2].JobStateInfo.State | Should -BeExactly 'NotStarted' + $job.ChildJobs[3].JobStateInfo.State | Should -BeExactly 'NotStarted' + + $job | Remove-Job -Force + } + + It 'Verifies jobs work with Receive-Job -AutoRemove parameter' { + + $job = 1..4 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello:$_" } + $null = $job | Receive-Job -Wait -AutoRemoveJob + Get-Job | Should -Not -Contain $job + } + + It 'Verifies parallel task queuing' { + + $results = 10..1 | ForEach-Object -Parallel -ScriptBlock { Start-Sleep 1; $_ } -ThrottleLimit 5 + $results[0] | Should -BeGreaterThan 5 + $results[1] | Should -BeGreaterThan 5 + $results[2] | Should -BeGreaterThan 5 + $results[3] | Should -BeGreaterThan 5 + $results[4] | Should -BeGreaterThan 5 + } + It 'Verifies timeout and throttle parameters' { # With ThrottleLimit set to 1, the two 60 second long script blocks will run sequentially, From 61e5fd918906f1bc4f34071df482240c35077e9f Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Thu, 25 Jul 2019 10:30:57 -0700 Subject: [PATCH 10/27] Code clean up and a couple new tests --- .../engine/InternalCommands.cs | 4 +- .../engine/hostifaces/PSTask.cs | 517 ++++++++++-------- .../Foreach-Object-Parallel.Tests.ps1 | 14 + 3 files changed, 316 insertions(+), 219 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index e951f378f3e..3107330ca76 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -418,7 +418,9 @@ private void InitParallelParameterSet() ); } - _taskJob = new PSTaskJob(ThrottleLimit); + _taskJob = new PSTaskJob( + ScriptBlock.ToString(), + ThrottleLimit); } else { diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index aca1af27bd5..a6a9d412d42 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -15,187 +15,43 @@ namespace System.Management.Automation.PSTasks #region PSTask /// - /// Class to encapsulate running a PowerShell script concurrently in a cmdlet or job context + /// Class to encapsulate synchronous running scripts in parallel /// - internal sealed class PSTask : IDisposable + internal sealed class PSTask : PSTaskBase { #region Members - private readonly ScriptBlock _scriptBlockToRun; - private readonly Dictionary _usingValuesMap; - private readonly object _dollarUnderbar; private readonly PSTaskDataStreamWriter _dataStreamWriter; - private readonly Job _job; - private readonly int _id; - - private Runspace _runspace; - private PowerShell _powershell; - private PSDataCollection _output; - - private const string VERBATIM_ARGUMENT = "--%"; - - private static int s_taskId = 0; - private const string RunspaceName = "PSTask"; - - #endregion - - #region Events - - /// - /// Event that fires when the task running state changes - /// - public event EventHandler StateChanged; - - #endregion - - #region Properties - - /// - /// Current running state of the task - /// - public PSInvocationState State - { - get - { - PowerShell ps = _powershell; - if (ps != null) - { - return ps.InvocationStateInfo.State; - } - - return PSInvocationState.NotStarted; - } - } - - /// - /// Task Id - /// - public int Id - { - get { return _id; } - } #endregion #region Constructor - private PSTask() - { - _id = Interlocked.Increment(ref s_taskId); - } - /// - /// Constructor for data streaming + /// Constructor /// public PSTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, object dollarUnderbar, - PSTaskDataStreamWriter dataStreamWriter) : this() + PSTaskDataStreamWriter dataStreamWriter) + : base( + scriptBlock, + usingValuesMap, + dollarUnderbar) { - _scriptBlockToRun = scriptBlock; - _usingValuesMap = usingValuesMap; - _dollarUnderbar = dollarUnderbar; _dataStreamWriter = dataStreamWriter; } - /// - /// Constructor for jobs - /// - public PSTask( - ScriptBlock scriptBlock, - Dictionary usingValuesMap, - object dollarUnderbar, - Job job) : this() - { - _scriptBlockToRun = scriptBlock; - _usingValuesMap = usingValuesMap; - _dollarUnderbar = dollarUnderbar; - _job = job; - } - - #endregion - - #region IDisposable - - public void Dispose() - { - _runspace.Dispose(); - _powershell.Dispose(); - _output.Dispose(); - } - #endregion - #region Public Methods - - /// - /// Start task - /// - public void Start() - { - if (_powershell != null) - { - Dbg.Assert(false, "A PSTask can be started only once."); - return; - } - - // Create and open Runspace for this task to run in - var iss = InitialSessionState.CreateDefault2(); - iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) - ? PSLanguageMode.ConstrainedLanguage : PSLanguageMode.FullLanguage; - _runspace = RunspaceFactory.CreateRunspace(iss); - _runspace.Name = string.Format(CultureInfo.InvariantCulture, "{0}:{1}", RunspaceName, s_taskId); - _runspace.Open(); - - // Create the PowerShell command pipeline for the provided script block - // The script will run on the provided Runspace in a new thread by default - _powershell = PowerShell.Create(); - _powershell.Runspace = _runspace; - - // Initialize PowerShell object data streams and event handlers - _output = new PSDataCollection(); - if (_dataStreamWriter != null) - { - InitializePowerShellforDataStreaming(); - } - else - { - InitializePowerShellforJobs(); - } - - // State change handler - _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(sender, args); - - // Start the script running in a new thread - _powershell.AddScript(_scriptBlockToRun.ToString()); - _powershell.Commands.Commands[0].DollarUnderbar = _dollarUnderbar; - if (_usingValuesMap != null && _usingValuesMap.Count > 0) - { - _powershell.AddParameter(VERBATIM_ARGUMENT, _usingValuesMap); - } - _powershell.BeginInvoke(null, _output); - } + #region Overrides /// - /// Signals the running task to stop + /// Initialize PowerShell object /// - public void SignalStop() + protected override void InitializePowershell() { - if (_powershell != null) - { - _powershell.BeginStop(null, null); - } - } - - #endregion - - #region Private Methods - - private void InitializePowerShellforDataStreaming() - { - Dbg.Assert(_dataStreamWriter != null, "Data stream writer cannot be null"); - // Writer data stream handlers _output.DataAdded += (sender, args) => HandleOutputData(sender, args); _powershell.Streams.Error.DataAdded += (sender, args) => HandleErrorData(sender, args); @@ -203,22 +59,12 @@ private void InitializePowerShellforDataStreaming() _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleVerboseData(sender, args); _powershell.Streams.Debug.DataAdded += (sender, args) => HandleDebugData(sender, args); _powershell.Streams.Information.DataAdded += (sender, args) => HandleInformationData(sender, args); - } - - private void InitializePowerShellforJobs() - { - Dbg.Assert(_job != null, "Job object cannot be null"); - // Job data stream handlers - _output.DataAdded += (sender, args) => HandleJobOutputData(sender, args); - _powershell.Streams.Error.DataAdded += (sender, args) => HandleJobErrorData(sender, args); - _powershell.Streams.Warning.DataAdded += (sender, args) => HandleJobWarningData(sender, args); - _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleJobVerboseData(sender, args); - _powershell.Streams.Debug.DataAdded += (sender, args) => HandleJobDebugData(sender, args); - _powershell.Streams.Information.DataAdded += (sender, args) => HandleJobInformationData(sender, args); + // State change handler + _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(sender, args); } - #region Event handlers + #endregion #region Writer data stream handlers @@ -281,6 +127,85 @@ private void HandleInformationData(object sender, DataAddedEventArgs args) ); } } + + #endregion + + #region Event handlers + + private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) + { + if (_dataStreamWriter != null) + { + // Treat any terminating exception as a non-terminating error record + var newStateInfo = stateChangeInfo.InvocationStateInfo; + if (newStateInfo.Reason != null) + { + var errorRecord = new ErrorRecord( + newStateInfo.Reason, + "PSTaskException", + ErrorCategory.InvalidOperation, + this); + + _dataStreamWriter.Add( + new PSStreamObject(PSStreamObjectType.Error, errorRecord) + ); + } + } + + RaiseStateChangedEvent(stateChangeInfo); + } + + #endregion + } + + /// + /// Class to encapsulate asynchronous running scripts in parallel as jobs + /// + internal sealed class PSJobTask : PSTaskBase + { + #region Members + + private readonly Job _job; + + #endregion + + #region Constructor + + /// + /// Constructor + /// + public PSJobTask( + ScriptBlock scriptBlock, + Dictionary usingValuesMap, + object dollarUnderbar, + Job job) : base( + scriptBlock, + usingValuesMap, + dollarUnderbar) + { + _job = job; + } + + #endregion + + #region Overrides + + /// + /// Initialize PowerShell object + /// + protected override void InitializePowershell() + { + // Job data stream handlers + _output.DataAdded += (sender, args) => HandleJobOutputData(sender, args); + _powershell.Streams.Error.DataAdded += (sender, args) => HandleJobErrorData(sender, args); + _powershell.Streams.Warning.DataAdded += (sender, args) => HandleJobWarningData(sender, args); + _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleJobVerboseData(sender, args); + _powershell.Streams.Debug.DataAdded += (sender, args) => HandleJobDebugData(sender, args); + _powershell.Streams.Information.DataAdded += (sender, args) => HandleJobInformationData(sender, args); + + // State change handler + _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(sender, args); + } #endregion @@ -354,31 +279,174 @@ private void HandleJobInformationData(object sender, DataAddedEventArgs args) #endregion + #region Event handlers + private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) { - if (_dataStreamWriter != null) + RaiseStateChangedEvent(stateChangeInfo); + } + + #endregion + } + + /// + /// Base class to encapsulate running a PowerShell script concurrently in a cmdlet or job context + /// + internal abstract class PSTaskBase : IDisposable + { + #region Members + + private readonly ScriptBlock _scriptBlockToRun; + private readonly Dictionary _usingValuesMap; + private readonly object _dollarUnderbar; + private readonly int _id; + private Runspace _runspace; + protected PowerShell _powershell; + protected PSDataCollection _output; + + private const string VERBATIM_ARGUMENT = "--%"; + private const string RunspaceName = "PSTask"; + + private static int s_taskId = 0; + + #endregion + + #region Events + + /// + /// Event that fires when the task running state changes + /// + public event EventHandler StateChanged; + + internal void RaiseStateChangedEvent(PSInvocationStateChangedEventArgs args) + { + StateChanged.SafeInvoke(this, args); + } + + #endregion + + #region Properties + + /// + /// Current running state of the task + /// + public PSInvocationState State + { + get { - // Treat any terminating exception as a non-terminating error record - var newStateInfo = stateChangeInfo.InvocationStateInfo; - if (newStateInfo.Reason != null) + PowerShell ps = _powershell; + if (ps != null) { - var errorRecord = new ErrorRecord( - newStateInfo.Reason, - "PSTaskException", - ErrorCategory.InvalidOperation, - this); - - _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Error, errorRecord) - ); + return ps.InvocationStateInfo.State; } + + return PSInvocationState.NotStarted; } + } - StateChanged.Invoke(this, stateChangeInfo); + /// + /// Task Id + /// + public int Id + { + get { return _id; } } #endregion + #region Constructor + + private PSTaskBase() + { + _id = Interlocked.Increment(ref s_taskId); + } + + /// + /// Constructor + /// + public PSTaskBase( + ScriptBlock scriptBlock, + Dictionary usingValuesMap, + object dollarUnderbar) : this() + { + _scriptBlockToRun = scriptBlock; + _usingValuesMap = usingValuesMap; + _dollarUnderbar = dollarUnderbar; + } + + #endregion + + #region Abstract Methods + + /// + /// Initialize PowerShell object + /// + protected abstract void InitializePowershell(); + + #endregion + + #region IDisposable + + public void Dispose() + { + _runspace.Dispose(); + _powershell.Dispose(); + _output.Dispose(); + } + + #endregion + + #region Public Methods + + /// + /// Start task + /// + public void Start() + { + if (_powershell != null) + { + Dbg.Assert(false, "A PSTask can be started only once."); + return; + } + + // Create and open Runspace for this task to run in + var iss = InitialSessionState.CreateDefault2(); + iss.LanguageMode = (SystemPolicy.GetSystemLockdownPolicy() == SystemEnforcementMode.Enforce) + ? PSLanguageMode.ConstrainedLanguage : PSLanguageMode.FullLanguage; + _runspace = RunspaceFactory.CreateRunspace(iss); + _runspace.Name = string.Format(CultureInfo.InvariantCulture, "{0}:{1}", RunspaceName, s_taskId); + _runspace.Open(); + + // Create the PowerShell command pipeline for the provided script block + // The script will run on the provided Runspace in a new thread by default + _powershell = PowerShell.Create(); + _powershell.Runspace = _runspace; + + // Initialize PowerShell object data streams and event handlers + _output = new PSDataCollection(); + InitializePowershell(); + + // Start the script running in a new thread + _powershell.AddScript(_scriptBlockToRun.ToString()); + _powershell.Commands.Commands[0].DollarUnderbar = _dollarUnderbar; + if (_usingValuesMap != null && _usingValuesMap.Count > 0) + { + _powershell.AddParameter(VERBATIM_ARGUMENT, _usingValuesMap); + } + _powershell.BeginInvoke(null, _output); + } + + /// + /// Signals the running task to stop + /// + public void SignalStop() + { + if (_powershell != null) + { + _powershell.BeginStop(null, null); + } + } + #endregion } @@ -387,7 +455,7 @@ private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs #region PSTaskDataStreamWriter /// - /// Class that handles writing data stream objects to a cmdlet + /// Class that handles writing task data stream objects to a cmdlet /// internal sealed class PSTaskDataStreamWriter : IDisposable { @@ -511,7 +579,7 @@ internal sealed class PSTaskPool : IDisposable private readonly object _syncObject; private readonly ManualResetEvent _addAvailable; private readonly ManualResetEvent _stopAll; - private readonly Dictionary _taskPool; + private readonly Dictionary _taskPool; #endregion @@ -529,7 +597,7 @@ public PSTaskPool(int size) _syncObject = new object(); _addAvailable = new ManualResetEvent(true); _stopAll = new ManualResetEvent(false); - _taskPool = new Dictionary(size); + _taskPool = new Dictionary(size); } #endregion @@ -572,10 +640,10 @@ public void Dispose() /// /// Method to add a task to the pool. - /// If the pool is full, then this method blocks until room is available. + /// If the pool is full, then this method blocks until space is available. /// This method is not multi-thread safe and assumes only one thread waits and adds tasks. /// - public bool Add(PSTask task) + public bool Add(PSTaskBase task) { if (! _isOpen) { @@ -656,8 +724,8 @@ public void Close() private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEventArgs args) { - var task = sender as PSTask; - Dbg.Assert(task != null, "State changed sender must always be PSTask"); + var task = sender as PSTaskBase; + Dbg.Assert(task != null, "State changed sender must always be PSTaskBase"); var stateInfo = args.InvocationStateInfo; switch (stateInfo.State) { @@ -725,36 +793,15 @@ internal sealed class PSTaskJob : Job private PSTaskJob() { } - public PSTaskJob(int throttleLimit) : base(string.Empty, string.Empty) + public PSTaskJob( + string command, + int throttleLimit) : base(command, string.Empty) { _taskPool = new PSTaskPool(throttleLimit); _isOpen = true; PSJobTypeName = nameof(PSTaskJob); - _taskPool.PoolComplete += (sender, args) => - { - try - { - if (_stopSignaled) - { - SetJobState(JobState.Stopped, new PipelineStoppedException()); - return; - } - - // Final state will be 'Complete', only if all child jobs completed successfully. - JobState finalState = JobState.Completed; - foreach (var childJob in ChildJobs) - { - if (childJob.JobStateInfo.State != JobState.Completed) - { - finalState = JobState.Failed; - break; - } - } - SetJobState(finalState); - } - catch (ObjectDisposedException) { } - }; + _taskPool.PoolComplete += (sender, args) => HandleTaskPoolComplete(sender, args); } #endregion @@ -864,9 +911,37 @@ public void Start() } #endregion + + #region Private Methods + + private void HandleTaskPoolComplete(object sender, EventArgs args) + { + try + { + if (_stopSignaled) + { + SetJobState(JobState.Stopped, new PipelineStoppedException()); + return; + } + + // Final state will be 'Complete', only if all child jobs completed successfully. + JobState finalState = JobState.Completed; + foreach (var childJob in ChildJobs) + { + if (childJob.JobStateInfo.State != JobState.Completed) + { + finalState = JobState.Failed; + break; + } + } + SetJobState(finalState); + } + catch (ObjectDisposedException) { } + } + + #endregion } - /// /// Task child job that wraps asynchronously running tasks /// @@ -874,7 +949,7 @@ internal sealed class PSTaskChildJob : Job { #region Members - private readonly PSTask _task; + private readonly PSJobTask _task; #endregion @@ -882,6 +957,9 @@ internal sealed class PSTaskChildJob : Job private PSTaskChildJob() { } + /// + /// Constructor + /// public PSTaskChildJob( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -890,7 +968,7 @@ public PSTaskChildJob( { PSJobTypeName = nameof(PSTaskChildJob); - _task = new PSTask(scriptBlock, usingValuesMap, dollarUnderbar, this); + _task = new PSJobTask(scriptBlock, usingValuesMap, dollarUnderbar, this); _task.StateChanged += (sender, args) => HandleTaskStateChange(sender, args); } @@ -898,7 +976,10 @@ public PSTaskChildJob( #region Properties - internal PSTask Task + /// + /// Child job task + /// + internal PSTaskBase Task { get { return _task; } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 9ad0d050e8c..fc3791795c9 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -91,6 +91,12 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { { $sb | ForEach-Object -Parallel -ScriptBlock { "Hello" } -ErrorAction Stop } | Should -Throw -ErrorId 'ParallelPipedInputObjectCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' } + + It 'Verifies that parallel script blocks run in FullLanguage mode by default' { + + $results = 1..1 | ForEach-Object -Parallel -ScriptBlock { $ExecutionContext.SessionState.LanguageMode } + $results | Should -BeExactly 'FullLanguage' + } } Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { @@ -212,6 +218,14 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { $job | Remove-Job $results.MessageData | Should -BeExactly "Information:1" } + + It 'Verifies job Command property' { + + $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock {"Hello"} + $job.Command | Should -BeExactly '"Hello"' + $job.ChildJobs[0].Command | Should -BeExactly '"Hello"' + $job | Remove-Job + } } Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { From 675aea2872f696f6b1e0157886284b904c28b810 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Thu, 25 Jul 2019 14:09:34 -0700 Subject: [PATCH 11/27] Fix test. Move event callback outside lock. --- .../engine/hostifaces/PSTask.cs | 27 ++++++++++--------- .../Foreach-Object-Parallel.Tests.ps1 | 2 +- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index a6a9d412d42..f2bcb760f68 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -750,21 +750,24 @@ private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEvent private void CheckForComplete() { + bool isTaskPoolComplete; lock (_syncObject) { - if (!_isOpen && _taskPool.Count == 0) + isTaskPoolComplete = (!_isOpen && _taskPool.Count == 0); + } + + if (isTaskPoolComplete) + { + try { - try - { - PoolComplete.SafeInvoke( - this, - new EventArgs() - ); - } - catch - { - Dbg.Assert(false, "Exceptions should not be thrown on event thread"); - } + PoolComplete.SafeInvoke( + this, + new EventArgs() + ); + } + catch + { + Dbg.Assert(false, "Exceptions should not be thrown on event thread"); } } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index fc3791795c9..a998864fe17 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -224,7 +224,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock {"Hello"} $job.Command | Should -BeExactly '"Hello"' $job.ChildJobs[0].Command | Should -BeExactly '"Hello"' - $job | Remove-Job + $job | Wait-Job | Remove-Job } } From 44e4e7dd5946e21aa31977fa2affb38172eadb1c Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Thu, 25 Jul 2019 15:59:41 -0700 Subject: [PATCH 12/27] Add skip when experimental feature is not enabled --- .../ConstrainedLanguageRestriction.Tests.ps1 | 17 +++++++ .../Foreach-Object-Parallel.Tests.ps1 | 51 ++++++++++++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 index 91e0952c8b0..4df954e4d07 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 @@ -880,6 +880,23 @@ try Describe "ForEach-Object -Parallel Constrained Language Tests" -Tags 'Feature','RequireAdminOnWindows' { + BeforeAll { + + $skipTest = -not $EnabledExperimentalFeatures.Contains('PSForEachObjectParallel') + if ($skipTest) { + Write-Verbose "Test Suite Skipped. The test suite requires the experimental feature 'PSForEachObjectParallel' to be enabled." -Verbose + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + $PSDefaultParameterValues["it:skip"] = $true + } + } + + AfterAll { + + if ($skipTest) { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + } + } + It 'Foreach-Object -Parallel must run in ConstrainedLanguage mode under system lock down' { try diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index a998864fe17..a53483f7632 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -5,7 +5,22 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { BeforeAll { - $sb = { "Hello!" } + $skipTest = -not $EnabledExperimentalFeatures.Contains('PSForEachObjectParallel') + if ($skipTest) { + Write-Verbose "Test Suite Skipped. The test suite requires the experimental feature 'PSForEachObjectParallel' to be enabled." -Verbose + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + $PSDefaultParameterValues["it:skip"] = $true + } + else { + $sb = { "Hello!" } + } + } + + AfterAll { + + if ($skipTest) { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + } } It "Verifies dollar underbar variable" { @@ -101,6 +116,23 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { + BeforeAll { + + $skipTest = -not $EnabledExperimentalFeatures.Contains('PSForEachObjectParallel') + if ($skipTest) { + Write-Verbose "Test Suite Skipped. The test suite requires the experimental feature 'PSForEachObjectParallel' to be enabled." -Verbose + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + $PSDefaultParameterValues["it:skip"] = $true + } + } + + AfterAll { + + if ($skipTest) { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + } + } + It 'Verifies TimeoutSeconds parameter is excluded from AsJob' { { 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello" } -TimeoutSeconds 60 } | Should -Throw -ErrorId 'ParallelCannotUseTimeoutWithJob,Microsoft.PowerShell.Commands.ForEachObjectCommand' @@ -230,6 +262,23 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { + BeforeAll { + + $skipTest = -not $EnabledExperimentalFeatures.Contains('PSForEachObjectParallel') + if ($skipTest) { + Write-Verbose "Test Suite Skipped. The test suite requires the experimental feature 'PSForEachObjectParallel' to be enabled." -Verbose + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + $PSDefaultParameterValues["it:skip"] = $true + } + } + + AfterAll { + + if ($skipTest) { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + } + } + It 'Verifies job queuing and throttle limit' { # Run four job tasks, two in parallel at a time. From d26aff32875486a9daaca97aec9bd408540b86d3 Mon Sep 17 00:00:00 2001 From: PaulHigin Date: Fri, 26 Jul 2019 13:53:58 -0700 Subject: [PATCH 13/27] Code clean up. Fix taskpool Add block to allow data stream writing during wait. --- .../engine/InternalCommands.cs | 32 ++----- .../engine/hostifaces/PSTask.cs | 90 +++++++++++++------ 2 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index 3107330ca76..3580a8d766b 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -211,11 +211,7 @@ public object[] ArgumentList /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - public SwitchParameter Parallel - { - get; - set; - } + public SwitchParameter Parallel { get; set; } /// /// Script block to run for each pipeline object @@ -223,11 +219,7 @@ public SwitchParameter Parallel [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(Position = 0, Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] [ValidateNotNull()] - public ScriptBlock ScriptBlock - { - get; - set; - } + public ScriptBlock ScriptBlock { get; set; } /// /// Specifies the maximum number of concurrently running scriptblocks on separate threads. @@ -236,11 +228,7 @@ public ScriptBlock ScriptBlock [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] [ValidateRange(1, Int32.MaxValue)] - public int ThrottleLimit - { - get; - set; - } = 5; + public int ThrottleLimit { get; set; } = 5; /// /// Specifies a timeout time in seconds, after which the parallel running scripts will be stopped @@ -249,11 +237,7 @@ public int ThrottleLimit [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] [ValidateRange(0, (Int32.MaxValue/1000))] - public int TimeoutSeconds - { - get; - set; - } + public int TimeoutSeconds { get; set; } /// /// Flag that returns a job object immediately for the parallel operation, instead of returning after @@ -261,11 +245,7 @@ public int TimeoutSeconds /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - public SwitchParameter AsJob - { - get; - set; - } + public SwitchParameter AsJob { get; set; } #endregion @@ -479,7 +459,7 @@ private void ProcessParallelParameterSet() // Add task to task pool. // Block if the pool is full and wait until task can be added. - _taskPool.Add(task); + _taskPool.Add(task, _taskDataStreamWriter); } } diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index f2bcb760f68..d4f49c4e80c 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -347,10 +347,7 @@ public PSInvocationState State /// /// Task Id /// - public int Id - { - get { return _id; } - } + public int Id { get { return _id; } } #endregion @@ -467,6 +464,18 @@ internal sealed class PSTaskDataStreamWriter : IDisposable #endregion + #region Properties + + /// + /// Wait-able handle that signals when new data has been added to + /// the data stream collection. + internal WaitHandle DataAddedWaitHandle + { + get { return _dataStream.WaitHandle; } + } + + #endregion + #region Constructor private PSTaskDataStreamWriter() { } @@ -643,43 +652,74 @@ public void Dispose() /// If the pool is full, then this method blocks until space is available. /// This method is not multi-thread safe and assumes only one thread waits and adds tasks. /// - public bool Add(PSTaskBase task) + public bool Add( + PSTaskBase task, + PSTaskDataStreamWriter dataStreamWriter = null) { if (! _isOpen) { return false; } - // Block until either room is available or a stop is commanded - var index = WaitHandle.WaitAny(new WaitHandle[] { - _addAvailable, // index 0 - _stopAll // index 1 - }); - - if (index == 1) + WaitHandle[] waitHandles; + if (dataStreamWriter != null) { - return false; + waitHandles = new WaitHandle[] { + _addAvailable, // index 0 + _stopAll, // index 1 + dataStreamWriter.DataAddedWaitHandle // index 2 + }; + } + else + { + waitHandles = new WaitHandle[] { + _addAvailable, // index 0 + _stopAll, // index 1 + }; } - task.StateChanged += (sender, args) => HandleTaskStateChanged(sender, args); - - lock (_syncObject) + // Block until either room is available, data is ready for writing, or a stop command + while (true) { - if (! _isOpen) + var index = WaitHandle.WaitAny(waitHandles); + + // Add new task + if (index == 0) { - return false; + task.StateChanged += (sender, args) => HandleTaskStateChanged(sender, args); + + lock (_syncObject) + { + if (! _isOpen) + { + return false; + } + + _taskPool.Add(task.Id, task); + if (_taskPool.Count == _sizeLimit) + { + _addAvailable.Reset(); + } + + task.Start(); + } + + return true; } - _taskPool.Add(task.Id, task); - if (_taskPool.Count == _sizeLimit) + // Stop all + if (index == 1) { - _addAvailable.Reset(); + return false; + } + + // Data ready for writing + if (index == 2) + { + dataStreamWriter.WriteImmediate(); + continue; } - - task.Start(); } - - return true; } /// From 17c181b9d3f918eaced3267da0cda7d31c173e4f Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 29 Jul 2019 09:36:24 -0700 Subject: [PATCH 14/27] Initialize dollarUnder variable to correct null value --- .../engine/ScriptCommandProcessor.cs | 2 +- src/System.Management.Automation/engine/hostifaces/Command.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs index f3a40d7fedb..ed1eb7c698e 100644 --- a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs +++ b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs @@ -235,7 +235,7 @@ internal sealed class DlrScriptCommandProcessor : ScriptCommandProcessorBase private MutableTuple _localsTuple; private bool _runOptimizedCode; private bool _argsBound; - private object _dollarUnderbar; + private object _dollarUnderbar = AutomationNull.Value; private FunctionContext _functionContext; internal DlrScriptCommandProcessor(ScriptBlock scriptBlock, ExecutionContext context, bool useNewScope, CommandOrigin origin, SessionStateInternal sessionState, object dollarUnderbar) diff --git a/src/System.Management.Automation/engine/hostifaces/Command.cs b/src/System.Management.Automation/engine/hostifaces/Command.cs index afb06dd5a2e..aafc93a150a 100644 --- a/src/System.Management.Automation/engine/hostifaces/Command.cs +++ b/src/System.Management.Automation/engine/hostifaces/Command.cs @@ -178,7 +178,7 @@ internal bool? UseLocalScopeNullable /// This is used by foreach-object -parallel where each piped input ($_) is associated /// with a parallel running script block. /// - internal object DollarUnderbar { get; set; } + internal object DollarUnderbar { get; set; } = AutomationNull.Value; /// /// Checks if the current command marks the end of a statement (see PowerShell.AddStatement()) From 2ce706711e5c732f0220ebd35ab637bbb1e973b3 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 29 Jul 2019 10:26:14 -0700 Subject: [PATCH 15/27] Some code clean up --- .../engine/ScriptCommandProcessor.cs | 2 +- .../engine/hostifaces/PSTask.cs | 68 +++++++++---------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs index ed1eb7c698e..5b475ee6b98 100644 --- a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs +++ b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs @@ -235,7 +235,7 @@ internal sealed class DlrScriptCommandProcessor : ScriptCommandProcessorBase private MutableTuple _localsTuple; private bool _runOptimizedCode; private bool _argsBound; - private object _dollarUnderbar = AutomationNull.Value; + private readonly object _dollarUnderbar = AutomationNull.Value; private FunctionContext _functionContext; internal DlrScriptCommandProcessor(ScriptBlock scriptBlock, ExecutionContext context, bool useNewScope, CommandOrigin origin, SessionStateInternal sessionState, object dollarUnderbar) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index d4f49c4e80c..b700fa6a223 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -53,22 +53,22 @@ public PSTask( protected override void InitializePowershell() { // Writer data stream handlers - _output.DataAdded += (sender, args) => HandleOutputData(sender, args); - _powershell.Streams.Error.DataAdded += (sender, args) => HandleErrorData(sender, args); - _powershell.Streams.Warning.DataAdded += (sender, args) => HandleWarningData(sender, args); - _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleVerboseData(sender, args); - _powershell.Streams.Debug.DataAdded += (sender, args) => HandleDebugData(sender, args); - _powershell.Streams.Information.DataAdded += (sender, args) => HandleInformationData(sender, args); + _output.DataAdded += (sender, args) => HandleOutputData(); + _powershell.Streams.Error.DataAdded += (sender, args) => HandleErrorData(); + _powershell.Streams.Warning.DataAdded += (sender, args) => HandleWarningData(); + _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleVerboseData(); + _powershell.Streams.Debug.DataAdded += (sender, args) => HandleDebugData(); + _powershell.Streams.Information.DataAdded += (sender, args) => HandleInformationData(); // State change handler - _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(sender, args); + _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(args); } - #endregion + #endregion #region Writer data stream handlers - private void HandleOutputData(object sender, DataAddedEventArgs args) + private void HandleOutputData() { foreach (var item in _output.ReadAll()) { @@ -78,7 +78,7 @@ private void HandleOutputData(object sender, DataAddedEventArgs args) } } - private void HandleErrorData(object sender, DataAddedEventArgs args) + private void HandleErrorData() { foreach (var item in _powershell.Streams.Error.ReadAll()) { @@ -88,7 +88,7 @@ private void HandleErrorData(object sender, DataAddedEventArgs args) } } - private void HandleWarningData(object sender, DataAddedEventArgs args) + private void HandleWarningData() { foreach (var item in _powershell.Streams.Warning.ReadAll()) { @@ -98,7 +98,7 @@ private void HandleWarningData(object sender, DataAddedEventArgs args) } } - private void HandleVerboseData(object sender, DataAddedEventArgs args) + private void HandleVerboseData() { foreach (var item in _powershell.Streams.Verbose.ReadAll()) { @@ -108,7 +108,7 @@ private void HandleVerboseData(object sender, DataAddedEventArgs args) } } - private void HandleDebugData(object sender, DataAddedEventArgs args) + private void HandleDebugData() { foreach (var item in _powershell.Streams.Debug.ReadAll()) { @@ -118,7 +118,7 @@ private void HandleDebugData(object sender, DataAddedEventArgs args) } } - private void HandleInformationData(object sender, DataAddedEventArgs args) + private void HandleInformationData() { foreach (var item in _powershell.Streams.Information.ReadAll()) { @@ -132,7 +132,7 @@ private void HandleInformationData(object sender, DataAddedEventArgs args) #region Event handlers - private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) + private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInfo) { if (_dataStreamWriter != null) { @@ -196,22 +196,22 @@ public PSJobTask( protected override void InitializePowershell() { // Job data stream handlers - _output.DataAdded += (sender, args) => HandleJobOutputData(sender, args); - _powershell.Streams.Error.DataAdded += (sender, args) => HandleJobErrorData(sender, args); - _powershell.Streams.Warning.DataAdded += (sender, args) => HandleJobWarningData(sender, args); - _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleJobVerboseData(sender, args); - _powershell.Streams.Debug.DataAdded += (sender, args) => HandleJobDebugData(sender, args); - _powershell.Streams.Information.DataAdded += (sender, args) => HandleJobInformationData(sender, args); + _output.DataAdded += (sender, args) => HandleJobOutputData(); + _powershell.Streams.Error.DataAdded += (sender, args) => HandleJobErrorData(); + _powershell.Streams.Warning.DataAdded += (sender, args) => HandleJobWarningData(); + _powershell.Streams.Verbose.DataAdded += (sender, args) => HandleJobVerboseData(); + _powershell.Streams.Debug.DataAdded += (sender, args) => HandleJobDebugData(); + _powershell.Streams.Information.DataAdded += (sender, args) => HandleJobInformationData(); // State change handler - _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(sender, args); + _powershell.InvocationStateChanged += (sender, args) => HandleStateChanged(args); } #endregion #region Job data stream handlers - private void HandleJobOutputData(object sender, DataAddedEventArgs args) + private void HandleJobOutputData() { foreach (var item in _output.ReadAll()) { @@ -222,7 +222,7 @@ private void HandleJobOutputData(object sender, DataAddedEventArgs args) } } - private void HandleJobErrorData(object sender, DataAddedEventArgs args) + private void HandleJobErrorData() { foreach (var item in _powershell.Streams.Error.ReadAll()) { @@ -233,7 +233,7 @@ private void HandleJobErrorData(object sender, DataAddedEventArgs args) } } - private void HandleJobWarningData(object sender, DataAddedEventArgs args) + private void HandleJobWarningData() { foreach (var item in _powershell.Streams.Warning.ReadAll()) { @@ -244,7 +244,7 @@ private void HandleJobWarningData(object sender, DataAddedEventArgs args) } } - private void HandleJobVerboseData(object sender, DataAddedEventArgs args) + private void HandleJobVerboseData() { foreach (var item in _powershell.Streams.Verbose.ReadAll()) { @@ -255,7 +255,7 @@ private void HandleJobVerboseData(object sender, DataAddedEventArgs args) } } - private void HandleJobDebugData(object sender, DataAddedEventArgs args) + private void HandleJobDebugData() { foreach (var item in _powershell.Streams.Debug.ReadAll()) { @@ -266,7 +266,7 @@ private void HandleJobDebugData(object sender, DataAddedEventArgs args) } } - private void HandleJobInformationData(object sender, DataAddedEventArgs args) + private void HandleJobInformationData() { foreach (var item in _powershell.Streams.Information.ReadAll()) { @@ -281,7 +281,7 @@ private void HandleJobInformationData(object sender, DataAddedEventArgs args) #region Event handlers - private void HandleStateChanged(object sender, PSInvocationStateChangedEventArgs stateChangeInfo) + private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInfo) { RaiseStateChangedEvent(stateChangeInfo); } @@ -307,7 +307,7 @@ internal abstract class PSTaskBase : IDisposable private const string VERBATIM_ARGUMENT = "--%"; private const string RunspaceName = "PSTask"; - private static int s_taskId = 0; + private static int s_taskId; #endregion @@ -361,7 +361,7 @@ private PSTaskBase() /// /// Constructor /// - public PSTaskBase( + protected PSTaskBase( ScriptBlock scriptBlock, Dictionary usingValuesMap, object dollarUnderbar) : this() @@ -686,7 +686,7 @@ public bool Add( // Add new task if (index == 0) { - task.StateChanged += (sender, args) => HandleTaskStateChanged(sender, args); + task.StateChanged += HandleTaskStateChangedDelegate; lock (_syncObject) { @@ -717,7 +717,6 @@ public bool Add( if (index == 2) { dataStreamWriter.WriteImmediate(); - continue; } } } @@ -762,6 +761,7 @@ public void Close() #region Private Methods + private void HandleTaskStateChangedDelegate(object sender, PSInvocationStateChangedEventArgs args) => HandleTaskStateChanged(sender, args); private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEventArgs args) { var task = sender as PSTaskBase; @@ -781,7 +781,7 @@ private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEvent _addAvailable.Set(); } } - task.StateChanged -= (sender, args) => HandleTaskStateChanged(sender, args); + task.StateChanged -= HandleTaskStateChangedDelegate; task.Dispose(); CheckForComplete(); break; From b87c518a1fb754663939ef34e65c625ab81c80ef Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 29 Jul 2019 10:58:08 -0700 Subject: [PATCH 16/27] Fix style issues --- .../engine/InternalCommands.cs | 27 ++++++------- .../engine/hostifaces/PSTask.cs | 40 +++++++------------ 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index 3580a8d766b..b0ebdca803f 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -207,22 +207,22 @@ public object[] ArgumentList #region ParallelParameterSet /// - /// Flag to indicate that foreach iterations should be run in parallel instead of sequentially. + /// Gets and sets a flag to indicate that foreach iterations should be run in parallel instead of sequentially. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] public SwitchParameter Parallel { get; set; } /// - /// Script block to run for each pipeline object + /// Gets and sets a script block to run for each pipeline object. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(Position = 0, Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - [ValidateNotNull()] + [ValidateNotNull] public ScriptBlock ScriptBlock { get; set; } /// - /// Specifies the maximum number of concurrently running scriptblocks on separate threads. + /// Gets and sets the maximum number of concurrently running scriptblocks on separate threads. /// The default number is 5. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] @@ -231,16 +231,16 @@ public object[] ArgumentList public int ThrottleLimit { get; set; } = 5; /// - /// Specifies a timeout time in seconds, after which the parallel running scripts will be stopped + /// Gets and sets a timeout time in seconds, after which the parallel running scripts will be stopped /// The default value is 0, indicating no timeout. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - [ValidateRange(0, (Int32.MaxValue/1000))] + [ValidateRange(0, (Int32.MaxValue / 1000))] public int TimeoutSeconds { get; set; } /// - /// Flag that returns a job object immediately for the parallel operation, instead of returning after + /// Gets and sets a flag that returns a job object immediately for the parallel operation, instead of returning after /// all foreach processing is completed. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] @@ -367,7 +367,7 @@ public void Dispose() private void InitParallelParameterSet() { - bool allowUsingExpression = (this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage); + bool allowUsingExpression = this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage; _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(ScriptBlock, allowUsingExpression, this.Context, null); // Validate using values map @@ -380,8 +380,7 @@ private void InitParallelParameterSet() new PSArgumentException(InternalCommandStrings.ParallelUsingVariableCannotBeScriptBlock), "ParallelUsingVariableCannotBeScriptBlock", ErrorCategory.InvalidType, - this) - ); + this)); } } @@ -394,8 +393,7 @@ private void InitParallelParameterSet() new PSArgumentException(InternalCommandStrings.ParallelCannotUseTimeoutWithJob), "ParallelCannotUseTimeoutWithJob", ErrorCategory.InvalidOperation, - this) - ); + this)); } _taskJob = new PSTaskJob( @@ -415,7 +413,7 @@ private void InitParallelParameterSet() _taskTimer = new System.Threading.Timer( (_) => _taskPool.StopAll(), null, - (TimeoutSeconds * 1000), + TimeoutSeconds * 1000, System.Threading.Timeout.Infinite); } } @@ -431,8 +429,7 @@ private void ProcessParallelParameterSet() new PSArgumentException(InternalCommandStrings.ParallelPipedInputObjectCannotBeScriptBlock), "ParallelPipedInputObjectCannotBeScriptBlock", ErrorCategory.InvalidType, - this) - ); + this)); return; } diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index b700fa6a223..3444fb84334 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -73,8 +73,7 @@ private void HandleOutputData() foreach (var item in _output.ReadAll()) { _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Output, item) - ); + new PSStreamObject(PSStreamObjectType.Output, item)); } } @@ -83,8 +82,7 @@ private void HandleErrorData() foreach (var item in _powershell.Streams.Error.ReadAll()) { _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Error, item) - ); + new PSStreamObject(PSStreamObjectType.Error, item)); } } @@ -93,8 +91,7 @@ private void HandleWarningData() foreach (var item in _powershell.Streams.Warning.ReadAll()) { _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Warning, item.Message) - ); + new PSStreamObject(PSStreamObjectType.Warning, item.Message)); } } @@ -103,8 +100,7 @@ private void HandleVerboseData() foreach (var item in _powershell.Streams.Verbose.ReadAll()) { _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Verbose, item.Message) - ); + new PSStreamObject(PSStreamObjectType.Verbose, item.Message)); } } @@ -113,8 +109,7 @@ private void HandleDebugData() foreach (var item in _powershell.Streams.Debug.ReadAll()) { _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Debug, item.Message) - ); + new PSStreamObject(PSStreamObjectType.Debug, item.Message)); } } @@ -123,8 +118,7 @@ private void HandleInformationData() foreach (var item in _powershell.Streams.Information.ReadAll()) { _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Information, item) - ); + new PSStreamObject(PSStreamObjectType.Information, item)); } } @@ -147,8 +141,7 @@ private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInf this); _dataStreamWriter.Add( - new PSStreamObject(PSStreamObjectType.Error, errorRecord) - ); + new PSStreamObject(PSStreamObjectType.Error, errorRecord)); } } @@ -217,8 +210,7 @@ private void HandleJobOutputData() { _job.Output.Add(item); _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Output, item) - ); + new PSStreamObject(PSStreamObjectType.Output, item)); } } @@ -228,8 +220,7 @@ private void HandleJobErrorData() { _job.Error.Add(item); _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Error, item) - ); + new PSStreamObject(PSStreamObjectType.Error, item)); } } @@ -239,8 +230,7 @@ private void HandleJobWarningData() { _job.Warning.Add(item); _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Warning, item.Message) - ); + new PSStreamObject(PSStreamObjectType.Warning, item.Message)); } } @@ -250,8 +240,7 @@ private void HandleJobVerboseData() { _job.Verbose.Add(item); _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Verbose, item.Message) - ); + new PSStreamObject(PSStreamObjectType.Verbose, item.Message)); } } @@ -261,8 +250,7 @@ private void HandleJobDebugData() { _job.Debug.Add(item); _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Debug, item.Message) - ); + new PSStreamObject(PSStreamObjectType.Debug, item.Message)); } } @@ -272,8 +260,7 @@ private void HandleJobInformationData() { _job.Information.Add(item); _job.Results.Add( - new PSStreamObject(PSStreamObjectType.Information, item) - ); + new PSStreamObject(PSStreamObjectType.Information, item)); } } @@ -762,6 +749,7 @@ public void Close() #region Private Methods private void HandleTaskStateChangedDelegate(object sender, PSInvocationStateChangedEventArgs args) => HandleTaskStateChanged(sender, args); + private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEventArgs args) { var task = sender as PSTaskBase; From fd158e08c14d3f0c4ca1aee43e5614a2e6a3a91b Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Tue, 30 Jul 2019 08:34:56 -0700 Subject: [PATCH 17/27] Remove ScriptBlock parameter and converted Parallel switch parameter to take ScriptBlock type. Add support for IJobDebugger interface. --- .../engine/InternalCommands.cs | 22 +- .../engine/hostifaces/PSTask.cs | 222 +++++++++++++++++- .../ConstrainedLanguageRestriction.Tests.ps1 | 2 +- .../Foreach-Object-Parallel.Tests.ps1 | 54 ++--- 4 files changed, 256 insertions(+), 44 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index b0ebdca803f..e31bd88ca38 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -207,19 +207,11 @@ public object[] ArgumentList #region ParallelParameterSet /// - /// Gets and sets a flag to indicate that foreach iterations should be run in parallel instead of sequentially. + /// Gets and sets a script block to run in parallel for each pipeline object. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] - [Parameter(ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - public SwitchParameter Parallel { get; set; } - - /// - /// Gets and sets a script block to run for each pipeline object. - /// - [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] - [Parameter(Position = 0, Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] - [ValidateNotNull] - public ScriptBlock ScriptBlock { get; set; } + [Parameter(Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] + public ScriptBlock Parallel { get; set; } /// /// Gets and sets the maximum number of concurrently running scriptblocks on separate threads. @@ -368,7 +360,7 @@ public void Dispose() private void InitParallelParameterSet() { bool allowUsingExpression = this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage; - _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(ScriptBlock, allowUsingExpression, this.Context, null); + _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(Parallel, allowUsingExpression, this.Context, null); // Validate using values map foreach (var item in _usingValuesMap.Values) @@ -397,7 +389,7 @@ private void InitParallelParameterSet() } _taskJob = new PSTaskJob( - ScriptBlock.ToString(), + Parallel.ToString(), ThrottleLimit); } else @@ -437,7 +429,7 @@ private void ProcessParallelParameterSet() if (AsJob) { var taskChildJob = new PSTaskChildJob( - ScriptBlock, + Parallel, _usingValuesMap, InputObject); @@ -449,7 +441,7 @@ private void ProcessParallelParameterSet() _taskDataStreamWriter.WriteImmediate(); var task = new System.Management.Automation.PSTasks.PSTask( - ScriptBlock, + Parallel, _usingValuesMap, InputObject, _taskDataStreamWriter); diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 3444fb84334..7225fdce6e4 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Globalization; +using System.Management.Automation.Host; +using System.Management.Automation.Language; using System.Management.Automation.Remoting.Internal; using System.Management.Automation.Runspaces; using System.Management.Automation.Security; @@ -274,6 +276,21 @@ private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInf } #endregion + + #region Properties + + /// + /// Debugger + /// + public Debugger Debugger + { + get + { + return _powershell.Runspace.Debugger; + } + } + + #endregion } /// @@ -973,14 +990,190 @@ private void HandleTaskPoolComplete(object sender, EventArgs args) #endregion } + /// + /// PSTaskChildJob debugger wrapper + /// + internal sealed class PSTaskChildDebugger : Debugger + { + #region Members + + private Debugger _wrappedDebugger; + private string _jobName; + + #endregion + + #region Constructor + + private PSTaskChildDebugger() { } + + public PSTaskChildDebugger( + Debugger debugger, + string jobName) + { + if (debugger == null) + { + throw new PSArgumentNullException("debugger"); + } + + _wrappedDebugger = debugger; + _jobName = jobName ?? string.Empty; + + // Create handlers for wrapped debugger events. + _wrappedDebugger.BreakpointUpdated += HandleBreakpointUpdated; + _wrappedDebugger.DebuggerStop += HandleDebuggerStop; + } + + #endregion + + #region Debugger overrides + + /// + /// Evaluates provided command either as a debugger specific command + /// or a PowerShell command. + /// + /// PowerShell command. + /// Output. + /// DebuggerCommandResults. + public override DebuggerCommandResults ProcessCommand(PSCommand command, PSDataCollection output) + { + // Special handling for the prompt command. + if (command.Commands[0].CommandText.Trim().Equals("prompt", StringComparison.OrdinalIgnoreCase)) + { + return HandlePromptCommand(output); + } + + return _wrappedDebugger.ProcessCommand(command, output); + } + + /// + /// Adds the provided set of breakpoints to the debugger. + /// + /// Breakpoints. + public override void SetBreakpoints(IEnumerable breakpoints) + { + _wrappedDebugger.SetBreakpoints(breakpoints); + } + + /// + /// Sets the debugger resume action. + /// + /// DebuggerResumeAction. + public override void SetDebuggerAction(DebuggerResumeAction resumeAction) + { + _wrappedDebugger.SetDebuggerAction(resumeAction); + } + + /// + /// Stops a running command. + /// + public override void StopProcessCommand() + { + _wrappedDebugger.StopProcessCommand(); + } + + /// + /// Returns current debugger stop event arguments if debugger is in + /// debug stop state. Otherwise returns null. + /// + /// DebuggerStopEventArgs. + public override DebuggerStopEventArgs GetDebuggerStopArgs() + { + return _wrappedDebugger.GetDebuggerStopArgs(); + } + + /// + /// Sets the parent debugger, breakpoints, and other debugging context information. + /// + /// Parent debugger. + /// List of breakpoints. + /// Debugger mode. + /// PowerShell host. + /// Current path. + public override void SetParent( + Debugger parent, + IEnumerable breakPoints, + DebuggerResumeAction? startAction, + PSHost host, + PathInfo path) + { + // For now always enable step mode debugging. + SetDebuggerStepMode(true); + } + + /// + /// Sets the debugger mode. + /// + public override void SetDebugMode(DebugModes mode) + { + _wrappedDebugger.SetDebugMode(mode); + + base.SetDebugMode(mode); + } + + /// + /// Returns IEnumerable of CallStackFrame objects. + /// + /// + public override IEnumerable GetCallStack() + { + return _wrappedDebugger.GetCallStack(); + } + + /// + /// Sets debugger stepping mode. + /// + /// True if stepping is to be enabled. + public override void SetDebuggerStepMode(bool enabled) + { + _wrappedDebugger.SetDebuggerStepMode(enabled); + } + + /// + /// True when debugger is stopped at a breakpoint. + /// + public override bool InBreakpoint + { + get { return _wrappedDebugger.InBreakpoint; } + } + + #endregion + + #region Private methods + + private void HandleDebuggerStop(object sender, DebuggerStopEventArgs e) + { + this.RaiseDebuggerStopEvent(e); + } + + private void HandleBreakpointUpdated(object sender, BreakpointUpdatedEventArgs e) + { + this.RaiseBreakpointUpdatedEvent(e); + } + + private DebuggerCommandResults HandlePromptCommand(PSDataCollection output) + { + // Nested debugged runspace prompt should look like: + // [DBG]: [JobName]: PS C:\>> + string promptScript = "'[DBG]: '" + " + " + "'[" + CodeGeneration.EscapeSingleQuotedStringContent(_jobName) + "]: '" + " + " + @"""PS $($executionContext.SessionState.Path.CurrentLocation)>> """; + PSCommand promptCommand = new PSCommand(); + promptCommand.AddScript(promptScript); + _wrappedDebugger.ProcessCommand(promptCommand, output); + + return new DebuggerCommandResults(null, true); + } + + #endregion + } + /// /// Task child job that wraps asynchronously running tasks /// - internal sealed class PSTaskChildJob : Job + internal sealed class PSTaskChildJob : Job, IJobDebugger { #region Members private readonly PSJobTask _task; + private PSTaskChildDebugger _jobDebuggerWrapper; #endregion @@ -1062,6 +1255,33 @@ public override void StopJob() #endregion + #region IJobDebugger + + /// + /// Job Debugger + /// + public Debugger Debugger + { + get + { + if (_jobDebuggerWrapper == null) + { + _jobDebuggerWrapper = new PSTaskChildDebugger( + _task.Debugger, + this.Name); + } + + return _jobDebuggerWrapper; + } + } + + /// + /// IsAsync + /// + public bool IsAsync { get; set; } + + #endregion + #region Private Methods private void HandleTaskStateChange(object sender, PSInvocationStateChangedEventArgs args) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 index 4df954e4d07..c0604c64667 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Security/ConstrainedLanguageRestriction.Tests.ps1 @@ -904,7 +904,7 @@ try $ExecutionContext.SessionState.LanguageMode = "ConstrainedLanguage" Invoke-LanguageModeTestingSupportCmdlet -SetLockdownMode - $results = 1..1 | ForEach-Object -Parallel -ScriptBlock { $ExecutionContext.SessionState.LanguageMode } + $results = 1..1 | ForEach-Object -Parallel { $ExecutionContext.SessionState.LanguageMode } } finally { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index a53483f7632..ef122e06745 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -26,7 +26,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { It "Verifies dollar underbar variable" { $expected = 1..10 - $result = $expected | ForEach-Object -Parallel -ScriptBlock { $_ } + $result = $expected | ForEach-Object -Parallel { $_ } $result.Count | Should -BeExactly $expected.Count $result | Should -Contain 1 $result | Should -Contain 10 @@ -36,7 +36,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { $var = "Hello" $varArray = "Hello","There" - $result = 1..1 | ForEach-Object -Parallel -ScriptBlock { $using:var; $using:varArray[1] } + $result = 1..1 | ForEach-Object -Parallel { $using:var; $using:varArray[1] } $result.Count | Should -BeExactly 2 $result[0] | Should -BeExactly $var $result[1] | Should -BeExactly $varArray[1] @@ -44,7 +44,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { It 'Verifies terminating error streaming' { - $result = 1..1 | ForEach-Object -Parallel -ScriptBlock { throw 'Terminating Error!'; "Hello" } 2>&1 + $result = 1..1 | ForEach-Object -Parallel { throw 'Terminating Error!'; "Hello" } 2>&1 $result.Count | Should -BeExactly 1 $result.ToString() | Should -BeExactly 'Terminating Error!' $result.FullyQualifiedErrorId | Should -BeExactly 'PSTaskException' @@ -52,7 +52,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { It 'Verifies terminating error in multiple iterations' { - $results = 1..2 | ForEach-Object -Parallel -ScriptBlock { + $results = 1..2 | ForEach-Object -Parallel { if ($_ -eq 1) { throw 'Terminating Error!' "Hello!" @@ -68,48 +68,48 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { It 'Verifies non-terminating error streaming' { - $expectedError = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Error "Error!" } 2>&1 + $expectedError = 1..1 | ForEach-Object -Parallel { Write-Error "Error!" } 2>&1 $expectedError.ToString() | Should -BeExactly 'Error!' $expectedError.FullyQualifiedErrorId | Should -BeExactly 'Microsoft.PowerShell.Commands.WriteErrorException' } It 'Verifies warning data streaming' { - $expectedWarning = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Warning "Warning!" } 3>&1 + $expectedWarning = 1..1 | ForEach-Object -Parallel { Write-Warning "Warning!" } 3>&1 $expectedWarning.Message | Should -BeExactly 'Warning!' } It 'Verifies verbose data streaming' { - $expectedVerbose = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Verbose "Verbose!" -Verbose } -Verbose 4>&1 + $expectedVerbose = 1..1 | ForEach-Object -Parallel { Write-Verbose "Verbose!" -Verbose } -Verbose 4>&1 $expectedVerbose.Message | Should -BeExactly 'Verbose!' } It 'Verifies debug data streaming' { - $expectedDebug = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Debug "Debug!" -Debug } -Debug 5>&1 + $expectedDebug = 1..1 | ForEach-Object -Parallel { Write-Debug "Debug!" -Debug } -Debug 5>&1 $expectedDebug.Message | Should -BeExactly 'Debug!' } It 'Verifies information data streaming' { - $expectedInformation = 1..1 | ForEach-Object -Parallel -ScriptBlock { Write-Information "Information!" } 6>&1 + $expectedInformation = 1..1 | ForEach-Object -Parallel { Write-Information "Information!" } 6>&1 $expectedInformation.MessageData | Should -BeExactly 'Information!' } It 'Verifies error for using script block variable' { - { 1..1 | ForEach-Object -Parallel -ScriptBlock { $using:sb } } | Should -Throw -ErrorId 'ParallelUsingVariableCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' + { 1..1 | ForEach-Object -Parallel { $using:sb } } | Should -Throw -ErrorId 'ParallelUsingVariableCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' } It 'Verifies error for script block piped variable' { - { $sb | ForEach-Object -Parallel -ScriptBlock { "Hello" } -ErrorAction Stop } | Should -Throw -ErrorId 'ParallelPipedInputObjectCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' + { $sb | ForEach-Object -Parallel { "Hello" } -ErrorAction Stop } | Should -Throw -ErrorId 'ParallelPipedInputObjectCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' } It 'Verifies that parallel script blocks run in FullLanguage mode by default' { - $results = 1..1 | ForEach-Object -Parallel -ScriptBlock { $ExecutionContext.SessionState.LanguageMode } + $results = 1..1 | ForEach-Object -Parallel { $ExecutionContext.SessionState.LanguageMode } $results | Should -BeExactly 'FullLanguage' } } @@ -135,12 +135,12 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies TimeoutSeconds parameter is excluded from AsJob' { - { 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello" } -TimeoutSeconds 60 } | Should -Throw -ErrorId 'ParallelCannotUseTimeoutWithJob,Microsoft.PowerShell.Commands.ForEachObjectCommand' + { 1..1 | ForEach-Object -AsJob -Parallel { "Hello" } -TimeoutSeconds 60 } | Should -Throw -ErrorId 'ParallelCannotUseTimeoutWithJob,Microsoft.PowerShell.Commands.ForEachObjectCommand' } It 'Verifies ForEach-Object -Parallel jobs appear in job repository' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello" } + $job = 1..1 | ForEach-Object -AsJob -Parallel { "Hello" } Get-Job | Should -Contain $job $job | Wait-Job | Remove-Job } @@ -148,7 +148,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies dollar underbar variable' { $expected = 1..10 - $job = $expected | ForEach-Object -Parallel -AsJob -ScriptBlock { $_ } + $job = $expected | ForEach-Object -AsJob -Parallel { $_ } $result = $job | Wait-Job | Receive-Job $job | Remove-Job $result.Count | Should -BeExactly $expected.Count @@ -162,7 +162,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { $Var2 = "Goodbye" $Var3 = 105 $Var4 = "One","Two","Three" - $job = 1..1 | Foreach-Object -Parallel -AsJob -ScriptBlock { + $job = 1..1 | Foreach-Object -AsJob -Parallel { Write-Output $using:Var1 Write-Output $using:Var2 Write-Output $using:Var3 @@ -181,7 +181,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies terminating error in single iteration' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { throw "Terminating Error!"; "Hello" } + $job = 1..1 | ForEach-Object -AsJob -Parallel { throw "Terminating Error!"; "Hello" } $results = $job | Wait-Job | Receive-Job 2>$null $results.Count | Should -BeExactly 0 $job.State | Should -BeExactly 'Failed' @@ -192,7 +192,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies terminating error in double iteration' { - $job = 1..2 | ForEach-Object -Parallel -AsJob -ScriptBlock { + $job = 1..2 | ForEach-Object -AsJob -Parallel { if ($_ -eq 1) { throw "Terminating Error!" "Goodbye!" @@ -213,7 +213,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies non-terminating error' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Error "Error:$_" } + $job = 1..1 | ForEach-Object -AsJob -Parallel { Write-Error "Error:$_" } $results = $job | Wait-Job | Receive-Job 2>&1 $job | Remove-Job $results.ToString() | Should -BeExactly "Error:1" @@ -221,7 +221,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies warning data' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Warning "Warning:$_" } + $job = 1..1 | ForEach-Object -AsJob -Parallel { Write-Warning "Warning:$_" } $results = $job | Wait-Job | Receive-Job 3>&1 $job | Remove-Job $results.Message | Should -BeExactly "Warning:1" @@ -229,7 +229,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies verbose data' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Verbose "Verbose:$_" -Verbose } + $job = 1..1 | ForEach-Object -AsJob -Parallel { Write-Verbose "Verbose:$_" -Verbose } $results = $job | Wait-Job | Receive-Job -Verbose 4>&1 $job | Remove-Job $results.Message | Should -BeExactly "Verbose:1" @@ -237,7 +237,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies debug data' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Debug "Debug:$_" -Debug } + $job = 1..1 | ForEach-Object -AsJob -Parallel { Write-Debug "Debug:$_" -Debug } $results = $job | Wait-Job | Receive-Job -Debug 5>&1 $job | Remove-Job $results.Message | Should -BeExactly "Debug:1" @@ -245,7 +245,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies information data' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock { Write-Information "Information:$_" } + $job = 1..1 | ForEach-Object -AsJob -Parallel { Write-Information "Information:$_" } $results = $job | Wait-Job | Receive-Job 6>&1 $job | Remove-Job $results.MessageData | Should -BeExactly "Information:1" @@ -253,7 +253,7 @@ Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { It 'Verifies job Command property' { - $job = 1..1 | ForEach-Object -Parallel -AsJob -ScriptBlock {"Hello"} + $job = 1..1 | ForEach-Object -AsJob -Parallel {"Hello"} $job.Command | Should -BeExactly '"Hello"' $job.ChildJobs[0].Command | Should -BeExactly '"Hello"' $job | Wait-Job | Remove-Job @@ -282,7 +282,7 @@ Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { It 'Verifies job queuing and throttle limit' { # Run four job tasks, two in parallel at a time. - $job = 1..4 | ForEach-Object -Parallel -ScriptBlock { Start-Sleep 60 } -AsJob -ThrottleLimit 2 + $job = 1..4 | ForEach-Object -Parallel { Start-Sleep 60 } -AsJob -ThrottleLimit 2 # Wait for child job 2 to begin running for up to ten seconds $count = 0 @@ -306,14 +306,14 @@ Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { It 'Verifies jobs work with Receive-Job -AutoRemove parameter' { - $job = 1..4 | ForEach-Object -Parallel -AsJob -ScriptBlock { "Hello:$_" } + $job = 1..4 | ForEach-Object -AsJob -Parallel { "Hello:$_" } $null = $job | Receive-Job -Wait -AutoRemoveJob Get-Job | Should -Not -Contain $job } It 'Verifies parallel task queuing' { - $results = 10..1 | ForEach-Object -Parallel -ScriptBlock { Start-Sleep 1; $_ } -ThrottleLimit 5 + $results = 10..1 | ForEach-Object -Parallel { Start-Sleep 1; $_ } -ThrottleLimit 5 $results[0] | Should -BeGreaterThan 5 $results[1] | Should -BeGreaterThan 5 $results[2] | Should -BeGreaterThan 5 From 08b3291a5b6daa59ebc994044bc6cd95818a33f1 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Wed, 31 Jul 2019 14:29:29 -0700 Subject: [PATCH 18/27] Address PR comments --- .../engine/hostifaces/PSTask.cs | 98 ++++++++++--------- .../resources/InternalCommandStrings.resx | 4 +- .../Foreach-Object-Parallel.Tests.ps1 | 8 +- 3 files changed, 57 insertions(+), 53 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 7225fdce6e4..fe76ca7d2fe 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -17,7 +17,7 @@ namespace System.Management.Automation.PSTasks #region PSTask /// - /// Class to encapsulate synchronous running scripts in parallel + /// Class to encapsulate synchronous running scripts in parallel. /// internal sealed class PSTask : PSTaskBase { @@ -30,7 +30,7 @@ internal sealed class PSTask : PSTaskBase #region Constructor /// - /// Constructor + /// Constructor. /// public PSTask( ScriptBlock scriptBlock, @@ -50,7 +50,7 @@ public PSTask( #region Overrides /// - /// Initialize PowerShell object + /// Initialize PowerShell object. /// protected override void InitializePowershell() { @@ -154,7 +154,7 @@ private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInf } /// - /// Class to encapsulate asynchronous running scripts in parallel as jobs + /// Class to encapsulate asynchronous running scripts in parallel as jobs. /// internal sealed class PSJobTask : PSTaskBase { @@ -167,7 +167,7 @@ internal sealed class PSJobTask : PSTaskBase #region Constructor /// - /// Constructor + /// Constructor. /// public PSJobTask( ScriptBlock scriptBlock, @@ -186,7 +186,7 @@ public PSJobTask( #region Overrides /// - /// Initialize PowerShell object + /// Initialize PowerShell object. /// protected override void InitializePowershell() { @@ -280,7 +280,7 @@ private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInf #region Properties /// - /// Debugger + /// Gets Debugger. /// public Debugger Debugger { @@ -294,7 +294,7 @@ public Debugger Debugger } /// - /// Base class to encapsulate running a PowerShell script concurrently in a cmdlet or job context + /// Base class to encapsulate running a PowerShell script concurrently in a cmdlet or job context. /// internal abstract class PSTaskBase : IDisposable { @@ -308,7 +308,6 @@ internal abstract class PSTaskBase : IDisposable protected PowerShell _powershell; protected PSDataCollection _output; - private const string VERBATIM_ARGUMENT = "--%"; private const string RunspaceName = "PSTask"; private static int s_taskId; @@ -318,7 +317,7 @@ internal abstract class PSTaskBase : IDisposable #region Events /// - /// Event that fires when the task running state changes + /// Event that fires when the task running state changes. /// public event EventHandler StateChanged; @@ -332,7 +331,7 @@ internal void RaiseStateChangedEvent(PSInvocationStateChangedEventArgs args) #region Properties /// - /// Current running state of the task + /// Gets current running state of the task. /// public PSInvocationState State { @@ -349,7 +348,7 @@ public PSInvocationState State } /// - /// Task Id + /// Gets Task Id. /// public int Id { get { return _id; } } @@ -363,7 +362,7 @@ private PSTaskBase() } /// - /// Constructor + /// Constructor. /// protected PSTaskBase( ScriptBlock scriptBlock, @@ -380,7 +379,7 @@ protected PSTaskBase( #region Abstract Methods /// - /// Initialize PowerShell object + /// Initialize PowerShell object. /// protected abstract void InitializePowershell(); @@ -388,6 +387,9 @@ protected PSTaskBase( #region IDisposable + /// + /// Dispose. + /// public void Dispose() { _runspace.Dispose(); @@ -400,7 +402,7 @@ public void Dispose() #region Public Methods /// - /// Start task + /// Start task. /// public void Start() { @@ -432,13 +434,13 @@ public void Start() _powershell.Commands.Commands[0].DollarUnderbar = _dollarUnderbar; if (_usingValuesMap != null && _usingValuesMap.Count > 0) { - _powershell.AddParameter(VERBATIM_ARGUMENT, _usingValuesMap); + _powershell.AddParameter(Parser.VERBATIM_ARGUMENT, _usingValuesMap); } _powershell.BeginInvoke(null, _output); } /// - /// Signals the running task to stop + /// Signals the running task to stop. /// public void SignalStop() { @@ -456,7 +458,7 @@ public void SignalStop() #region PSTaskDataStreamWriter /// - /// Class that handles writing task data stream objects to a cmdlet + /// Class that handles writing task data stream objects to a cmdlet. /// internal sealed class PSTaskDataStreamWriter : IDisposable { @@ -485,7 +487,7 @@ internal WaitHandle DataAddedWaitHandle private PSTaskDataStreamWriter() { } /// - /// Constructor + /// Constructor. /// public PSTaskDataStreamWriter(PSCmdlet psCmdlet) { @@ -499,7 +501,7 @@ public PSTaskDataStreamWriter(PSCmdlet psCmdlet) #region Public Methods /// - /// Add data stream object to the writer + /// Add data stream object to the writer. /// public void Add(PSStreamObject streamObject) { @@ -507,7 +509,7 @@ public void Add(PSStreamObject streamObject) } /// - /// Write all objects in data stream collection to the cmdlet data stream + /// Write all objects in data stream collection to the cmdlet data stream. /// public void WriteImmediate() { @@ -542,7 +544,7 @@ public void WaitAndWrite() } /// - /// Closes the stream writer + /// Closes the stream writer. /// public void Close() { @@ -566,7 +568,7 @@ private void CheckCmdletThread() #region IDisposable /// - /// Disposes the stream writer + /// Disposes the stream writer. /// public void Dispose() { @@ -601,7 +603,7 @@ internal sealed class PSTaskPool : IDisposable private PSTaskPool() { } /// - /// Constructor + /// Constructor. /// public PSTaskPool(int size) { @@ -618,7 +620,7 @@ public PSTaskPool(int size) #region Events /// - /// Event that fires when pool is closed and drained of all tasks + /// Event that fires when pool is closed and drained of all tasks. /// public event EventHandler PoolComplete; @@ -627,7 +629,7 @@ public PSTaskPool(int size) #region Public Properties /// - /// Returns true if pool is currently open for accepting tasks + /// Returns true if pool is currently open for accepting tasks. /// public bool IsOpen { @@ -639,7 +641,7 @@ public bool IsOpen #region IDisposable /// - /// Dispose task pool + /// Dispose task pool. /// public void Dispose() { @@ -726,7 +728,7 @@ public bool Add( } /// - /// Add child job task to task pool + /// Add child job task to task pool. /// public bool Add(PSTaskChildJob childJob) { @@ -734,7 +736,7 @@ public bool Add(PSTaskChildJob childJob) } /// - /// Signals all running tasks to stop and closes pool for any new tasks + /// Signals all running tasks to stop and closes pool for any new tasks. /// public void StopAll() { @@ -753,7 +755,7 @@ public void StopAll() } /// - /// Closes the pool and prevents any new tasks from being added + /// Closes the pool and prevents any new tasks from being added. /// public void Close() { @@ -825,7 +827,7 @@ private void CheckForComplete() #region PSTaskJobs /// - /// Job for running ForEach-Object parallel task child jobs asynchronously + /// Job for running ForEach-Object parallel task child jobs asynchronously. /// internal sealed class PSTaskJob : Job { @@ -841,6 +843,9 @@ internal sealed class PSTaskJob : Job private PSTaskJob() { } + /// + /// Constructor. + /// public PSTaskJob( string command, int throttleLimit) : base(command, string.Empty) @@ -857,7 +862,7 @@ public PSTaskJob( #region Overrides /// - /// Location + /// Gets Location. /// public override string Location { @@ -865,7 +870,7 @@ public override string Location } /// - /// HasMoreData + /// Gets HasMoreData. /// public override bool HasMoreData { @@ -884,7 +889,7 @@ public override bool HasMoreData } /// - /// StatusMessage + /// Gets StatusMessage. /// public override string StatusMessage { @@ -892,7 +897,7 @@ public override string StatusMessage } /// - /// StopJob + /// StopJob. /// public override void StopJob() { @@ -904,7 +909,7 @@ public override void StopJob() } /// - /// Dispose + /// Dispose. /// protected override void Dispose(bool disposing) { @@ -1006,6 +1011,9 @@ internal sealed class PSTaskChildDebugger : Debugger private PSTaskChildDebugger() { } + /// + /// Constructor. + /// public PSTaskChildDebugger( Debugger debugger, string jobName) @@ -1166,7 +1174,7 @@ private DebuggerCommandResults HandlePromptCommand(PSDataCollection ou } /// - /// Task child job that wraps asynchronously running tasks + /// Task child job that wraps asynchronously running tasks. /// internal sealed class PSTaskChildJob : Job, IJobDebugger { @@ -1182,7 +1190,7 @@ internal sealed class PSTaskChildJob : Job, IJobDebugger private PSTaskChildJob() { } /// - /// Constructor + /// Constructor. /// public PSTaskChildJob( ScriptBlock scriptBlock, @@ -1201,7 +1209,7 @@ public PSTaskChildJob( #region Properties /// - /// Child job task + /// Gets child job task. /// internal PSTaskBase Task { @@ -1213,7 +1221,7 @@ internal PSTaskBase Task #region Overrides /// - /// Location + /// Gets Location. /// public override string Location { @@ -1221,7 +1229,7 @@ public override string Location } /// - /// HasMoreData + /// Gets HasMoreData. /// public override bool HasMoreData { @@ -1238,7 +1246,7 @@ public override bool HasMoreData } /// - /// StatusMessage + /// Gets StatusMessage. /// public override string StatusMessage { @@ -1246,7 +1254,7 @@ public override string StatusMessage } /// - /// StopJob + /// StopJob. /// public override void StopJob() { @@ -1258,7 +1266,7 @@ public override void StopJob() #region IJobDebugger /// - /// Job Debugger + /// Gets Job Debugger. /// public Debugger Debugger { @@ -1276,7 +1284,7 @@ public Debugger Debugger } /// - /// IsAsync + /// Gets or sets IsAsync. /// public bool IsAsync { get; set; } diff --git a/src/System.Management.Automation/resources/InternalCommandStrings.resx b/src/System.Management.Automation/resources/InternalCommandStrings.resx index 5b84636f77a..857dd762664 100644 --- a/src/System.Management.Automation/resources/InternalCommandStrings.resx +++ b/src/System.Management.Automation/resources/InternalCommandStrings.resx @@ -169,10 +169,10 @@ This method cannot be run on the current thread. It can only be called on the cmdlet thread. - A ForEach-Object -Parallel using variable cannot be a script block. Script blocks have undefined behavior when used in parallel. + A ForEach-Object -Parallel using variable cannot be a script block. Passed-in script block variables are not supported with ForEach-Object -Parallel, and can result in undefined behavior. - A ForEach-Object -Parallel piped input object cannot be a script block. Script blocks have undefined behavior when used in parallel. + A ForEach-Object -Parallel piped input object cannot be a script block. Passed-in script block variables are not supported with ForEach-Object -Parallel, and can result in undefined behavior. The 'TimeoutSeconds' parameter cannot be used with the 'AsJob' parameter. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index ef122e06745..97c2d152c3d 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -285,12 +285,8 @@ Describe 'ForEach-Object -Parallel Functional Tests' -Tags 'Feature' { $job = 1..4 | ForEach-Object -Parallel { Start-Sleep 60 } -AsJob -ThrottleLimit 2 # Wait for child job 2 to begin running for up to ten seconds - $count = 0 - while (($job.ChildJobs[1].JobStateInfo.State -ne 'Running') -and (++$count -lt 40)) - { - Start-Sleep -Milliseconds 250 - } - if ($job.ChildJobs[1].JobStateInfo.State -ne 'Running') + if ( !(Wait-UntilTrue -TimeoutInMilliseconds 10000 -IntervalInMilliseconds 250 ` + { $job.ChildJobs[1].JobStateInfo.State -eq 'Running' })) { throw "ForEach-Object -Parallel child job 2 did not start" } From e43488738c9e973bbc2b49adc86a28fb3aea7854 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 5 Aug 2019 11:13:13 -0700 Subject: [PATCH 19/27] Clean up comments based on Codacy and CodeFactor --- .../engine/InternalCommands.cs | 12 +- .../engine/hostifaces/PSTask.cs | 120 +++++++++++------- 2 files changed, 78 insertions(+), 54 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index e31bd88ca38..5ee0059c0a1 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -207,14 +207,14 @@ public object[] ArgumentList #region ParallelParameterSet /// - /// Gets and sets a script block to run in parallel for each pipeline object. + /// Gets or sets a script block to run in parallel for each pipeline object. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] [Parameter(Mandatory = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] public ScriptBlock Parallel { get; set; } /// - /// Gets and sets the maximum number of concurrently running scriptblocks on separate threads. + /// Gets or sets the maximum number of concurrently running scriptblocks on separate threads. /// The default number is 5. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] @@ -223,7 +223,7 @@ public object[] ArgumentList public int ThrottleLimit { get; set; } = 5; /// - /// Gets and sets a timeout time in seconds, after which the parallel running scripts will be stopped + /// Gets or sets a timeout time in seconds, after which the parallel running scripts will be stopped /// The default value is 0, indicating no timeout. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] @@ -232,7 +232,7 @@ public object[] ArgumentList public int TimeoutSeconds { get; set; } /// - /// Gets and sets a flag that returns a job object immediately for the parallel operation, instead of returning after + /// Gets or sets a flag that returns a job object immediately for the parallel operation, instead of returning after /// all foreach processing is completed. /// [Experimental("PSForEachObjectParallel", ExperimentAction.Show)] @@ -309,7 +309,7 @@ protected override void EndProcessing() } /// - /// Handle pipeline stop signal + /// Handle pipeline stop signal. /// protected override void StopProcessing() { @@ -326,7 +326,7 @@ protected override void StopProcessing() #region IDisposable /// - /// Dispose + /// Dispose cmdlet instance. /// public void Dispose() { diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index fe76ca7d2fe..a42db38f462 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -30,8 +30,12 @@ internal sealed class PSTask : PSTaskBase #region Constructor /// - /// Constructor. + /// Initializes new instance of PSTask. /// + /// Script block to run in task + /// Using values passed into script block + /// Dollar underbar variable value + /// Cmdlet data stream writer public PSTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -167,8 +171,12 @@ internal sealed class PSJobTask : PSTaskBase #region Constructor /// - /// Constructor. + /// Initializes new instance PSJobTask. /// + /// Script block to run + /// Using variable values passed to script block + /// Dollar underbar variable value for script block + /// Job object associated with task public PSJobTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -362,8 +370,11 @@ private PSTaskBase() } /// - /// Constructor. + /// Initializes instance of PSTaskBase. /// + /// Script block to run + /// Using variable values passed to script block + /// Dollar underbar variable value protected PSTaskBase( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -388,7 +399,7 @@ protected PSTaskBase( #region IDisposable /// - /// Dispose. + /// Dispose PSTaskBase instance. /// public void Dispose() { @@ -473,7 +484,7 @@ internal sealed class PSTaskDataStreamWriter : IDisposable #region Properties /// - /// Wait-able handle that signals when new data has been added to + /// Gets wait-able handle that signals when new data has been added to /// the data stream collection. internal WaitHandle DataAddedWaitHandle { @@ -487,7 +498,7 @@ internal WaitHandle DataAddedWaitHandle private PSTaskDataStreamWriter() { } /// - /// Constructor. + /// Initializes instance of PSTaskDataStreamWriter. /// public PSTaskDataStreamWriter(PSCmdlet psCmdlet) { @@ -503,6 +514,7 @@ public PSTaskDataStreamWriter(PSCmdlet psCmdlet) /// /// Add data stream object to the writer. /// + /// Data stream object to write public void Add(PSStreamObject streamObject) { _dataStream.Add(streamObject); @@ -568,7 +580,7 @@ private void CheckCmdletThread() #region IDisposable /// - /// Disposes the stream writer. + /// Dispose the stream writer. /// public void Dispose() { @@ -589,12 +601,12 @@ internal sealed class PSTaskPool : IDisposable { #region Members - private readonly int _sizeLimit; - private bool _isOpen; - private readonly object _syncObject; private readonly ManualResetEvent _addAvailable; private readonly ManualResetEvent _stopAll; private readonly Dictionary _taskPool; + private readonly int _sizeLimit; + private bool _isOpen; + private readonly object _syncObject; #endregion @@ -603,8 +615,9 @@ internal sealed class PSTaskPool : IDisposable private PSTaskPool() { } /// - /// Constructor. + /// Initializes instance of PSTaskPool. /// + /// Total number of allowed running objects in pool at one time public PSTaskPool(int size) { _sizeLimit = size; @@ -629,7 +642,7 @@ public PSTaskPool(int size) #region Public Properties /// - /// Returns true if pool is currently open for accepting tasks. + /// Gets boolean indicating if pool is currently open for accepting tasks. /// public bool IsOpen { @@ -658,11 +671,13 @@ public void Dispose() /// If the pool is full, then this method blocks until space is available. /// This method is not multi-thread safe and assumes only one thread waits and adds tasks. /// + /// Task to be added to pool + /// Optional cmdlet data stream writer public bool Add( PSTaskBase task, PSTaskDataStreamWriter dataStreamWriter = null) { - if (! _isOpen) + if (!_isOpen) { return false; } @@ -696,7 +711,7 @@ public bool Add( lock (_syncObject) { - if (! _isOpen) + if (!_isOpen) { return false; } @@ -730,6 +745,7 @@ public bool Add( /// /// Add child job task to task pool. /// + /// Child job to be added to pool public bool Add(PSTaskChildJob childJob) { return Add(childJob.Task); @@ -800,7 +816,7 @@ private void CheckForComplete() bool isTaskPoolComplete; lock (_syncObject) { - isTaskPoolComplete = (!_isOpen && _taskPool.Count == 0); + isTaskPoolComplete = !_isOpen && _taskPool.Count == 0; } if (isTaskPoolComplete) @@ -844,8 +860,10 @@ internal sealed class PSTaskJob : Job private PSTaskJob() { } /// - /// Constructor. + /// Initializes instance of PSTaskJob. /// + /// Job command text + /// Pool size limit for task job public PSTaskJob( string command, int throttleLimit) : base(command, string.Empty) @@ -897,7 +915,7 @@ public override string StatusMessage } /// - /// StopJob. + /// Stops running job. /// public override void StopJob() { @@ -909,7 +927,7 @@ public override void StopJob() } /// - /// Dispose. + /// Disposes task job. /// protected override void Dispose(bool disposing) { @@ -928,9 +946,10 @@ protected override void Dispose(bool disposing) /// /// Add a child job to the collection. /// + /// Child job to add public bool AddJob(PSTaskChildJob childJob) { - if (! _isOpen) + if (!_isOpen) { return false; } @@ -959,8 +978,7 @@ public void Start() _taskPool.Add((PSTaskChildJob) childJob); } _taskPool.Close(); - } - ); + }); } #endregion @@ -996,14 +1014,14 @@ private void HandleTaskPoolComplete(object sender, EventArgs args) } /// - /// PSTaskChildJob debugger wrapper + /// PSTaskChildJob debugger wrapper. /// internal sealed class PSTaskChildDebugger : Debugger { #region Members - private Debugger _wrappedDebugger; - private string _jobName; + private readonly Debugger _wrappedDebugger; + private readonly string _jobName; #endregion @@ -1012,8 +1030,10 @@ internal sealed class PSTaskChildDebugger : Debugger private PSTaskChildDebugger() { } /// - /// Constructor. + /// Initializes new instance of PSTaskChildDebugger. /// + /// Script debugger associated with task + /// Job name for associated task public PSTaskChildDebugger( Debugger debugger, string jobName) @@ -1039,10 +1059,12 @@ public PSTaskChildDebugger( /// Evaluates provided command either as a debugger specific command /// or a PowerShell command. /// - /// PowerShell command. - /// Output. - /// DebuggerCommandResults. - public override DebuggerCommandResults ProcessCommand(PSCommand command, PSDataCollection output) + /// PowerShell command + /// Output + /// DebuggerCommandResults + public override DebuggerCommandResults ProcessCommand( + PSCommand command, + PSDataCollection output) { // Special handling for the prompt command. if (command.Commands[0].CommandText.Trim().Equals("prompt", StringComparison.OrdinalIgnoreCase)) @@ -1065,7 +1087,7 @@ public override void SetBreakpoints(IEnumerable breakpoints) /// /// Sets the debugger resume action. /// - /// DebuggerResumeAction. + /// DebuggerResumeAction public override void SetDebuggerAction(DebuggerResumeAction resumeAction) { _wrappedDebugger.SetDebuggerAction(resumeAction); @@ -1083,7 +1105,7 @@ public override void StopProcessCommand() /// Returns current debugger stop event arguments if debugger is in /// debug stop state. Otherwise returns null. /// - /// DebuggerStopEventArgs. + /// DebuggerStopEventArgs public override DebuggerStopEventArgs GetDebuggerStopArgs() { return _wrappedDebugger.GetDebuggerStopArgs(); @@ -1092,11 +1114,11 @@ public override DebuggerStopEventArgs GetDebuggerStopArgs() /// /// Sets the parent debugger, breakpoints, and other debugging context information. /// - /// Parent debugger. - /// List of breakpoints. - /// Debugger mode. - /// PowerShell host. - /// Current path. + /// Parent debugger + /// List of breakpoints + /// Debugger mode + /// PowerShell host + /// Current path public override void SetParent( Debugger parent, IEnumerable breakPoints, @@ -1111,6 +1133,7 @@ public override void SetParent( /// /// Sets the debugger mode. /// + /// Debugger mode to set public override void SetDebugMode(DebugModes mode) { _wrappedDebugger.SetDebugMode(mode); @@ -1121,7 +1144,6 @@ public override void SetDebugMode(DebugModes mode) /// /// Returns IEnumerable of CallStackFrame objects. /// - /// public override IEnumerable GetCallStack() { return _wrappedDebugger.GetCallStack(); @@ -1130,14 +1152,13 @@ public override IEnumerable GetCallStack() /// /// Sets debugger stepping mode. /// - /// True if stepping is to be enabled. public override void SetDebuggerStepMode(bool enabled) { _wrappedDebugger.SetDebuggerStepMode(enabled); } /// - /// True when debugger is stopped at a breakpoint. + /// Gets boolean indicating when debugger is stopped at a breakpoint. /// public override bool InBreakpoint { @@ -1190,8 +1211,11 @@ internal sealed class PSTaskChildJob : Job, IJobDebugger private PSTaskChildJob() { } /// - /// Constructor. + /// Initializes a new instance of PSTaskChildJob. /// + /// Script block to run + /// Using variable values passed to script block + /// Dollar underbar variable value public PSTaskChildJob( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -1235,13 +1259,13 @@ public override bool HasMoreData { get { - return (this.Output.Count > 0 || - this.Error.Count > 0 || - this.Progress.Count > 0 || - this.Verbose.Count > 0 || - this.Debug.Count > 0 || - this.Warning.Count > 0 || - this.Information.Count > 0); + return this.Output.Count > 0 || + this.Error.Count > 0 || + this.Progress.Count > 0 || + this.Verbose.Count > 0 || + this.Debug.Count > 0 || + this.Warning.Count > 0 || + this.Information.Count > 0; } } @@ -1254,7 +1278,7 @@ public override string StatusMessage } /// - /// StopJob. + /// Stops running job. /// public override void StopJob() { From 1ff1155d7491512e63002f66b07c44f914f3df84 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Wed, 7 Aug 2019 15:38:26 -0700 Subject: [PATCH 20/27] More code clean up. Use newer syntax for properties. --- .../engine/hostifaces/PSTask.cs | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index a42db38f462..70d1dff9665 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -292,10 +292,7 @@ private void HandleStateChanged(PSInvocationStateChangedEventArgs stateChangeInf /// public Debugger Debugger { - get - { - return _powershell.Runspace.Debugger; - } + get => _powershell.Runspace.Debugger; } #endregion @@ -358,7 +355,7 @@ public PSInvocationState State /// /// Gets Task Id. /// - public int Id { get { return _id; } } + public int Id { get => _id; } #endregion @@ -488,7 +485,7 @@ internal sealed class PSTaskDataStreamWriter : IDisposable /// the data stream collection. internal WaitHandle DataAddedWaitHandle { - get { return _dataStream.WaitHandle; } + get => _dataStream.WaitHandle; } #endregion @@ -639,14 +636,14 @@ public PSTaskPool(int size) #endregion - #region Public Properties + #region Properties /// /// Gets boolean indicating if pool is currently open for accepting tasks. /// public bool IsOpen { - get { return _isOpen; } + get => _isOpen; } #endregion @@ -884,7 +881,7 @@ public PSTaskJob( /// public override string Location { - get { return "PowerShell"; } + get => "PowerShell"; } /// @@ -911,7 +908,7 @@ public override bool HasMoreData /// public override string StatusMessage { - get { return string.Empty; } + get => string.Empty; } /// @@ -1144,6 +1141,7 @@ public override void SetDebugMode(DebugModes mode) /// /// Returns IEnumerable of CallStackFrame objects. /// + /// Enumerable call stack public override IEnumerable GetCallStack() { return _wrappedDebugger.GetCallStack(); @@ -1152,6 +1150,7 @@ public override IEnumerable GetCallStack() /// /// Sets debugger stepping mode. /// + /// True to enable debugger step mode public override void SetDebuggerStepMode(bool enabled) { _wrappedDebugger.SetDebuggerStepMode(enabled); @@ -1162,7 +1161,7 @@ public override void SetDebuggerStepMode(bool enabled) /// public override bool InBreakpoint { - get { return _wrappedDebugger.InBreakpoint; } + get => _wrappedDebugger.InBreakpoint; } #endregion @@ -1237,7 +1236,7 @@ public PSTaskChildJob( /// internal PSTaskBase Task { - get { return _task; } + get => _task; } #endregion @@ -1249,7 +1248,7 @@ internal PSTaskBase Task /// public override string Location { - get { return "PowerShell"; } + get => "PowerShell"; } /// @@ -1257,16 +1256,13 @@ public override string Location /// public override bool HasMoreData { - get - { - return this.Output.Count > 0 || - this.Error.Count > 0 || - this.Progress.Count > 0 || - this.Verbose.Count > 0 || - this.Debug.Count > 0 || - this.Warning.Count > 0 || - this.Information.Count > 0; - } + get => this.Output.Count > 0 || + this.Error.Count > 0 || + this.Progress.Count > 0 || + this.Verbose.Count > 0 || + this.Debug.Count > 0 || + this.Warning.Count > 0 || + this.Information.Count > 0; } /// @@ -1274,7 +1270,7 @@ public override bool HasMoreData /// public override string StatusMessage { - get { return string.Empty; } + get => string.Empty; } /// From cb72fa7cb4012755876987053f6d513c68a258ea Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Fri, 9 Aug 2019 09:51:44 -0700 Subject: [PATCH 21/27] Response to PR comments --- .../engine/InternalCommands.cs | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index 5ee0059c0a1..cbf87e3b518 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -14,6 +14,7 @@ using System.Management.Automation.PSTasks; using System.Runtime.CompilerServices; using System.Text; +using System.Threading; using Dbg = System.Management.Automation.Diagnostics; namespace Microsoft.PowerShell.Commands @@ -57,21 +58,23 @@ public object GetValue(PSObject inputObject, string propertyName) /// to each element of the pipeline. /// [Cmdlet("ForEach", "Object", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.Low, - DefaultParameterSetName = "ScriptBlockSet", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=113300", + DefaultParameterSetName = ForEachObjectCommand.ScriptBlockSet, HelpUri = "https://go.microsoft.com/fwlink/?LinkID=113300", RemotingCapability = RemotingCapability.None)] public sealed class ForEachObjectCommand : PSCmdlet, IDisposable { #region Private Members private const string ParallelParameterSet = "ParallelParameterSet"; + private const string ScriptBlockSet = "ScriptBlockSet"; + private const string PropertyAndMethodSet = "PropertyAndMethodSet"; #endregion /// /// This parameter specifies the current pipeline object. /// - [Parameter(ValueFromPipeline = true, ParameterSetName = "ScriptBlockSet")] - [Parameter(ValueFromPipeline = true, ParameterSetName = "PropertyAndMethodSet")] + [Parameter(ValueFromPipeline = true, ParameterSetName = ForEachObjectCommand.ScriptBlockSet)] + [Parameter(ValueFromPipeline = true, ParameterSetName = ForEachObjectCommand.PropertyAndMethodSet)] [Parameter(ValueFromPipeline = true, ParameterSetName = ForEachObjectCommand.ParallelParameterSet)] public PSObject InputObject { @@ -89,7 +92,7 @@ public PSObject InputObject /// /// The script block to apply in begin processing. /// - [Parameter(ParameterSetName = "ScriptBlockSet")] + [Parameter(ParameterSetName = ForEachObjectCommand.ScriptBlockSet)] public ScriptBlock Begin { set @@ -106,7 +109,7 @@ public ScriptBlock Begin /// /// The script block to apply. /// - [Parameter(Mandatory = true, Position = 0, ParameterSetName = "ScriptBlockSet")] + [Parameter(Mandatory = true, Position = 0, ParameterSetName = ForEachObjectCommand.ScriptBlockSet)] [AllowNull] [AllowEmptyCollection] public ScriptBlock[] Process @@ -130,7 +133,7 @@ public ScriptBlock[] Process /// /// The script block to apply in complete processing. /// - [Parameter(ParameterSetName = "ScriptBlockSet")] + [Parameter(ParameterSetName = ForEachObjectCommand.ScriptBlockSet)] public ScriptBlock End { set @@ -148,7 +151,7 @@ public ScriptBlock End /// /// The remaining script blocks to apply. /// - [Parameter(ParameterSetName = "ScriptBlockSet", ValueFromRemainingArguments = true)] + [Parameter(ParameterSetName = ForEachObjectCommand.ScriptBlockSet, ValueFromRemainingArguments = true)] [AllowNull] [AllowEmptyCollection] public ScriptBlock[] RemainingScripts @@ -173,7 +176,7 @@ public ScriptBlock[] RemainingScripts /// /// The property or method name. /// - [Parameter(Mandatory = true, Position = 0, ParameterSetName = "PropertyAndMethodSet")] + [Parameter(Mandatory = true, Position = 0, ParameterSetName = ForEachObjectCommand.PropertyAndMethodSet)] [ValidateTrustedData] [ValidateNotNullOrEmpty] public string MemberName @@ -190,7 +193,7 @@ public string MemberName /// /// The arguments passed to a method invocation. /// - [Parameter(ParameterSetName = "PropertyAndMethodSet", ValueFromRemainingArguments = true)] + [Parameter(ParameterSetName = ForEachObjectCommand.PropertyAndMethodSet, ValueFromRemainingArguments = true)] [ValidateTrustedData] [Alias("Args")] public object[] ArgumentList @@ -253,7 +256,7 @@ protected override void BeginProcessing() { switch (ParameterSetName) { - case "ScriptBlockSet": + case ForEachObjectCommand.ScriptBlockSet: InitScriptBlockParameterSet(); break; @@ -274,11 +277,11 @@ protected override void ProcessRecord() { switch (ParameterSetName) { - case "ScriptBlockSet": + case ForEachObjectCommand.ScriptBlockSet: ProcessScriptBlockParameterSet(); break; - case "PropertyAndMethodSet": + case ForEachObjectCommand.PropertyAndMethodSet: ProcessPropertyAndMethodParameterSet(); break; @@ -298,7 +301,7 @@ protected override void EndProcessing() { switch (ParameterSetName) { - case "ScriptBlockSet": + case ForEachObjectCommand.ScriptBlockSet: EndBlockParameterSet(); break; @@ -331,18 +334,9 @@ protected override void StopProcessing() public void Dispose() { // Ensure all parallel task objects are disposed - if (_taskTimer != null) - { - _taskTimer.Dispose(); - } - if (_taskDataStreamWriter != null) - { - _taskDataStreamWriter.Dispose(); - } - if (_taskPool != null) - { - _taskPool.Dispose(); - } + _taskTimer?.Dispose(); + _taskDataStreamWriter?.Dispose(); + _taskPool?.Dispose(); } #endregion @@ -354,16 +348,22 @@ public void Dispose() private PSTaskPool _taskPool; private PSTaskDataStreamWriter _taskDataStreamWriter; private Dictionary _usingValuesMap; - private System.Threading.Timer _taskTimer; + private Timer _taskTimer; private PSTaskJob _taskJob; private void InitParallelParameterSet() { bool allowUsingExpression = this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage; - _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(Parallel, allowUsingExpression, this.Context, null); + _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary( + Parallel, + allowUsingExpression, + this.Context, + null); - // Validate using values map - foreach (var item in _usingValuesMap.Values) + // Validate using values map, which is a map of '$using:' variables referenced in the script. + // Script block variables are not allowed since their behavior is undefined outside the runspace + // in which they were created. + foreach (object item in _usingValuesMap.Values) { if (item is ScriptBlock) { @@ -402,11 +402,11 @@ private void InitParallelParameterSet() }; if (TimeoutSeconds != 0) { - _taskTimer = new System.Threading.Timer( + _taskTimer = new Timer( (_) => _taskPool.StopAll(), null, TimeoutSeconds * 1000, - System.Threading.Timeout.Infinite); + Timeout.Infinite); } } } From 644eeada70910fe0e44bed0a0d95e68aac459424 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Fri, 9 Aug 2019 14:15:38 -0700 Subject: [PATCH 22/27] Response to PR review comments --- .../engine/hostifaces/PSTask.cs | 7 ++--- .../Foreach-Object-Parallel.Tests.ps1 | 28 ++++++++++--------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 70d1dff9665..bab0904dc82 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -430,8 +430,7 @@ public void Start() // Create the PowerShell command pipeline for the provided script block // The script will run on the provided Runspace in a new thread by default - _powershell = PowerShell.Create(); - _powershell.Runspace = _runspace; + _powershell = PowerShell.Create(_runspace); // Initialize PowerShell object data streams and event handlers _output = new PSDataCollection(); @@ -444,7 +443,7 @@ public void Start() { _powershell.AddParameter(Parser.VERBATIM_ARGUMENT, _usingValuesMap); } - _powershell.BeginInvoke(null, _output); + _powershell.BeginInvoke(input:null, output:_output); } /// @@ -526,7 +525,7 @@ public void WriteImmediate() foreach (var item in _dataStream.ReadAll()) { - item.WriteStreamObject(_cmdlet, true); + item.WriteStreamObject(cmdlet:_cmdlet, overrideInquire:true); } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 97c2d152c3d..16be0a7a623 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -62,39 +62,41 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { } } 2>&1 - $results | Should -Not -Contain "Hello!" - $results | Should -Contain "Goodbye!" + $resultStrings = $results | ForEach-Object { $_.ToString() } + $resultStrings | Should -Not -Contain "Hello!" + $resultStrings | Should -Contain "Goodbye!" + $resultStrings | Should -Contain "Terminating Error!" } It 'Verifies non-terminating error streaming' { - $expectedError = 1..1 | ForEach-Object -Parallel { Write-Error "Error!" } 2>&1 - $expectedError.ToString() | Should -BeExactly 'Error!' - $expectedError.FullyQualifiedErrorId | Should -BeExactly 'Microsoft.PowerShell.Commands.WriteErrorException' + $actualError = 1..1 | ForEach-Object -Parallel { Write-Error "Error!" } 2>&1 + $actualError.ToString() | Should -BeExactly 'Error!' + $actualError.FullyQualifiedErrorId | Should -BeExactly 'Microsoft.PowerShell.Commands.WriteErrorException' } It 'Verifies warning data streaming' { - $expectedWarning = 1..1 | ForEach-Object -Parallel { Write-Warning "Warning!" } 3>&1 - $expectedWarning.Message | Should -BeExactly 'Warning!' + $actualWarning = 1..1 | ForEach-Object -Parallel { Write-Warning "Warning!" } 3>&1 + $actualWarning.Message | Should -BeExactly 'Warning!' } It 'Verifies verbose data streaming' { - $expectedVerbose = 1..1 | ForEach-Object -Parallel { Write-Verbose "Verbose!" -Verbose } -Verbose 4>&1 - $expectedVerbose.Message | Should -BeExactly 'Verbose!' + $actualVerbose = 1..1 | ForEach-Object -Parallel { Write-Verbose "Verbose!" -Verbose } -Verbose 4>&1 + $actualVerbose.Message | Should -BeExactly 'Verbose!' } It 'Verifies debug data streaming' { - $expectedDebug = 1..1 | ForEach-Object -Parallel { Write-Debug "Debug!" -Debug } -Debug 5>&1 - $expectedDebug.Message | Should -BeExactly 'Debug!' + $actualDebug = 1..1 | ForEach-Object -Parallel { Write-Debug "Debug!" -Debug } -Debug 5>&1 + $actualDebug.Message | Should -BeExactly 'Debug!' } It 'Verifies information data streaming' { - $expectedInformation = 1..1 | ForEach-Object -Parallel { Write-Information "Information!" } 6>&1 - $expectedInformation.MessageData | Should -BeExactly 'Information!' + $actualInformation = 1..1 | ForEach-Object -Parallel { Write-Information "Information!" } 6>&1 + $actualInformation.MessageData | Should -BeExactly 'Information!' } It 'Verifies error for using script block variable' { From af0cbe96e636e8b095eba86a79454338904c1167 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 12 Aug 2019 09:16:55 -0700 Subject: [PATCH 23/27] Add error for unsupported common parameters --- .../engine/InternalCommands.cs | 22 +++++- .../resources/InternalCommandStrings.resx | 60 +++++++++-------- .../Foreach-Object-Parallel.Tests.ps1 | 67 ++++++++++++++++++- 3 files changed, 117 insertions(+), 32 deletions(-) diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index cbf87e3b518..2b901b782ca 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -15,6 +15,7 @@ using System.Runtime.CompilerServices; using System.Text; using System.Threading; +using CommonParamSet = System.Management.Automation.Internal.CommonParameters; using Dbg = System.Management.Automation.Diagnostics; namespace Microsoft.PowerShell.Commands @@ -353,6 +354,21 @@ public void Dispose() private void InitParallelParameterSet() { + // The following common parameters are not (yet) supported in this parameter set. + // ErrorAction, WarningAction, InformationAction, PipelineVariable. + if (MyInvocation.BoundParameters.ContainsKey(nameof(CommonParamSet.ErrorAction)) || + MyInvocation.BoundParameters.ContainsKey(nameof(CommonParamSet.WarningAction)) || + MyInvocation.BoundParameters.ContainsKey(nameof(CommonParamSet.InformationAction)) || + MyInvocation.BoundParameters.ContainsKey(nameof(CommonParamSet.PipelineVariable))) + { + ThrowTerminatingError( + new ErrorRecord( + new PSNotSupportedException(InternalCommandStrings.ParallelCommonParametersNotSupported), + "ParallelCommonParametersNotSupported", + ErrorCategory.NotImplemented, + this)); + } + bool allowUsingExpression = this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage; _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary( Parallel, @@ -367,7 +383,7 @@ private void InitParallelParameterSet() { if (item is ScriptBlock) { - this.ThrowTerminatingError( + ThrowTerminatingError( new ErrorRecord( new PSArgumentException(InternalCommandStrings.ParallelUsingVariableCannotBeScriptBlock), "ParallelUsingVariableCannotBeScriptBlock", @@ -380,7 +396,7 @@ private void InitParallelParameterSet() { if (MyInvocation.BoundParameters.ContainsKey(nameof(TimeoutSeconds))) { - this.ThrowTerminatingError( + ThrowTerminatingError( new ErrorRecord( new PSArgumentException(InternalCommandStrings.ParallelCannotUseTimeoutWithJob), "ParallelCannotUseTimeoutWithJob", @@ -416,7 +432,7 @@ private void ProcessParallelParameterSet() // Validate piped InputObject if (_inputObject.BaseObject is ScriptBlock) { - this.WriteError( + WriteError( new ErrorRecord( new PSArgumentException(InternalCommandStrings.ParallelPipedInputObjectCannotBeScriptBlock), "ParallelPipedInputObjectCannotBeScriptBlock", diff --git a/src/System.Management.Automation/resources/InternalCommandStrings.resx b/src/System.Management.Automation/resources/InternalCommandStrings.resx index 857dd762664..153bb3e0bbb 100644 --- a/src/System.Management.Automation/resources/InternalCommandStrings.resx +++ b/src/System.Management.Automation/resources/InternalCommandStrings.resx @@ -1,17 +1,17 @@ - @@ -177,4 +177,8 @@ The 'TimeoutSeconds' parameter cannot be used with the 'AsJob' parameter. - + + The following common parameters are not currently supported in the Parallel parameter set: +ErrorAction, WarningAction, InformationAction, PipelineVariable + + \ No newline at end of file diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 16be0a7a623..85cb75475f6 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -106,7 +106,8 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { It 'Verifies error for script block piped variable' { - { $sb | ForEach-Object -Parallel { "Hello" } -ErrorAction Stop } | Should -Throw -ErrorId 'ParallelPipedInputObjectCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' + $actualError = $sb | ForEach-Object -Parallel { "Hello" } 2>&1 + $actualError.FullyQualifiedErrorId | Should -BeExactly 'ParallelPipedInputObjectCannotBeScriptBlock,Microsoft.PowerShell.Commands.ForEachObjectCommand' } It 'Verifies that parallel script blocks run in FullLanguage mode by default' { @@ -116,6 +117,70 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { } } +Describe 'ForEach-Object -Parallel common parameters' -Tags 'CI' { + + BeforeAll { + + # Test cases + $TestCasesNotSupportedCommonParameters = @( + @{ + testName = 'Verifies that ErrorAction common parameter is not supported' + scriptBlock = { 1..1 | ForEach-Object -Parallel { "Hello" } -ErrorAction Stop } + }, + @{ + testName = 'Verifies that WarningAction common parameter is not supported' + scriptBlock = { 1..1 | ForEach-Object -Parallel { "Hello" } -WarningAction SilentlyContinue } + }, + @{ + testName = 'Verifies that InformationAction common parameter is not supported' + scriptBlock = { 1..1 | ForEach-Object -Parallel { "Hello" } -InformationAction SilentlyContinue } + }, + @{ + testName = 'Verifies that PipelineVariable common parameter is not supported' + scriptBlock = { 1..1 | ForEach-Object -Parallel { "Hello" } -PipelineVariable pipeVar } + } + ) + + $TestCasesForSupportedCommonParameters = @( + @{ + testName = 'Verifies ErrorVariable common parameter' + scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel { Write-Error "Error:$_" } -ErrorVariable global:var } + expectedResult = 'Error:1' + }, + @{ + testName = 'Verifies WarningVarible common parameter' + scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel { Write-Warning "Warning:$_" } -WarningVariable global:var } + expectedResult = 'Warning:1' + }, + @{ + testName = 'Verifies InformationVariable common parameter' + scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel { Write-Information "Information:$_"} -InformationVariable global:var } + expectedResult = 'Information:1' + }, + @{ + testName = 'Verifies OutVariable common parameter' + scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel {Write-Output "Output:$_"} -OutVariable global:var } + expectedResult = 'Output:1' + } + ) + } + + It "" -TestCases $TestCasesNotSupportedCommonParameters { + + param ($scriptBlock) + + { & $scriptBlock } | Should -Throw -ErrorId 'ParallelCommonParametersNotSupported,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } + + It "" -TestCases $TestCasesForSupportedCommonParameters { + + param ($scriptBlock, $expectedResult) + + & $scriptBlock *>$null + $var[0].ToString() | Should -BeExactly $expectedResult + } +} + Describe 'ForEach-Object -Parallel -AsJob Basic Tests' -Tags 'CI' { BeforeAll { From 062f5b0b569b44fb23fd7a933322ad6b28310167 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 12 Aug 2019 11:19:58 -0700 Subject: [PATCH 24/27] Remove global var conflict with other test --- .../Foreach-Object-Parallel.Tests.ps1 | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 85cb75475f6..d70df77202a 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -144,27 +144,37 @@ Describe 'ForEach-Object -Parallel common parameters' -Tags 'CI' { $TestCasesForSupportedCommonParameters = @( @{ testName = 'Verifies ErrorVariable common parameter' - scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel { Write-Error "Error:$_" } -ErrorVariable global:var } + scriptBlock = { 1..1 | ForEach-Object -Parallel { Write-Error "Error:$_" } -ErrorVariable global:actualVariable } expectedResult = 'Error:1' }, @{ testName = 'Verifies WarningVarible common parameter' - scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel { Write-Warning "Warning:$_" } -WarningVariable global:var } + scriptBlock = { 1..1 | ForEach-Object -Parallel { Write-Warning "Warning:$_" } -WarningVariable global:actualVariable } expectedResult = 'Warning:1' }, @{ testName = 'Verifies InformationVariable common parameter' - scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel { Write-Information "Information:$_"} -InformationVariable global:var } + scriptBlock = { 1..1 | ForEach-Object -Parallel { Write-Information "Information:$_"} -InformationVariable global:actualVariable } expectedResult = 'Information:1' }, @{ testName = 'Verifies OutVariable common parameter' - scriptBlock = { $global:var = $null; 1..1 | ForEach-Object -Parallel {Write-Output "Output:$_"} -OutVariable global:var } + scriptBlock = { 1..1 | ForEach-Object -Parallel {Write-Output "Output:$_"} -OutVariable global:actualVariable } expectedResult = 'Output:1' } ) } + BeforeEach { + + $global:actualVariable = $null + } + + AfterAll { + + $global:actualVariable = $null + } + It "" -TestCases $TestCasesNotSupportedCommonParameters { param ($scriptBlock) @@ -177,7 +187,7 @@ Describe 'ForEach-Object -Parallel common parameters' -Tags 'CI' { param ($scriptBlock, $expectedResult) & $scriptBlock *>$null - $var[0].ToString() | Should -BeExactly $expectedResult + $global:actualVariable[0].ToString() | Should -BeExactly $expectedResult } } From e31cdf656cbfb091ad08521bb2a0c9febcf6adf8 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Mon, 12 Aug 2019 15:39:54 -0700 Subject: [PATCH 25/27] Fix CodeFlow comments --- .../engine/hostifaces/PSTask.cs | 116 ++++++++++-------- 1 file changed, 63 insertions(+), 53 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index bab0904dc82..76ecc378b6f 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -30,12 +30,12 @@ internal sealed class PSTask : PSTaskBase #region Constructor /// - /// Initializes new instance of PSTask. + /// Initializes a new instance of PSTask. /// - /// Script block to run in task - /// Using values passed into script block - /// Dollar underbar variable value - /// Cmdlet data stream writer + /// Script block to run in task. + /// Using values passed into script block. + /// Dollar underbar variable value. + /// Cmdlet data stream writer. public PSTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -171,12 +171,12 @@ internal sealed class PSJobTask : PSTaskBase #region Constructor /// - /// Initializes new instance PSJobTask. + /// Initializes a new instance PSJobTask. /// - /// Script block to run - /// Using variable values passed to script block - /// Dollar underbar variable value for script block - /// Job object associated with task + /// Script block to run. + /// Using variable values passed to script block. + /// Dollar underbar variable value for script block. + /// Job object associated with task. public PSJobTask( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -367,11 +367,11 @@ private PSTaskBase() } /// - /// Initializes instance of PSTaskBase. + /// Initializes a new instance of PSTaskBase. /// - /// Script block to run - /// Using variable values passed to script block - /// Dollar underbar variable value + /// Script block to run. + /// Using variable values passed to script block. + /// Dollar underbar variable value. protected PSTaskBase( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -443,7 +443,8 @@ public void Start() { _powershell.AddParameter(Parser.VERBATIM_ARGUMENT, _usingValuesMap); } - _powershell.BeginInvoke(input:null, output:_output); + + _powershell.BeginInvoke(input: null, output: _output); } /// @@ -482,6 +483,7 @@ internal sealed class PSTaskDataStreamWriter : IDisposable /// /// Gets wait-able handle that signals when new data has been added to /// the data stream collection. + /// Data added wait handle. internal WaitHandle DataAddedWaitHandle { get => _dataStream.WaitHandle; @@ -494,8 +496,9 @@ internal WaitHandle DataAddedWaitHandle private PSTaskDataStreamWriter() { } /// - /// Initializes instance of PSTaskDataStreamWriter. + /// Initializes a new instance of PSTaskDataStreamWriter. /// + /// Parent cmdlet. public PSTaskDataStreamWriter(PSCmdlet psCmdlet) { _cmdlet = psCmdlet; @@ -525,7 +528,7 @@ public void WriteImmediate() foreach (var item in _dataStream.ReadAll()) { - item.WriteStreamObject(cmdlet:_cmdlet, overrideInquire:true); + item.WriteStreamObject(cmdlet: _cmdlet, overrideInquire: true); } } @@ -601,8 +604,8 @@ internal sealed class PSTaskPool : IDisposable private readonly ManualResetEvent _stopAll; private readonly Dictionary _taskPool; private readonly int _sizeLimit; - private bool _isOpen; private readonly object _syncObject; + private bool _isOpen; #endregion @@ -611,9 +614,9 @@ internal sealed class PSTaskPool : IDisposable private PSTaskPool() { } /// - /// Initializes instance of PSTaskPool. + /// Initializes a new instance of PSTaskPool. /// - /// Total number of allowed running objects in pool at one time + /// Total number of allowed running objects in pool at one time. public PSTaskPool(int size) { _sizeLimit = size; @@ -638,7 +641,7 @@ public PSTaskPool(int size) #region Properties /// - /// Gets boolean indicating if pool is currently open for accepting tasks. + /// Gets a value indicating if pool is currently open for accepting tasks. /// public bool IsOpen { @@ -667,8 +670,9 @@ public void Dispose() /// If the pool is full, then this method blocks until space is available. /// This method is not multi-thread safe and assumes only one thread waits and adds tasks. /// - /// Task to be added to pool - /// Optional cmdlet data stream writer + /// Task to be added to pool. + /// Optional cmdlet data stream writer. + /// True when task is successfully added. public bool Add( PSTaskBase task, PSTaskDataStreamWriter dataStreamWriter = null) @@ -681,7 +685,8 @@ public bool Add( WaitHandle[] waitHandles; if (dataStreamWriter != null) { - waitHandles = new WaitHandle[] { + waitHandles = new WaitHandle[] + { _addAvailable, // index 0 _stopAll, // index 1 dataStreamWriter.DataAddedWaitHandle // index 2 @@ -689,7 +694,8 @@ public bool Add( } else { - waitHandles = new WaitHandle[] { + waitHandles = new WaitHandle[] + { _addAvailable, // index 0 _stopAll, // index 1 }; @@ -741,7 +747,8 @@ public bool Add( /// /// Add child job task to task pool. /// - /// Child job to be added to pool + /// Child job to be added to pool. + /// True when child job is successfully added. public bool Add(PSTaskChildJob childJob) { return Add(childJob.Task); @@ -821,8 +828,7 @@ private void CheckForComplete() { PoolComplete.SafeInvoke( this, - new EventArgs() - ); + new EventArgs()); } catch { @@ -856,10 +862,10 @@ internal sealed class PSTaskJob : Job private PSTaskJob() { } /// - /// Initializes instance of PSTaskJob. + /// Initializes a new instance of PSTaskJob. /// - /// Job command text - /// Pool size limit for task job + /// Job command text. + /// Pool size limit for task job. public PSTaskJob( string command, int throttleLimit) : base(command, string.Empty) @@ -925,6 +931,7 @@ public override void StopJob() /// /// Disposes task job. /// + /// Indicates disposing action. protected override void Dispose(bool disposing) { if (disposing) @@ -942,7 +949,8 @@ protected override void Dispose(bool disposing) /// /// Add a child job to the collection. /// - /// Child job to add + /// Child job to add. + /// True when child job is successfully added. public bool AddJob(PSTaskChildJob childJob) { if (!_isOpen) @@ -973,6 +981,7 @@ public void Start() { _taskPool.Add((PSTaskChildJob) childJob); } + _taskPool.Close(); }); } @@ -1003,7 +1012,8 @@ private void HandleTaskPoolComplete(object sender, EventArgs args) } SetJobState(finalState); } - catch (ObjectDisposedException) { } + catch (ObjectDisposedException) + { } } #endregion @@ -1026,10 +1036,10 @@ internal sealed class PSTaskChildDebugger : Debugger private PSTaskChildDebugger() { } /// - /// Initializes new instance of PSTaskChildDebugger. + /// Initializes a new instance of PSTaskChildDebugger. /// - /// Script debugger associated with task - /// Job name for associated task + /// Script debugger associated with task. + /// Job name for associated task. public PSTaskChildDebugger( Debugger debugger, string jobName) @@ -1055,9 +1065,9 @@ public PSTaskChildDebugger( /// Evaluates provided command either as a debugger specific command /// or a PowerShell command. /// - /// PowerShell command - /// Output - /// DebuggerCommandResults + /// PowerShell command. + /// PowerShell output. + /// Debugger command results. public override DebuggerCommandResults ProcessCommand( PSCommand command, PSDataCollection output) @@ -1083,7 +1093,7 @@ public override void SetBreakpoints(IEnumerable breakpoints) /// /// Sets the debugger resume action. /// - /// DebuggerResumeAction + /// DebuggerResumeAction. public override void SetDebuggerAction(DebuggerResumeAction resumeAction) { _wrappedDebugger.SetDebuggerAction(resumeAction); @@ -1101,7 +1111,7 @@ public override void StopProcessCommand() /// Returns current debugger stop event arguments if debugger is in /// debug stop state. Otherwise returns null. /// - /// DebuggerStopEventArgs + /// Debugger stop eventArgs. public override DebuggerStopEventArgs GetDebuggerStopArgs() { return _wrappedDebugger.GetDebuggerStopArgs(); @@ -1110,11 +1120,11 @@ public override DebuggerStopEventArgs GetDebuggerStopArgs() /// /// Sets the parent debugger, breakpoints, and other debugging context information. /// - /// Parent debugger - /// List of breakpoints - /// Debugger mode - /// PowerShell host - /// Current path + /// Parent debugger. + /// List of breakpoints. + /// Debugger mode. + /// PowerShell host. + /// Current path. public override void SetParent( Debugger parent, IEnumerable breakPoints, @@ -1129,7 +1139,7 @@ public override void SetParent( /// /// Sets the debugger mode. /// - /// Debugger mode to set + /// Debugger mode to set. public override void SetDebugMode(DebugModes mode) { _wrappedDebugger.SetDebugMode(mode); @@ -1140,7 +1150,7 @@ public override void SetDebugMode(DebugModes mode) /// /// Returns IEnumerable of CallStackFrame objects. /// - /// Enumerable call stack + /// Enumerable call stack. public override IEnumerable GetCallStack() { return _wrappedDebugger.GetCallStack(); @@ -1149,7 +1159,7 @@ public override IEnumerable GetCallStack() /// /// Sets debugger stepping mode. /// - /// True to enable debugger step mode + /// True to enable debugger step mode. public override void SetDebuggerStepMode(bool enabled) { _wrappedDebugger.SetDebuggerStepMode(enabled); @@ -1211,9 +1221,9 @@ private PSTaskChildJob() { } /// /// Initializes a new instance of PSTaskChildJob. /// - /// Script block to run - /// Using variable values passed to script block - /// Dollar underbar variable value + /// Script block to run. + /// Using variable values passed to script block. + /// Dollar underbar variable value. public PSTaskChildJob( ScriptBlock scriptBlock, Dictionary usingValuesMap, @@ -1303,7 +1313,7 @@ public Debugger Debugger } /// - /// Gets or sets IsAsync. + /// Gets or sets a value indicating IAsync. /// public bool IsAsync { get; set; } From 466ba4a4085d41ce0ef02eba68cc39a9ed44921b Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Tue, 13 Aug 2019 08:20:03 -0700 Subject: [PATCH 26/27] Another attempt to address style issues --- .../engine/ScriptCommandProcessor.cs | 4 +-- .../engine/hostifaces/Command.cs | 2 +- .../engine/hostifaces/PSTask.cs | 29 ++++++++++--------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs index 5b475ee6b98..5a5b2df239c 100644 --- a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs +++ b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs @@ -230,12 +230,12 @@ internal override bool IsHelpRequested(out string helpTarget, out HelpCategory h /// internal sealed class DlrScriptCommandProcessor : ScriptCommandProcessorBase { - private new ScriptBlock _scriptBlock; private readonly ArrayList _input = new ArrayList(); + private readonly object _dollarUnderbar = AutomationNull.Value; + private new ScriptBlock _scriptBlock; private MutableTuple _localsTuple; private bool _runOptimizedCode; private bool _argsBound; - private readonly object _dollarUnderbar = AutomationNull.Value; private FunctionContext _functionContext; internal DlrScriptCommandProcessor(ScriptBlock scriptBlock, ExecutionContext context, bool useNewScope, CommandOrigin origin, SessionStateInternal sessionState, object dollarUnderbar) diff --git a/src/System.Management.Automation/engine/hostifaces/Command.cs b/src/System.Management.Automation/engine/hostifaces/Command.cs index aafc93a150a..ae091e2ed59 100644 --- a/src/System.Management.Automation/engine/hostifaces/Command.cs +++ b/src/System.Management.Automation/engine/hostifaces/Command.cs @@ -174,7 +174,7 @@ internal bool? UseLocalScopeNullable } /// - /// DollarUnderbar ($_) value to be used with script command. + /// Gets or sets DollarUnderbar ($_) value to be used with script command. /// This is used by foreach-object -parallel where each piped input ($_) is associated /// with a parallel running script block. /// diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index 76ecc378b6f..fdeba10971d 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -30,7 +30,7 @@ internal sealed class PSTask : PSTaskBase #region Constructor /// - /// Initializes a new instance of PSTask. + /// Initializes a new instance of the class. /// /// Script block to run in task. /// Using values passed into script block. @@ -171,7 +171,7 @@ internal sealed class PSJobTask : PSTaskBase #region Constructor /// - /// Initializes a new instance PSJobTask. + /// Initializes a new instance class. /// /// Script block to run. /// Using variable values passed to script block. @@ -367,7 +367,7 @@ private PSTaskBase() } /// - /// Initializes a new instance of PSTaskBase. + /// Initializes a new instance of the class. /// /// Script block to run. /// Using variable values passed to script block. @@ -483,6 +483,7 @@ internal sealed class PSTaskDataStreamWriter : IDisposable /// /// Gets wait-able handle that signals when new data has been added to /// the data stream collection. + /// /// Data added wait handle. internal WaitHandle DataAddedWaitHandle { @@ -496,7 +497,7 @@ internal WaitHandle DataAddedWaitHandle private PSTaskDataStreamWriter() { } /// - /// Initializes a new instance of PSTaskDataStreamWriter. + /// Initializes a new instance of the class. /// /// Parent cmdlet. public PSTaskDataStreamWriter(PSCmdlet psCmdlet) @@ -513,7 +514,7 @@ public PSTaskDataStreamWriter(PSCmdlet psCmdlet) /// /// Add data stream object to the writer. /// - /// Data stream object to write + /// Data stream object to write. public void Add(PSStreamObject streamObject) { _dataStream.Add(streamObject); @@ -614,7 +615,7 @@ internal sealed class PSTaskPool : IDisposable private PSTaskPool() { } /// - /// Initializes a new instance of PSTaskPool. + /// Initializes a new instance of the class. /// /// Total number of allowed running objects in pool at one time. public PSTaskPool(int size) @@ -641,7 +642,7 @@ public PSTaskPool(int size) #region Properties /// - /// Gets a value indicating if pool is currently open for accepting tasks. + /// Gets a value indicating whether a pool is currently open for accepting tasks. /// public bool IsOpen { @@ -807,6 +808,7 @@ private void HandleTaskStateChanged(object sender, PSInvocationStateChangedEvent _addAvailable.Set(); } } + task.StateChanged -= HandleTaskStateChangedDelegate; task.Dispose(); CheckForComplete(); @@ -862,7 +864,7 @@ internal sealed class PSTaskJob : Job private PSTaskJob() { } /// - /// Initializes a new instance of PSTaskJob. + /// Initializes a new instance of the class. /// /// Job command text. /// Pool size limit for task job. @@ -979,7 +981,7 @@ public void Start() { foreach (var childJob in ChildJobs) { - _taskPool.Add((PSTaskChildJob) childJob); + _taskPool.Add((PSTaskChildJob)childJob); } _taskPool.Close(); @@ -1010,6 +1012,7 @@ private void HandleTaskPoolComplete(object sender, EventArgs args) break; } } + SetJobState(finalState); } catch (ObjectDisposedException) @@ -1036,7 +1039,7 @@ internal sealed class PSTaskChildDebugger : Debugger private PSTaskChildDebugger() { } /// - /// Initializes a new instance of PSTaskChildDebugger. + /// Initializes a new instance of the class. /// /// Script debugger associated with task. /// Job name for associated task. @@ -1084,7 +1087,7 @@ public override DebuggerCommandResults ProcessCommand( /// /// Adds the provided set of breakpoints to the debugger. /// - /// Breakpoints. + /// List of breakpoints. public override void SetBreakpoints(IEnumerable breakpoints) { _wrappedDebugger.SetBreakpoints(breakpoints); @@ -1219,7 +1222,7 @@ internal sealed class PSTaskChildJob : Job, IJobDebugger private PSTaskChildJob() { } /// - /// Initializes a new instance of PSTaskChildJob. + /// Initializes a new instance of the class. /// /// Script block to run. /// Using variable values passed to script block. @@ -1313,7 +1316,7 @@ public Debugger Debugger } /// - /// Gets or sets a value indicating IAsync. + /// Gets or sets a value indicating whether IAsync. /// public bool IsAsync { get; set; } From 86dd9a5bb32b018063417b151c8e9e399f0c64a7 Mon Sep 17 00:00:00 2001 From: Paul Higinbotham Date: Tue, 13 Aug 2019 08:34:19 -0700 Subject: [PATCH 27/27] More style changes --- src/System.Management.Automation/engine/hostifaces/PSTask.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/PSTask.cs b/src/System.Management.Automation/engine/hostifaces/PSTask.cs index fdeba10971d..61f66d75e5d 100644 --- a/src/System.Management.Automation/engine/hostifaces/PSTask.cs +++ b/src/System.Management.Automation/engine/hostifaces/PSTask.cs @@ -171,7 +171,7 @@ internal sealed class PSJobTask : PSTaskBase #region Constructor /// - /// Initializes a new instance class. + /// Initializes a new instance of the class. /// /// Script block to run. /// Using variable values passed to script block. @@ -1096,7 +1096,7 @@ public override void SetBreakpoints(IEnumerable breakpoints) /// /// Sets the debugger resume action. /// - /// DebuggerResumeAction. + /// Debugger resume action. public override void SetDebuggerAction(DebuggerResumeAction resumeAction) { _wrappedDebugger.SetDebuggerAction(resumeAction);