diff --git a/src/System.Management.Automation/engine/CommandMetadata.cs b/src/System.Management.Automation/engine/CommandMetadata.cs
index b29f784b448db..bfe5ca5f7a924 100644
--- a/src/System.Management.Automation/engine/CommandMetadata.cs
+++ b/src/System.Management.Automation/engine/CommandMetadata.cs
@@ -1067,6 +1067,11 @@ internal string GetBeginBlock()
internal string GetProcessBlock()
{
+ // The reason we wrap scripts in 'try { } catch { throw }' (here and elsewhere) is to turn
+ // an exception that could be thrown from .NET method invocation into a terminating error
+ // that can be propagated up.
+ // By default, an exception thrown from .NET method is not terminating, but when enclosed
+ // in try/catch, it will be turned into a terminating error.
return @"
try {
$steppablePipeline.Process($_)
@@ -1119,12 +1124,11 @@ internal string GetEndBlock()
internal string GetCleanupBlock()
{
+ // Here we don't need to enclose the script in a 'try/catch' like elsewhere, because
+ // 1. the 'Clean' block doesn't propagate up any exception (terminating error);
+ // 2. only one expression in the script, so nothing else needs to be stopped when invoking the method fails.
return @"
- try {
- $steppablePipeline.Dispose()
- } catch {
- throw
- }
+ $steppablePipeline.Clean()
";
}
diff --git a/src/System.Management.Automation/engine/lang/scriptblock.cs b/src/System.Management.Automation/engine/lang/scriptblock.cs
index 0c6c1d5dff1a7..e74fada27e908 100644
--- a/src/System.Management.Automation/engine/lang/scriptblock.cs
+++ b/src/System.Management.Automation/engine/lang/scriptblock.cs
@@ -1280,7 +1280,32 @@ public Array End()
{
// then pop this pipeline and dispose it...
_context.PopPipelineProcessor(true);
- _pipeline.Dispose();
+ Dispose();
+ }
+ }
+
+ ///
+ /// Clean resources for script commands of this steppable pipeline.
+ ///
+ public void Clean()
+ {
+ if (_pipeline.Commands is null)
+ {
+ // The pipeline commands have been disposed. In this case,
+ // 'Clean' should have already been called.
+ return;
+ }
+
+ try
+ {
+ _context.PushPipelineProcessor(_pipeline);
+ _pipeline.Clean();
+ }
+ finally
+ {
+ // then pop this pipeline and dispose it...
+ _context.PopPipelineProcessor(true);
+ Dispose();
}
}
@@ -1293,23 +1318,13 @@ public Array End()
/// When this object is disposed, the contained pipeline should also be disposed.
///
public void Dispose()
- {
- Dispose(true);
- GC.SuppressFinalize(this);
- }
-
- private void Dispose(bool disposing)
{
if (_disposed)
{
return;
}
- if (disposing)
- {
- _pipeline.Dispose();
- }
-
+ _pipeline.Dispose();
_disposed = true;
}
diff --git a/src/System.Management.Automation/engine/pipeline.cs b/src/System.Management.Automation/engine/pipeline.cs
index a1ff65b2cba7d..7f2d67c9ac96f 100644
--- a/src/System.Management.Automation/engine/pipeline.cs
+++ b/src/System.Management.Automation/engine/pipeline.cs
@@ -66,7 +66,6 @@ internal class PipelineProcessor : IDisposable
public void Dispose()
{
Dispose(true);
- GC.SuppressFinalize(this);
}
private void Dispose(bool disposing)
@@ -689,9 +688,9 @@ private void DoCompleteCore(CommandProcessorBase commandRequestingUpstreamComman
///
/// Clean up resources for script commands in this pipeline processor.
///
- private void Clean()
+ internal void Clean()
{
- if (_commands is null)
+ if (!_executionStarted || _commands is null)
{
return;
}
@@ -752,6 +751,12 @@ private void Clean()
/// The results of the execution.
internal Array DoComplete()
{
+ if (!_executionStarted)
+ {
+ throw PSTraceSource.NewInvalidOperationException(
+ PipelineStrings.PipelineNotStarted);
+ }
+
try
{
if (Stopping)
@@ -759,12 +764,6 @@ internal Array DoComplete()
throw new PipelineStoppedException();
}
- if (!_executionStarted)
- {
- throw PSTraceSource.NewInvalidOperationException(
- PipelineStrings.PipelineNotStarted);
- }
-
ExceptionDispatchInfo toRethrowInfo;
try
{
@@ -779,7 +778,7 @@ internal Array DoComplete()
// 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.
+ // The only reason we should get here is an exception should be rethrown.
Diagnostics.Assert(toRethrowInfo != null, "Alternate protocol path failure");
toRethrowInfo.Throw();
@@ -788,36 +787,42 @@ internal Array DoComplete()
}
finally
{
+ Clean();
DisposeCommands();
}
}
///
- /// This routine starts the stepping process. It is optional to
- /// call this but can be useful if you want the begin clauses
- /// of the pipeline to be run even when there may not be any input
- /// to process as is the case for I/O redirection into a file. We
- /// still want the file opened, even if there was nothing to write to it.
+ /// This routine starts the stepping process. It is optional to call this but can be useful
+ /// if you want the begin clauses of the pipeline to be run even when there may not be any
+ /// input to process as is the case for I/O redirection into a file. We still want the file
+ /// opened, even if there was nothing to write to it.
///
/// True if you want to write to this pipeline.
internal void StartStepping(bool expectInput)
{
+ bool startSucceeded = false;
try
{
Start(expectInput);
+ startSucceeded = true;
// Check if this pipeline is being stopped asynchronously.
ThrowFirstErrorIfExisting(logException: false);
}
- catch (PipelineStoppedException)
+ catch (Exception e)
{
+ Clean();
DisposeCommands();
- // The error we want to report is the first terminating error
- // which occurred during pipeline execution, regardless
- // of whether other errors occurred afterward.
- ThrowFirstErrorIfExisting(logException: false);
+ if (!startSucceeded && e is PipelineStoppedException)
+ {
+ // We want to report the first terminating error which occurred during pipeline execution,
+ // regardless of whether other errors occurred afterward.
+ ThrowFirstErrorIfExisting(logException: false);
+ }
+ // If we haven't recorded the first terminating error, then re-throw this exception.
throw;
}
}
@@ -832,23 +837,26 @@ internal void Stop()
// Only call StopProcessing if the pipeline is being stopped
// for the first time
- if (!RecordFailure(new PipelineStoppedException(), null))
+ if (!RecordFailure(new PipelineStoppedException(), command: null))
+ {
return;
+ }
// Retain copy of _commands in case Dispose() is called
List commands = _commands;
- if (commands == null)
+ if (commands is null)
+ {
return;
+ }
// Call StopProcessing() for all the commands.
- for (int i = 0; i < commands.Count; i++)
+ foreach (CommandProcessorBase commandProcessor in commands)
{
- CommandProcessorBase commandProcessor = commands[i];
-
if (commandProcessor == null)
{
throw PSTraceSource.NewInvalidOperationException();
}
+
try
{
commandProcessor.Command.DoStopProcessing();
@@ -900,36 +908,30 @@ internal void Stop()
///
internal Array Step(object input)
{
- if (Stopping)
- {
- DisposeCommands();
- throw new PipelineStoppedException();
- }
-
+ bool injectSucceeded = false;
try
{
Start(true);
Inject(input, enumerate: false);
+ injectSucceeded = true;
// Check if this pipeline is being stopped asynchronously.
ThrowFirstErrorIfExisting(logException: false);
-
return RetrieveResults();
}
- catch (PipelineStoppedException)
+ catch (Exception e)
{
+ Clean();
DisposeCommands();
- // The error we want to report is the first terminating error
- // which occurred during pipeline execution, regardless
- // of whether other errors occurred afterward.
- ThrowFirstErrorIfExisting(logException: false);
+ if (!injectSucceeded && e is PipelineStoppedException)
+ {
+ // We want to report the first terminating error which occurred during pipeline execution,
+ // regardless of whether other errors occurred afterward.
+ ThrowFirstErrorIfExisting(logException: false);
+ }
- throw;
- }
- catch (Exception)
- {
- DisposeCommands();
+ // If we haven't recorded the first terminating error, then re-throw this exception.
throw;
}
}
@@ -1296,41 +1298,40 @@ private void DisposeCommands()
LogToEventLog();
- if (_commands != null)
+ if (_commands is not null)
{
- for (int i = 0; i < _commands.Count; i++)
+ foreach (CommandProcessorBase commandProcessor in _commands)
{
- CommandProcessorBase commandProcessor = _commands[i];
- if (commandProcessor != null)
+ if (commandProcessor is null)
+ {
+ continue;
+ }
+
+ // If Dispose throws an exception, record it as a pipeline failure and continue disposing cmdlets.
+ try
{
- // If Dispose throws an exception, record it as a pipeline failure and continue disposing cmdlets.
- try
+ commandProcessor.CommandRuntime.RemoveVariableListsInPipe();
+ commandProcessor.Dispose();
+ }
+ catch (Exception e)
+ {
+ // The only vaguely plausible reason for a failure here is an exception in 'Command.Dispose'.
+ // As such, this should be covered by the overall exemption.
+ InvocationInfo myInvocation = commandProcessor.Command?.MyInvocation;
+
+ if (e is ProviderInvocationException pie)
{
- commandProcessor.CommandRuntime.RemoveVariableListsInPipe();
- commandProcessor.Dispose();
+ e = new CmdletProviderInvocationException(pie, myInvocation);
}
- // The only vaguely plausible reason for a failure here is an exception in Command.Dispose.
- // As such, this should be covered by the overall exemption.
- catch (Exception e)
+ else
{
- InvocationInfo myInvocation = null;
- if (commandProcessor.Command != null)
- myInvocation = commandProcessor.Command.MyInvocation;
-
- if (e is ProviderInvocationException pie)
- {
- e = new CmdletProviderInvocationException(pie, myInvocation);
- }
- else
- {
- e = new CmdletInvocationException(e, myInvocation);
-
- // Log a command health event
- MshLog.LogCommandHealthEvent(commandProcessor.Command.Context, e, Severity.Warning);
- }
-
- RecordFailure(e, commandProcessor.Command);
+ e = new CmdletInvocationException(e, myInvocation);
+
+ // Log a command health event
+ MshLog.LogCommandHealthEvent(commandProcessor.Command.Context, e, Severity.Warning);
}
+
+ RecordFailure(e, commandProcessor.Command);
}
}
}
@@ -1348,7 +1349,9 @@ private void DisposeCommands()
}
// Clean resources for script commands.
- // This method catches and handles all exceptions inside, so it will never throw.
+ // It is possible (though very unlikely) that the call to 'Step' on the redirection pipeline failed.
+ // In such a case, 'Clean' would have run and the 'pipelineProcessor' would have been disposed.
+ // Therefore, calling 'Clean' again will simply return, because '_commands' was already set to null.
redirPipe.Clean();
// The complicated logic of disposing the commands is taken care