Skip to content

Commit

Permalink
Changes to SteppablePipeline to support 'Clean' block
Browse files Browse the repository at this point in the history
  • Loading branch information
@daxian-dbw
daxian-dbw committed Apr 21, 2021
1 parent 162274c commit 88c8aa5
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 88 deletions.
14 changes: 9 additions & 5 deletions src/System.Management.Automation/engine/CommandMetadata.cs
View file
Expand Up @@ -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($_)
Expand Down Expand Up @@ -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()
";
}

Expand Down
39 changes: 27 additions & 12 deletions src/System.Management.Automation/engine/lang/scriptblock.cs
View file
Expand Up @@ -1280,7 +1280,32 @@ public Array End()
{
// then pop this pipeline and dispose it...
_context.PopPipelineProcessor(true);
_pipeline.Dispose();
Dispose();
}
}

/// <summary>
/// Clean resources for script commands of this steppable pipeline.
/// </summary>
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();
}
}

Expand All @@ -1293,23 +1318,13 @@ public Array End()
/// When this object is disposed, the contained pipeline should also be disposed.
/// </summary>
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

private void Dispose(bool disposing)
{
if (_disposed)
{
return;
}

if (disposing)
{
_pipeline.Dispose();
}

_pipeline.Dispose();
_disposed = true;
}

Expand Down
145 changes: 74 additions & 71 deletions src/System.Management.Automation/engine/pipeline.cs
View file
Expand Up @@ -66,7 +66,6 @@ internal class PipelineProcessor : IDisposable
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

private void Dispose(bool disposing)
Expand Down Expand Up @@ -689,9 +688,9 @@ private void DoCompleteCore(CommandProcessorBase commandRequestingUpstreamComman
/// <summary>
/// Clean up resources for script commands in this pipeline processor.
/// </summary>
private void Clean()
internal void Clean()
{
if (_commands is null)
if (!_executionStarted || _commands is null)
{
return;
}
Expand Down Expand Up @@ -752,19 +751,19 @@ private void Clean()
/// <returns>The results of the execution.</returns>
internal Array DoComplete()
{
if (!_executionStarted)
{
throw PSTraceSource.NewInvalidOperationException(
PipelineStrings.PipelineNotStarted);
}

try
{
if (Stopping)
{
throw new PipelineStoppedException();
}

if (!_executionStarted)
{
throw PSTraceSource.NewInvalidOperationException(
PipelineStrings.PipelineNotStarted);
}

ExceptionDispatchInfo toRethrowInfo;
try
{
Expand All @@ -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();

Expand All @@ -788,36 +787,42 @@ internal Array DoComplete()
}
finally
{
Clean();
DisposeCommands();
}
}

/// <summary>
/// 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.
/// </summary>
/// <param name="expectInput">True if you want to write to this pipeline.</param>
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;
}
}
Expand All @@ -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<CommandProcessorBase> 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();
Expand Down Expand Up @@ -900,36 +908,30 @@ internal void Stop()
/// </exception>
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;
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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
Expand Down

0 comments on commit 88c8aa5

Please sign in to comment.