From e90ebbbcc03012ca55814f0ff78f9c5c3c1a416b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 7 Apr 2021 15:58:34 -0700 Subject: [PATCH] Iron out the test failures --- .../engine/MshCommandRuntime.cs | 4 +- .../engine/Pipe.cs | 5 ++ .../engine/pipeline.cs | 88 ++++++++++--------- .../engine/runtime/CompiledScriptBlock.cs | 10 ++- ...ts.ps1 => PipelineBehaviour.Tests.ps1.bak} | 0 5 files changed, 61 insertions(+), 46 deletions(-) rename test/powershell/Language/Scripting/{PipelineBehaviour.Tests.ps1 => PipelineBehaviour.Tests.ps1.bak} (100%) diff --git a/src/System.Management.Automation/engine/MshCommandRuntime.cs b/src/System.Management.Automation/engine/MshCommandRuntime.cs index e500795e511f9..1f5d9b2de172e 100644 --- a/src/System.Management.Automation/engine/MshCommandRuntime.cs +++ b/src/System.Management.Automation/engine/MshCommandRuntime.cs @@ -3732,7 +3732,7 @@ internal void SetVariableListsInPipe() { Diagnostics.Assert(_thisCommand is PSScriptCmdlet, "this is only done for script cmdlets"); - if (_outVarList != null && !OutputPipe.NullPipe) + if (_outVarList != null && !OutputPipe.IgnoreOutVariableList) { // A null pipe is used when executing the 'Clean' block of a PSScriptCmdlet. // In such a case, we don't capture output to the out variable list. @@ -3775,7 +3775,7 @@ internal void RemoveVariableListsInPipe() { // Diagnostics.Assert(thisCommand is PSScriptCmdlet, "this is only done for script cmdlets"); - if (_outVarList != null) + if (_outVarList != null && !OutputPipe.IgnoreOutVariableList) { this.OutputPipe.RemoveVariableList(VariableStreamKind.Output, _outVarList); } diff --git a/src/System.Management.Automation/engine/Pipe.cs b/src/System.Management.Automation/engine/Pipe.cs index 8ee000a4faf92..6dfc372543f05 100644 --- a/src/System.Management.Automation/engine/Pipe.cs +++ b/src/System.Management.Automation/engine/Pipe.cs @@ -109,6 +109,11 @@ public override string ToString() /// internal int OutBufferCount { get; set; } = 0; + /// + /// Gets whether the out variable list should be ignored. + /// + internal bool IgnoreOutVariableList { get; set; } + /// /// If true, then all input added to this pipe will simply be discarded... /// diff --git a/src/System.Management.Automation/engine/pipeline.cs b/src/System.Management.Automation/engine/pipeline.cs index 3ac852513d974..1d9353d05e1f6 100644 --- a/src/System.Management.Automation/engine/pipeline.cs +++ b/src/System.Management.Automation/engine/pipeline.cs @@ -7,7 +7,6 @@ using System.Management.Automation.Tracing; using System.Reflection; using System.Runtime.ExceptionServices; -using System.Runtime.InteropServices; using Microsoft.PowerShell.Telemetry; using Dbg = System.Management.Automation.Diagnostics; @@ -539,11 +538,8 @@ internal Array SynchronousExecuteEnumerate(object input) // the call to 'pipelineProcessor.Step'. // It's possible (though very unlikely) that the call to 'pipelineProcessor.Step' failed with an // exception, and in such case, the 'pipelineProcessor' would have been disposed, and therefore - // we should skip the 'DoComplete' call on it. - if (!redirectPipelineProcessor._stopping) - { - redirectPipelineProcessor.DoCompleteCore(null); - } + // the call to 'DoComplete' will simply return, because '_commands' was already set to null. + redirectPipelineProcessor.DoCompleteCore(null); } } @@ -584,6 +580,10 @@ private void DoCompleteCore(CommandProcessorBase commandRequestingUpstreamComman { if (_commands is null) { + // This could happen to a redirection pipeline, either for an expression (e.g. 1 > a.txt) + // or for a command (e.g. command > a.txt). + // An exception may be thrown from the call to 'StartStepping' or 'Step' on the pipeline, + // which causes the pipeline commands to be disposed. return; } @@ -657,13 +657,6 @@ private void DoCompleteCore(CommandProcessorBase commandRequestingUpstreamComman lastCommandRuntime.PipelineProcessor.LogPipelineComplete(); } } - - // If a terminating error occurred, report it now. - if (_firstTerminatingError != null) - { - this.LogExecutionException(_firstTerminatingError.SourceException); - _firstTerminatingError.Throw(); - } } /// @@ -732,42 +725,44 @@ private void Clean() /// The results of the execution. internal Array DoComplete() { - if (Stopping) + try { - throw new PipelineStoppedException(); - } + if (Stopping) + { + throw new PipelineStoppedException(); + } - if (!_executionStarted) - { - throw PSTraceSource.NewInvalidOperationException( - PipelineStrings.PipelineNotStarted); - } + if (!_executionStarted) + { + throw PSTraceSource.NewInvalidOperationException( + PipelineStrings.PipelineNotStarted); + } - ExceptionDispatchInfo toRethrowInfo; - try - { - DoCompleteCore(null); + ExceptionDispatchInfo toRethrowInfo; + try + { + DoCompleteCore(null); + return RetrieveResults(); + } + catch (RuntimeException e) + { + toRethrowInfo = GetFirstError(e); + } - return RetrieveResults(); - } - catch (RuntimeException e) - { - toRethrowInfo = GetFirstError(e); + // By rethrowing the exception outside of the handler, we allow the CLR on X64/IA64 to free from the stack + // the exception records related to this exception. + + // The only reason we should get here is if an exception should be rethrown. + Diagnostics.Assert(toRethrowInfo != null, "Alternate protocol path failure"); + toRethrowInfo.Throw(); + + // UNREACHABLE + return null; } finally { DisposeCommands(); } - - // By rethrowing the exception outside of the handler, - // we allow the CLR on X64/IA64 to free from the stack - // the exception records related to this exception. - - // The only reason we should get here is if - // an exception should be rethrown. - Diagnostics.Assert(toRethrowInfo != null, "Alternate protocol path failure"); - toRethrowInfo.Throw(); - return null; // UNREACHABLE } /// @@ -1193,6 +1188,14 @@ private void Inject(object input, bool enumerate) /// private Array RetrieveResults() { + if (_commands is null) + { + // This could happen to an expression redirection pipeline (e.g. 1 > a.txt). + // An exception may be thrown from the call to 'StartStepping' or 'Step' on the pipeline, + // which causes the pipeline commands to be disposed. + return MshCommandRuntime.StaticEmptyArray; + } + // If the error queue has been linked, it's up to the link to // deal with the output. Don't do anything here... if (!_linkedErrorOutput) @@ -1214,17 +1217,17 @@ private Array RetrieveResults() // If the success queue has been linked, it's up to the link to // deal with the output. Don't do anything here... if (_linkedSuccessOutput) + { return MshCommandRuntime.StaticEmptyArray; + } CommandProcessorBase LastCommandProcessor = _commands[_commands.Count - 1]; ValidateCommandProcessorNotNull(LastCommandProcessor, errorMessage: null); Array results = LastCommandProcessor.CommandRuntime.GetResultsAsArray(); - // 2003/10/02-JonN // Do not return the same results more than once LastCommandProcessor.CommandRuntime.OutputPipe.Clear(); - return results is null ? MshCommandRuntime.StaticEmptyArray : results; } @@ -1288,6 +1291,7 @@ private void DisposeCommands() // If Dispose throws an exception, record it as a pipeline failure and continue disposing cmdlets. try { + commandProcessor.CommandRuntime.RemoveVariableListsInPipe(); commandProcessor.Dispose(); } // The only vaguely plausible reason for a failure here is an exception in Command.Dispose. diff --git a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs index 5b673c05f8ae3..474f4d5d173e9 100644 --- a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs +++ b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs @@ -2319,9 +2319,15 @@ internal override void DoCleanResource() { if (_scriptBlock.HasCleanupBlock && _anyClauseExecuted) { - // The 'Clean' block doesn't write to pipeline. + // The 'Clean' block doesn't write any output to pipeline, so we use a 'NullPipe' here and + // disallow the output to be collected by an 'out' variable. However, the error, warning, + // and information records should still be collectable by the corresponding variables. Pipe oldOutputPipe = _commandRuntime.OutputPipe; - _functionContext._outputPipe = _commandRuntime.OutputPipe = new Pipe { NullPipe = true }; + _functionContext._outputPipe = _commandRuntime.OutputPipe = new Pipe + { + NullPipe = true, + IgnoreOutVariableList = true, + }; try { diff --git a/test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1 b/test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1.bak similarity index 100% rename from test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1 rename to test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1.bak