Skip to content

Commit

Permalink
Add clean block to script block as a peer to begin, process, an…
Browse files Browse the repository at this point in the history
…d `end` to allow easy resource cleanup (PowerShell#15177)
  • Loading branch information
daxian-dbw authored and TrapGodBrim committed Jan 19, 2022
1 parent fd71fe6 commit 1833766
Show file tree
Hide file tree
Showing 30 changed files with 3,034 additions and 623 deletions.
Expand Up @@ -2823,13 +2823,14 @@ private void GenerateHelperFunctions(TextWriter writer)
$clientSideParameters = Get-PSImplicitRemotingClientSideParameters $PSBoundParameters ${8}
$scriptCmd = {{ & $script:InvokeCommand `
@clientSideParameters `
-HideComputerName `
-Session (Get-PSImplicitRemotingSession -CommandName '{0}') `
-Arg ('{0}', $PSBoundParameters, $positionalArguments) `
-Script {{ param($name, $boundParams, $unboundParams) & $name @boundParams @unboundParams }} `
}}
$scriptCmd = {{
& $script:InvokeCommand `
@clientSideParameters `
-HideComputerName `
-Session (Get-PSImplicitRemotingSession -CommandName '{0}') `
-Arg ('{0}', $PSBoundParameters, $positionalArguments) `
-Script {{ param($name, $boundParams, $unboundParams) & $name @boundParams @unboundParams }} `
}}
$steppablePipeline = $scriptCmd.GetSteppablePipeline($myInvocation.CommandOrigin)
$steppablePipeline.Begin($myInvocation.ExpectingInput, $ExecutionContext)
Expand Down
7 changes: 7 additions & 0 deletions src/System.Management.Automation/engine/CommandBase.cs
Expand Up @@ -233,6 +233,13 @@ internal virtual void DoStopProcessing()
{
}

/// <summary>
/// When overridden in the derived class, performs clean-up after the command execution.
/// </summary>
internal virtual void DoCleanResource()
{
}

#endregion Override

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/System.Management.Automation/engine/CommandInfo.cs
Expand Up @@ -529,7 +529,7 @@ private void GetMergedCommandParameterMetadata(out MergedCommandParameterMetadat
processor = scriptCommand != null
? new CommandProcessor(scriptCommand, _context, useLocalScope: true, fromScriptFile: false,
sessionState: scriptCommand.ScriptBlock.SessionStateInternal ?? Context.EngineSessionState)
: new CommandProcessor((CmdletInfo)this, _context) { UseLocalScope = true };
: new CommandProcessor((CmdletInfo)this, _context);

ParameterBinderController.AddArgumentsToCommandProcessor(processor, Arguments);
CommandProcessorBase oldCurrentCommandProcessor = Context.CurrentCommandProcessor;
Expand Down
21 changes: 20 additions & 1 deletion src/System.Management.Automation/engine/CommandMetadata.cs
Expand Up @@ -875,8 +875,11 @@ internal string GetProxyCommand(string helpComment, bool generateDynamicParamete
end
{{{5}}}
clean
{{{6}}}
<#
{6}
{7}
#>
",
GetDecl(),
Expand All @@ -885,6 +888,7 @@ internal string GetProxyCommand(string helpComment, bool generateDynamicParamete
GetBeginBlock(),
GetProcessBlock(),
GetEndBlock(),
GetCleanBlock(),
CodeGeneration.EscapeBlockCommentContent(helpComment));

return result;
Expand Down Expand Up @@ -1063,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 @@ -1113,6 +1122,16 @@ internal string GetEndBlock()
";
}

internal string GetCleanBlock()
{
// 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 @"
$steppablePipeline.Clean()
";
}

#endregion

#region Helper methods for restricting commands needed by implicit and interactive remoting
Expand Down
17 changes: 6 additions & 11 deletions src/System.Management.Automation/engine/CommandProcessor.cs
Expand Up @@ -309,13 +309,11 @@ internal override void DoBegin()
internal override void ProcessRecord()
{
// Invoke the Command method with the request object

if (!this.RanBeginAlready)
{
RanBeginAlready = true;
try
{
// NOTICE-2004/06/08-JonN 959638
using (commandRuntime.AllowThisCommandToWrite(true))
{
if (Context._debuggingMode > 0 && Command is not PSScriptCmdlet)
Expand All @@ -326,12 +324,9 @@ internal override void ProcessRecord()
Command.DoBeginProcessing();
}
}
// 2004/03/18-JonN This is understood to be
// an FXCOP violation, cleared by KCwalina.
catch (Exception e) // Catch-all OK, 3rd party callout.
catch (Exception e)
{
// This cmdlet threw an exception, so
// wrap it and bubble it up.
// This cmdlet threw an exception, so wrap it and bubble it up.
throw ManageInvocationException(e);
}
}
Expand Down Expand Up @@ -366,6 +361,7 @@ internal override void ProcessRecord()

// NOTICE-2004/06/08-JonN 959638
using (commandRuntime.AllowThisCommandToWrite(true))
using (ParameterBinderBase.bindingTracer.TraceScope("CALLING ProcessRecord"))
{
if (CmdletParameterBinderController.ObsoleteParameterWarningList != null &&
CmdletParameterBinderController.ObsoleteParameterWarningList.Count > 0)
Expand Down Expand Up @@ -400,14 +396,13 @@ internal override void ProcessRecord()
}
catch (LoopFlowException)
{
// Win8:84066 - Don't wrap LoopFlowException, we incorrectly raise a PipelineStoppedException
// Don't wrap LoopFlowException, we incorrectly raise a PipelineStoppedException
// which gets caught by a script try/catch if we wrap here.
throw;
}
// 2004/03/18-JonN This is understood to be
// an FXCOP violation, cleared by KCwalina.
catch (Exception e) // Catch-all OK, 3rd party callout.
catch (Exception e)
{
// Catch-all OK, 3rd party callout.
exceptionToThrow = e;
}
finally
Expand Down

0 comments on commit 1833766

Please sign in to comment.