From aabc7f8b176e49444b12e74aa1cef72c8f6bdb06 Mon Sep 17 00:00:00 2001 From: Joel Sallow <32407840+vexx32@users.noreply.github.com> Date: Thu, 14 May 2020 00:46:12 -0400 Subject: [PATCH 01/36] :sparkles: Add cleanup block --- .../utility/ImplicitRemotingCommands.cs | 24 +- .../engine/CommandMetadata.cs | 17 +- .../engine/CommandProcessorBase.cs | 123 ++++++++--- .../engine/EventManager.cs | 5 + .../engine/ProxyCommand.cs | 24 ++ .../engine/ScriptCommandProcessor.cs | 182 ++++++++++++--- .../engine/debugger/Breakpoint.cs | 11 +- .../engine/hostifaces/LocalPipeline.cs | 48 +++- .../engine/parser/Compiler.cs | 9 + .../engine/parser/Parser.cs | 33 ++- .../engine/parser/SemanticChecks.cs | 9 +- .../engine/parser/TypeInferenceVisitor.cs | 6 + .../engine/parser/VariableAnalysis.cs | 5 + .../engine/parser/ast.cs | 207 ++++++++++++++++-- .../engine/parser/token.cs | 14 +- .../engine/parser/tokenizer.cs | 28 +-- .../engine/pipeline.cs | 12 +- .../engine/runtime/CompiledScriptBlock.cs | 162 ++++++++++++-- .../engine/runtime/Operations/MiscOps.cs | 17 +- .../resources/ParserStrings.resx | 2 +- .../Language/Parser/Parser.Tests.ps1 | 2 + .../Language/Parser/Parsing.Tests.ps1 | 6 +- .../Scripting/PipelineBehaviour.Tests.ps1 | 122 +++++++++++ 23 files changed, 907 insertions(+), 161 deletions(-) create mode 100644 test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1 diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs index 325632c44cee..33c1a803dd40 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ImplicitRemotingCommands.cs @@ -2821,15 +2821,16 @@ private void GenerateHelperFunctions(TextWriter writer) $positionalArguments.AddRange($args) - $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 }} ` - }} + $clientSideParameters = Get-PSImplicitRemotingClientSideParameters $PSBoundParameters ${9} + + $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) @@ -2842,6 +2843,8 @@ private void GenerateHelperFunctions(TextWriter writer) End {{ {7} }} + Cleanup {{ {8} }} + # .ForwardHelpTargetName {1} # .ForwardHelpCategory {5} # .RemoteHelpRunspace PSSession @@ -2867,7 +2870,8 @@ private static void GenerateCommandProxy(TextWriter writer, CommandMetadata comm /* 5 */ commandMetadata.WrappedCommandType, /* 6 */ ProxyCommand.GetProcess(commandMetadata), /* 7 */ ProxyCommand.GetEnd(commandMetadata), - /* 8 */ commandMetadata.WrappedAnyCmdlet); + /* 8 */ ProxyCommand.GetCleanup(commandMetadata), + /* 9 */ commandMetadata.WrappedAnyCmdlet); } private static void GenerateCommandProxy(TextWriter writer, IEnumerable listOfCommandMetadata) diff --git a/src/System.Management.Automation/engine/CommandMetadata.cs b/src/System.Management.Automation/engine/CommandMetadata.cs index eb59f9140a29..b29f784b448d 100644 --- a/src/System.Management.Automation/engine/CommandMetadata.cs +++ b/src/System.Management.Automation/engine/CommandMetadata.cs @@ -875,8 +875,11 @@ internal string GetProxyCommand(string helpComment, bool generateDynamicParamete end {{{5}}} + +cleanup +{{{6}}} <# -{6} +{7} #> ", GetDecl(), @@ -885,6 +888,7 @@ internal string GetProxyCommand(string helpComment, bool generateDynamicParamete GetBeginBlock(), GetProcessBlock(), GetEndBlock(), + GetCleanupBlock(), CodeGeneration.EscapeBlockCommentContent(helpComment)); return result; @@ -1113,6 +1117,17 @@ internal string GetEndBlock() "; } + internal string GetCleanupBlock() + { + return @" + try { + $steppablePipeline.Dispose() + } catch { + throw + } +"; + } + #endregion #region Helper methods for restricting commands needed by implicit and interactive remoting diff --git a/src/System.Management.Automation/engine/CommandProcessorBase.cs b/src/System.Management.Automation/engine/CommandProcessorBase.cs index c6825cc69252..7d0a81d7d3e6 100644 --- a/src/System.Management.Automation/engine/CommandProcessorBase.cs +++ b/src/System.Management.Automation/engine/CommandProcessorBase.cs @@ -612,7 +612,6 @@ internal virtual void Complete() /// internal void DoComplete() { - Pipe oldErrorOutputPipe = _context.ShellFunctionErrorOutputPipe; CommandProcessorBase oldCurrentCommandProcessor = _context.CurrentCommandProcessor; try { @@ -643,32 +642,44 @@ internal void DoComplete() } finally { - OnRestorePreviousScope(); + RestorePreviousScope(); - _context.ShellFunctionErrorOutputPipe = oldErrorOutputPipe; _context.CurrentCommandProcessor = oldCurrentCommandProcessor; + } + } - // Destroy the local scope at this point if there is one... - if (_useLocalScope && CommandScope != null) - { - CommandSessionState.RemoveScope(CommandScope); - } - - // and the previous scope... - if (_previousScope != null) + /// + /// Executes the appropriate disposal methods for script commands. + /// is itself a command processor, so interpreted functions cannot be + /// reliably disposed without also disposing the entire command processor. Instead, it implements its own + /// method to perform the same function as its + /// compiled counterpart , which is a proper IDisposable implementation. + /// + internal virtual void CleanupScriptCommands() + { + try + { + if (Command is PSScriptCmdlet scriptCmdlet) { - // Restore the scope but use the same session state instance we - // got it from because the command may have changed the execution context - // session state... - CommandSessionState.CurrentScope = _previousScope; + using (commandRuntime.AllowThisCommandToWrite(true)) + using (ParameterBinderBase.bindingTracer.TraceScope("CALLING Dispose")) + { + scriptCmdlet.Dispose(); + } } - - // Restore the previous session state - if (_previousCommandSessionState != null) + else if (this is DlrScriptCommandProcessor scriptProcessor) { - Context.EngineSessionState = _previousCommandSessionState; + using (commandRuntime.AllowThisCommandToWrite(true)) + using (ParameterBinderBase.bindingTracer.TraceScope("CALLING Dispose")) + { + scriptProcessor.InvokeCleanupBlock(); + } } } + catch (Exception e) + { + throw ManageInvocationException(e); + } } /// @@ -943,21 +954,77 @@ public void Dispose() private void Dispose(bool disposing) { if (_disposed) + { return; + } - if (disposing) + try { - // 2004/03/05-JonN Look into using metadata to check - // whether IDisposable is implemented, in order to avoid - // this expensive reflection cast. - IDisposable id = Command as IDisposable; - if (id != null) + if (disposing) { - id.Dispose(); + Pipe oldErrorOutputPipe = _context.ShellFunctionErrorOutputPipe; + CommandProcessorBase oldCurrentCommandProcessor = _context.CurrentCommandProcessor; + + bool isStopping = false; + try + { + if (this.RedirectShellErrorOutputPipe || _context.ShellFunctionErrorOutputPipe != null) + { + _context.ShellFunctionErrorOutputPipe = this.commandRuntime.ErrorOutputPipe; + } + + _context.CurrentCommandProcessor = this; + isStopping = ExceptionHandlingOps.SuspendStoppingPipeline(Context); + SetCurrentScopeToExecutionScope(); + + // This is a no-op for compiled cmdlets + CleanupScriptCommands(); + } + finally + { + ExceptionHandlingOps.RestoreStoppingPipeline(Context, isStopping); + OnRestorePreviousScope(); + + _context.ShellFunctionErrorOutputPipe = oldErrorOutputPipe; + _context.CurrentCommandProcessor = oldCurrentCommandProcessor; + + // Destroy the local scope at this point if there is one... + if (_useLocalScope && CommandScope != null) + { + CommandSessionState.RemoveScope(CommandScope); + } + + // and the previous scope... + if (_previousScope != null) + { + // Restore the scope but use the same session state instance we + // got it from because the command may have changed the execution context + // session state... + CommandSessionState.CurrentScope = _previousScope; + } + + // Restore the previous session state + if (_previousCommandSessionState != null) + { + Context.EngineSessionState = _previousCommandSessionState; + } + + this.CommandRuntime.RemoveVariableListsInPipe(); + + // 2004/03/05-JonN Look into using metadata to check + // whether IDisposable is implemented, in order to avoid + // this expensive reflection cast. + if (Command is IDisposable id) + { + id.Dispose(); + } + } } } - - _disposed = true; + finally + { + _disposed = true; + } } #endregion IDispose diff --git a/src/System.Management.Automation/engine/EventManager.cs b/src/System.Management.Automation/engine/EventManager.cs index 3df9153f4e7d..94e04717b09c 100644 --- a/src/System.Management.Automation/engine/EventManager.cs +++ b/src/System.Management.Automation/engine/EventManager.cs @@ -1844,6 +1844,11 @@ public sealed class PSEngineEvent /// internal const string OnScriptBlockInvoke = "PowerShell.OnScriptBlockInvoke"; + /// + /// Called during invocation of cleanup{} for script commands. + /// + internal const string OnScriptCommandCleanup = "PowerShell.OnScriptCommandCleanup"; + /// /// Called during scriptblock invocation. /// diff --git a/src/System.Management.Automation/engine/ProxyCommand.cs b/src/System.Management.Automation/engine/ProxyCommand.cs index 5703adec664f..3a8b2faf03dc 100644 --- a/src/System.Management.Automation/engine/ProxyCommand.cs +++ b/src/System.Management.Automation/engine/ProxyCommand.cs @@ -247,6 +247,30 @@ public static string GetEnd(CommandMetadata commandMetadata) return commandMetadata.GetEndBlock(); } + /// + /// This method constructs a string representing the dispose block of the command + /// specified by . The returned string only contains the + /// script, it is not enclosed in "cleanup { }". + /// + /// + /// An instance of CommandMetadata representing a command. + /// + /// + /// A string representing the end block of the command. + /// + /// + /// If is null. + /// + public static string GetCleanup(CommandMetadata commandMetadata) + { + if (commandMetadata == null) + { + throw PSTraceSource.NewArgumentNullException(nameof(commandMetadata)); + } + + return commandMetadata.GetCleanupBlock(); + } + private static T GetProperty(PSObject obj, string property) where T : class { T result = null; diff --git a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs index 99b685e07d25..8399c211a3ca 100644 --- a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs +++ b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Management.Automation.Internal; using System.Management.Automation.Language; +using System.Management.Automation.Runspaces; using System.Reflection; using Dbg = System.Management.Automation.Diagnostics; @@ -394,46 +395,39 @@ internal override void ProcessRecord() internal override void Complete() { - try + if (_exitWasCalled) { - if (_exitWasCalled) - { - return; - } + return; + } - // process any items that may still be in the input pipeline - if (_scriptBlock.HasProcessBlock && IsPipelineInputExpected()) - { - DoProcessRecordWithInput(); - } + // process any items that may still be in the input pipeline + if (_scriptBlock.HasProcessBlock && IsPipelineInputExpected()) + { + DoProcessRecordWithInput(); + } - if (_scriptBlock.HasEndBlock) + if (_scriptBlock.HasEndBlock) + { + var endBlock = _runOptimizedCode ? _scriptBlock.EndBlock : _scriptBlock.UnoptimizedEndBlock; + if (this.CommandRuntime.InputPipe.ExternalReader == null) { - var endBlock = _runOptimizedCode ? _scriptBlock.EndBlock : _scriptBlock.UnoptimizedEndBlock; - if (this.CommandRuntime.InputPipe.ExternalReader == null) + if (IsPipelineInputExpected()) { - if (IsPipelineInputExpected()) + // read any items that may still be in the input pipe + while (Read()) { - // read any items that may still be in the input pipe - while (Read()) - { - _input.Add(Command.CurrentPipelineObject); - } + _input.Add(Command.CurrentPipelineObject); } - - // run with accumulated input - RunClause(endBlock, AutomationNull.Value, _input); - } - else - { - // run with asynchronously updated $input enumerator - RunClause(endBlock, AutomationNull.Value, this.CommandRuntime.InputPipe.ExternalReader.GetReadEnumerator()); } + + // run with accumulated input + RunClause(endBlock, AutomationNull.Value, _input); + } + else + { + // run with asynchronously updated $input enumerator + RunClause(endBlock, AutomationNull.Value, this.CommandRuntime.InputPipe.ExternalReader.GetReadEnumerator()); } - } - finally - { - ScriptBlock.LogScriptBlockEnd(_scriptBlock, Context.CurrentRunspace.InstanceId); } } @@ -633,5 +627,131 @@ protected override void OnRestorePreviousScope() CommandSessionState.CurrentScope.DottedScopes.Pop(); } } + + internal void InvokeCleanupBlock() + { + ExecutionContext currentContext = LocalPipeline.GetExecutionContextFromTLS(); + if (_scriptBlock.HasCleanupBlock && Context != currentContext) + { + // Something went wrong; Cleanup {} is being called from the wrong thread. + // Create an event to call it from the correct thread. + InvokeCleanupInOriginalContext(); + return; + } + + var oldOutputPipe = _functionContext?._outputPipe; + try + { + if (_scriptBlock.HasCleanupBlock) + { + var cleanupBlock = _runOptimizedCode + ? _scriptBlock.CleanupBlock + : _scriptBlock.UnoptimizedCleanupBlock; + + if (_functionContext != null) + { + _functionContext._outputPipe = new Pipe + { + NullPipe = true + }; + } + + // run with no pipeline input or $input enumerator + RunClause(cleanupBlock, AutomationNull.Value, AutomationNull.Value); + } + } + finally + { + if (_functionContext != null) + { + _functionContext._outputPipe = oldOutputPipe; + } + + ScriptBlock.LogScriptBlockEnd(_scriptBlock, Context.CurrentRunspace.InstanceId); + } + } + + #region Eventing for Cleanup + + private void InvokeCleanupInOriginalContext() + { + Context.Events.SubscribeEvent( + source: null, + eventName: PSEngineEvent.OnScriptCommandCleanup, + sourceIdentifier: PSEngineEvent.OnScriptCommandCleanup, + data: null, + handlerDelegate: new PSEventReceivedEventHandler(OnCleanupInvocationEventHandler), + supportEvent: true, + forwardEvent: false, + shouldQueueAndProcessInExecutionThread: true, + maxTriggerCount: 1); + + var cleanupInvocationEventArgs = new ScriptCommandCleanupInvocationEventArgs(this); + + Context.Events.GenerateEvent( + sourceIdentifier: PSEngineEvent.OnScriptCommandCleanup, + sender: null, + args: new object[] { cleanupInvocationEventArgs }, + extraData: null, + processInCurrentThread: true, + waitForCompletionInCurrentThread: true); + } + + /// + /// Handles OnCleanupInvoke event, this is called by the event manager. + /// + /// The source of the event. + /// + /// Arguments to pass to the event, which must be of the type + /// + /// + private static void OnCleanupInvocationEventHandler(object sender, PSEventArgs args) + { + var eventArgs = (object)args.SourceEventArgs as ScriptCommandCleanupInvocationEventArgs; + Diagnostics.Assert( + eventArgs?.ScriptCommandProcessor != null, + $"Event Arguments to {nameof(OnCleanupInvocationEventHandler)} should not be null"); + + bool wasAlreadyStopping = ExceptionHandlingOps.SuspendStoppingPipeline( + eventArgs.ScriptCommandProcessor.Context); + try + { + eventArgs.ScriptCommandProcessor.InvokeCleanupBlock(); + } + finally + { + ExceptionHandlingOps.RestoreStoppingPipeline( + eventArgs.ScriptCommandProcessor.Context, + wasAlreadyStopping); + } + } + + #endregion Eventing for Cleanup + } + + /// + /// Defines Event arguments passed to OnDisposeInvocationEventHandler. + /// + internal sealed class ScriptCommandCleanupInvocationEventArgs : EventArgs + { + /// + /// Initializes a new instance of the class. + /// + /// The command processor to invoke cleanup for. + /// is null. + internal ScriptCommandCleanupInvocationEventArgs(DlrScriptCommandProcessor commandProcessor) + { + if (commandProcessor == null) + { + throw new ArgumentNullException(nameof(commandProcessor)); + } + + ScriptCommandProcessor = commandProcessor; + } + + /// + /// Gets the script command processor to be disposed. + /// + internal DlrScriptCommandProcessor ScriptCommandProcessor { get; } } } diff --git a/src/System.Management.Automation/engine/debugger/Breakpoint.cs b/src/System.Management.Automation/engine/debugger/Breakpoint.cs index 5daf3bdc18b6..37b1babf4010 100644 --- a/src/System.Management.Automation/engine/debugger/Breakpoint.cs +++ b/src/System.Management.Automation/engine/debugger/Breakpoint.cs @@ -531,15 +531,16 @@ internal bool TrySetBreakpoint(string scriptFile, FunctionContext functionContex // Not found. First, we check if the line/column is before any real code. If so, we'll // move the breakpoint to the first interesting sequence point (could be a dynamicparam, - // begin, process, or end block.) + // begin, process, end, or cleanup block.) if (scriptBlock != null) { var ast = scriptBlock.Ast; var bodyAst = ((IParameterMetadataProvider)ast).Body; - if ((bodyAst.DynamicParamBlock == null || bodyAst.DynamicParamBlock.Extent.IsAfter(Line, Column)) && - (bodyAst.BeginBlock == null || bodyAst.BeginBlock.Extent.IsAfter(Line, Column)) && - (bodyAst.ProcessBlock == null || bodyAst.ProcessBlock.Extent.IsAfter(Line, Column)) && - (bodyAst.EndBlock == null || bodyAst.EndBlock.Extent.IsAfter(Line, Column))) + if ((bodyAst.DynamicParamBlock == null || bodyAst.DynamicParamBlock.Extent.IsAfter(Line, Column)) + && (bodyAst.BeginBlock == null || bodyAst.BeginBlock.Extent.IsAfter(Line, Column)) + && (bodyAst.ProcessBlock == null || bodyAst.ProcessBlock.Extent.IsAfter(Line, Column)) + && (bodyAst.EndBlock == null || bodyAst.EndBlock.Extent.IsAfter(Line, Column)) + && (bodyAst.CleanupBlock == null || bodyAst.CleanupBlock.Extent.IsAfter(Line, Column))) { SetBreakpoint(functionContext, 0); return true; diff --git a/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs b/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs index 5392c1781e19..02a49981ec2a 100644 --- a/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs +++ b/src/System.Management.Automation/engine/hostifaces/LocalPipeline.cs @@ -1268,6 +1268,10 @@ public void Dispose() /// internal class PipelineStopper { + private readonly SemaphoreSlim _criticalRegionSemaphore = new SemaphoreSlim(1, 1); + + private int _criticalRegionDepth; + /// /// Stack of current executing pipeline processor. /// @@ -1279,6 +1283,31 @@ internal class PipelineStopper private readonly object _syncRoot = new object(); private readonly LocalPipeline _localPipeline; + internal void AcquirePipelineStopPermission() + { + _criticalRegionSemaphore.Wait(); + _criticalRegionSemaphore.Release(); + } + + internal void EnterCriticalRegion() + { + Interlocked.Increment(ref _criticalRegionDepth); + _criticalRegionSemaphore.Wait(millisecondsTimeout: 0); + } + + internal void ExitCriticalRegion() + { + if (Interlocked.Decrement(ref _criticalRegionDepth) == 0) + { + _criticalRegionSemaphore.Release(); + } + } + + internal bool InCriticalRegion + { + get => Interlocked.CompareExchange(ref _criticalRegionDepth, value: 0, comparand: 0) != 0; + } + /// /// Default constructor. /// @@ -1294,15 +1323,8 @@ internal PipelineStopper(LocalPipeline localPipeline) internal bool IsStopping { - get - { - return _stopping; - } - - set - { - _stopping = value; - } + get => _stopping && !InCriticalRegion; + set => _stopping = value; } /// @@ -1318,7 +1340,7 @@ internal void Push(PipelineProcessor item) lock (_syncRoot) { - if (_stopping) + if (IsStopping) { PipelineStoppedException e = new PipelineStoppedException(); throw e; @@ -1340,7 +1362,7 @@ internal void Pop(bool fromSteppablePipeline) // If we are stopping, Stop will pop the entire stack, so // we shouldn't do any popping or some PipelineProcessor won't // be notified that it is being stopped. - if (_stopping) + if (IsStopping) { return; } @@ -1376,6 +1398,10 @@ internal void Stop() copyStack = _stack.ToArray(); } + // We don't want to stop when a pipeline is in a critical region. + // Typically it will be in the middle of command disposal. + AcquirePipelineStopPermission(); + // Propagate error from the toplevel operation through to enclosing the LocalPipeline. if (copyStack.Length > 0) { diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index bcddbee0b242..97dc15f192cc 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -2026,6 +2026,7 @@ internal void Compile(CompiledScriptBlockData scriptBlock, bool optimize) scriptBlock.BeginBlock = CompileTree(_beginBlockLambda, compileInterpretChoice); scriptBlock.ProcessBlock = CompileTree(_processBlockLambda, compileInterpretChoice); scriptBlock.EndBlock = CompileTree(_endBlockLambda, compileInterpretChoice); + scriptBlock.CleanupBlock = CompileTree(_cleanupBlockLambda, compileInterpretChoice); scriptBlock.LocalsMutableTupleType = LocalVariablesTupleType; scriptBlock.LocalsMutableTupleCreator = MutableTuple.TupleCreator(LocalVariablesTupleType); scriptBlock.NameToIndexMap = nameToIndexMap; @@ -2036,6 +2037,7 @@ internal void Compile(CompiledScriptBlockData scriptBlock, bool optimize) scriptBlock.UnoptimizedBeginBlock = CompileTree(_beginBlockLambda, compileInterpretChoice); scriptBlock.UnoptimizedProcessBlock = CompileTree(_processBlockLambda, compileInterpretChoice); scriptBlock.UnoptimizedEndBlock = CompileTree(_endBlockLambda, compileInterpretChoice); + scriptBlock.UnoptimizedCleanupBlock = CompileTree(_cleanupBlockLambda, compileInterpretChoice); scriptBlock.UnoptimizedLocalsMutableTupleType = LocalVariablesTupleType; scriptBlock.UnoptimizedLocalsMutableTupleCreator = MutableTuple.TupleCreator(LocalVariablesTupleType); } @@ -2221,6 +2223,7 @@ internal LoopGotoTargets(string label, LabelTarget breakLabel, LabelTarget conti private Expression> _beginBlockLambda; private Expression> _processBlockLambda; private Expression> _endBlockLambda; + private Expression> _cleanupBlockLambda; private readonly List _loopTargets = new List(); private bool _generatingWhileOrDoLoop; @@ -2465,6 +2468,12 @@ public object VisitScriptBlock(ScriptBlockAst scriptBlockAst) _endBlockLambda = CompileNamedBlock(scriptBlockAst.EndBlock, funcName, rootForDefiningTypesAndUsings); } + if (scriptBlockAst.CleanupBlock != null) + { + _cleanupBlockLambda = CompileNamedBlock(scriptBlockAst.CleanupBlock, funcName + "", rootForDefiningTypesAndUsings); + rootForDefiningTypesAndUsings = null; + } + return null; } diff --git a/src/System.Management.Automation/engine/parser/Parser.cs b/src/System.Management.Automation/engine/parser/Parser.cs index 3afc55e98613..8ce3f18fd7d3 100644 --- a/src/System.Management.Automation/engine/parser/Parser.cs +++ b/src/System.Management.Automation/engine/parser/Parser.cs @@ -724,12 +724,13 @@ internal static bool TryParseAsConstantHashtable(string input, out Hashtable res ParseError[] parseErrors; var ast = Parser.ParseInput(input, out throwAwayTokens, out parseErrors); - if ((ast == null) || - parseErrors.Length > 0 || - ast.BeginBlock != null || - ast.ProcessBlock != null || - ast.DynamicParamBlock != null || - ast.EndBlock.Traps != null) + if (ast == null + || parseErrors.Length > 0 + || ast.BeginBlock != null + || ast.ProcessBlock != null + || ast.CleanupBlock != null + || ast.DynamicParamBlock != null + || ast.EndBlock.Traps != null) { return false; } @@ -1713,9 +1714,9 @@ private ScriptBlockAst NamedBlockListRule(Token lCurly, List NamedBlockAst beginBlock = null; NamedBlockAst processBlock = null; NamedBlockAst endBlock = null; - IScriptExtent startExtent = lCurly != null - ? lCurly.Extent - : paramBlockAst?.Extent; + NamedBlockAst cleanupBlock = null; + + IScriptExtent startExtent = lCurly?.Extent ?? paramBlockAst?.Extent; IScriptExtent endExtent = null; IScriptExtent extent = null; IScriptExtent scriptBlockExtent = null; @@ -1757,6 +1758,7 @@ private ScriptBlockAst NamedBlockListRule(Token lCurly, List case TokenKind.Begin: case TokenKind.Process: case TokenKind.End: + case TokenKind.Cleanup: break; } @@ -1797,6 +1799,10 @@ private ScriptBlockAst NamedBlockListRule(Token lCurly, List { endBlock = new NamedBlockAst(extent, TokenKind.End, statementBlock, false); } + else if (blockNameToken.Kind == TokenKind.Cleanup && cleanupBlock == null) + { + cleanupBlock = new NamedBlockAst(extent, TokenKind.Cleanup, statementBlock, false); + } else if (blockNameToken.Kind == TokenKind.Dynamicparam && dynamicParamBlock == null) { dynamicParamBlock = new NamedBlockAst(extent, TokenKind.Dynamicparam, statementBlock, false); @@ -1818,7 +1824,14 @@ private ScriptBlockAst NamedBlockListRule(Token lCurly, List CompleteScriptBlockBody(lCurly, ref extent, out scriptBlockExtent); return_script_block_ast: - return new ScriptBlockAst(scriptBlockExtent, usingStatements, paramBlockAst, beginBlock, processBlock, endBlock, + return new ScriptBlockAst( + scriptBlockExtent, + usingStatements, + paramBlockAst, + beginBlock, + processBlock, + endBlock, + cleanupBlock, dynamicParamBlock); } diff --git a/src/System.Management.Automation/engine/parser/SemanticChecks.cs b/src/System.Management.Automation/engine/parser/SemanticChecks.cs index 927b2c33ae8b..3f0b789669a5 100644 --- a/src/System.Management.Automation/engine/parser/SemanticChecks.cs +++ b/src/System.Management.Automation/engine/parser/SemanticChecks.cs @@ -406,10 +406,11 @@ public override AstVisitAction VisitFunctionMember(FunctionMemberAst functionMem ParserStrings.ParamBlockNotAllowedInMethod); } - if (body.BeginBlock != null || - body.ProcessBlock != null || - body.DynamicParamBlock != null || - !body.EndBlock.Unnamed) + if (body.BeginBlock != null + || body.ProcessBlock != null + || body.CleanupBlock != null + || body.DynamicParamBlock != null + || !body.EndBlock.Unnamed) { _parser.ReportError(Parser.ExtentFromFirstOf(body.DynamicParamBlock, body.BeginBlock, body.ProcessBlock, body.EndBlock), nameof(ParserStrings.NamedBlockNotAllowedInMethod), diff --git a/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs b/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs index d1e3c7155d7f..dd1c71b23f6e 100644 --- a/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs +++ b/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs @@ -721,6 +721,7 @@ object ICustomAstVisitor.VisitScriptBlock(ScriptBlockAst scriptBlockAst) var beginBlock = scriptBlockAst.BeginBlock; var processBlock = scriptBlockAst.ProcessBlock; var endBlock = scriptBlockAst.EndBlock; + var cleanupBlock = scriptBlockAst.CleanupBlock; // The following is used when we don't find OutputType, which is checked elsewhere. if (beginBlock != null) @@ -738,6 +739,11 @@ object ICustomAstVisitor.VisitScriptBlock(ScriptBlockAst scriptBlockAst) res.AddRange(InferTypes(endBlock)); } + if (cleanupBlock != null) + { + res.AddRange(InferTypes(cleanupBlock)); + } + return res; } diff --git a/src/System.Management.Automation/engine/parser/VariableAnalysis.cs b/src/System.Management.Automation/engine/parser/VariableAnalysis.cs index b38954ebfc62..37f89963cac4 100644 --- a/src/System.Management.Automation/engine/parser/VariableAnalysis.cs +++ b/src/System.Management.Automation/engine/parser/VariableAnalysis.cs @@ -965,6 +965,11 @@ public object VisitScriptBlock(ScriptBlockAst scriptBlockAst) scriptBlockAst.EndBlock.Accept(this); } + if (scriptBlockAst.CleanupBlock != null) + { + scriptBlockAst.CleanupBlock.Accept(this); + } + _currentBlock.FlowsTo(_exitBlock); return null; diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index 1f03eb68df5e..c159c742aad9 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -863,6 +863,84 @@ public class ScriptBlockAst : Ast, IParameterMetadataProvider } } + /// + /// Initializes a new instance of the class. + /// This construction uses explicitly named begin/process/end/dispose blocks. + /// + /// The extent of the script block. + /// The list of using statments, may be null. + /// The set of attributes for the script block. + /// The ast for the param block, may be null. + /// The ast for the begin block, may be null. + /// The ast for the process block, may be null. + /// The ast for the end block, may be null. + /// The ast for the cleanup block, may be null. + /// The ast for the dynamicparam block, may be null. + /// + /// If is null. + /// + [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "param")] + public ScriptBlockAst( + IScriptExtent extent, + IEnumerable usingStatements, + IEnumerable attributes, + ParamBlockAst paramBlock, + NamedBlockAst beginBlock, + NamedBlockAst processBlock, + NamedBlockAst endBlock, + NamedBlockAst cleanupBlock, + NamedBlockAst dynamicParamBlock) + : base(extent) + { + SetUsingStatements(usingStatements); + + if (attributes != null) + { + this.Attributes = new ReadOnlyCollection(attributes.ToArray()); + SetParents(Attributes); + } + else + { + this.Attributes = s_emptyAttributeList; + } + + if (paramBlock != null) + { + this.ParamBlock = paramBlock; + SetParent(paramBlock); + } + + if (beginBlock != null) + { + this.BeginBlock = beginBlock; + SetParent(beginBlock); + } + + if (processBlock != null) + { + this.ProcessBlock = processBlock; + SetParent(processBlock); + } + + if (endBlock != null) + { + this.EndBlock = endBlock; + SetParent(endBlock); + } + + if (cleanupBlock != null) + { + this.CleanupBlock = cleanupBlock; + SetParent(cleanupBlock); + } + + if (dynamicParamBlock != null) + { + this.DynamicParamBlock = dynamicParamBlock; + SetParent(dynamicParamBlock); + } + } + /// /// Construct a ScriptBlockAst that uses explicitly named begin/process/end blocks. /// @@ -888,6 +966,35 @@ public class ScriptBlockAst : Ast, IParameterMetadataProvider { } + /// + /// Initializes a new instance of the class. + /// This construction uses explicitly named begin/process/end/dispose blocks. + /// + /// The extent of the script block. + /// The list of using statments, may be null. + /// The ast for the param block, may be null. + /// The ast for the begin block, may be null. + /// The ast for the process block, may be null. + /// The ast for the end block, may be null. + /// The ast for the dispose block, may be null. + /// The ast for the dynamicparam block, may be null. + /// + /// If is null. + /// + [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "param")] + public ScriptBlockAst( + IScriptExtent extent, + IEnumerable usingStatements, + ParamBlockAst paramBlock, + NamedBlockAst beginBlock, + NamedBlockAst processBlock, + NamedBlockAst endBlock, + NamedBlockAst cleanupBlock, + NamedBlockAst dynamicParamBlock) + : this(extent, usingStatements, null, paramBlock, beginBlock, processBlock, endBlock, cleanupBlock, dynamicParamBlock) + { + } + /// /// Construct a ScriptBlockAst that uses explicitly named begin/process/end blocks. /// @@ -911,6 +1018,33 @@ public class ScriptBlockAst : Ast, IParameterMetadataProvider { } + /// + /// Initializes a new instance of the class. + /// This construction uses explicitly named begin/process/end/dispose blocks. + /// + /// The extent of the script block. + /// The ast for the param block, may be null. + /// The ast for the begin block, may be null. + /// The ast for the process block, may be null. + /// The ast for the end block, may be null. + /// The ast for the dispose block, may be null. + /// The ast for the dynamicparam block, may be null. + /// + /// If is null. + /// + [SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "param")] + public ScriptBlockAst( + IScriptExtent extent, + ParamBlockAst paramBlock, + NamedBlockAst beginBlock, + NamedBlockAst processBlock, + NamedBlockAst endBlock, + NamedBlockAst cleanupBlock, + NamedBlockAst dynamicParamBlock) + : this(extent, null, paramBlock, beginBlock, processBlock, endBlock, cleanupBlock, dynamicParamBlock) + { + } + /// /// Construct a ScriptBlockAst that does not use explicitly named blocks. /// @@ -1115,6 +1249,11 @@ private void SetUsingStatements(IEnumerable usingStatements) /// public NamedBlockAst EndBlock { get; } + /// + /// Gets the ast representing the dispose block for a script block, or null if no dispose block was specified. + /// + public NamedBlockAst CleanupBlock { get; private set; } + /// /// The ast representing the dynamicparam block for a script block, or null if no dynamicparam block was specified. /// @@ -1194,17 +1333,25 @@ public override Ast Copy() var newBeginBlock = CopyElement(this.BeginBlock); var newProcessBlock = CopyElement(this.ProcessBlock); var newEndBlock = CopyElement(this.EndBlock); + var newCleanupBlock = CopyElement(this.CleanupBlock); var newDynamicParamBlock = CopyElement(this.DynamicParamBlock); var newAttributes = CopyElements(this.Attributes); var newUsingStatements = CopyElements(this.UsingStatements); - var scriptBlockAst = new ScriptBlockAst(this.Extent, newUsingStatements, newAttributes, newParamBlock, newBeginBlock, newProcessBlock, - newEndBlock, newDynamicParamBlock) + return new ScriptBlockAst( + this.Extent, + newUsingStatements, + newAttributes, + newParamBlock, + newBeginBlock, + newProcessBlock, + newEndBlock, + newCleanupBlock, + newDynamicParamBlock) { IsConfiguration = this.IsConfiguration, ScriptRequirements = this.ScriptRequirements }; - return scriptBlockAst; } internal string ToStringForSerialization() @@ -1367,19 +1514,38 @@ internal override AstVisitAction InternalVisit(AstVisitor visitor) } } - if (action == AstVisitAction.Continue && ParamBlock != null) - action = ParamBlock.InternalVisit(visitor); - if (action == AstVisitAction.Continue && DynamicParamBlock != null) - action = DynamicParamBlock.InternalVisit(visitor); - if (action == AstVisitAction.Continue && BeginBlock != null) - action = BeginBlock.InternalVisit(visitor); - if (action == AstVisitAction.Continue && ProcessBlock != null) - action = ProcessBlock.InternalVisit(visitor); - if (action == AstVisitAction.Continue && EndBlock != null) - action = EndBlock.InternalVisit(visitor); + if (!ShouldContinueAfterVisit(visitor, ParamBlock, ref action)) + { + return visitor.CheckForPostAction(this, action); + } + + if (!ShouldContinueAfterVisit(visitor, DynamicParamBlock, ref action)) + { + return visitor.CheckForPostAction(this, action); + } + + if (!ShouldContinueAfterVisit(visitor, BeginBlock, ref action)) + { + return visitor.CheckForPostAction(this, action); + } + + if (!ShouldContinueAfterVisit(visitor, ProcessBlock, ref action)) + { + return visitor.CheckForPostAction(this, action); + } + + if (!ShouldContinueAfterVisit(visitor, EndBlock, ref action)) + { + return visitor.CheckForPostAction(this, action); + } + + ShouldContinueAfterVisit(visitor, CleanupBlock, ref action); return visitor.CheckForPostAction(this, action); } + internal static bool ShouldContinueAfterVisit(AstVisitor visitor, Ast ast, ref AstVisitAction action) + => ast == null || (action = ast.InternalVisit(visitor)) == AstVisitAction.Continue; + #endregion Visitors #region IParameterMetadataProvider implementation @@ -1581,9 +1747,12 @@ bool IParameterMetadataProvider.UsesCmdletBinding() internal PipelineAst GetSimplePipeline(bool allowMultiplePipelines, out string errorId, out string errorMsg) { - if (BeginBlock != null || ProcessBlock != null || DynamicParamBlock != null) + if (BeginBlock != null + || ProcessBlock != null + || CleanupBlock != null + || DynamicParamBlock != null) { - errorId = "CanConvertOneClauseOnly"; + errorId = nameof(AutomationExceptions.CanConvertOneClauseOnly); errorMsg = AutomationExceptions.CanConvertOneClauseOnly; return null; } @@ -1749,7 +1918,7 @@ internal static bool UsesCmdletBinding(IEnumerable parameters) public class NamedBlockAst : Ast { /// - /// Construct the ast for a begin, process, end, or dynamic param block. + /// Construct the ast for a begin, process, end, cleanup, or dynamic param block. /// /// /// The extent of the block. If is false, the extent includes @@ -1761,6 +1930,7 @@ public class NamedBlockAst : Ast /// /// /// + /// /// /// /// @@ -1780,7 +1950,9 @@ public NamedBlockAst(IScriptExtent extent, TokenKind blockName, StatementBlockAs // Validate the block name. If the block is unnamed, it must be an End block (for a function) // or Process block (for a filter). if (!blockName.HasTrait(TokenFlags.ScriptBlockBlockName) - || (unnamed && (blockName == TokenKind.Begin || blockName == TokenKind.Dynamicparam))) + || (unnamed && (blockName == TokenKind.Begin + || blockName == TokenKind.Cleanup + || blockName == TokenKind.Dynamicparam))) { throw PSTraceSource.NewArgumentException(nameof(blockName)); } @@ -1838,6 +2010,7 @@ public NamedBlockAst(IScriptExtent extent, TokenKind blockName, StatementBlockAs /// /// /// + /// /// /// /// diff --git a/src/System.Management.Automation/engine/parser/token.cs b/src/System.Management.Automation/engine/parser/token.cs index 893ec9fc8e06..f1ffe8f2207d 100644 --- a/src/System.Management.Automation/engine/parser/token.cs +++ b/src/System.Management.Automation/engine/parser/token.cs @@ -587,6 +587,8 @@ public enum TokenKind /// The 'default' keyword Default = 169, + /// The 'cleanup' keyword. + Cleanup = 170, #endregion Keywords } @@ -948,6 +950,7 @@ public static class TokenTraits /* Hidden */ TokenFlags.Keyword, /* Base */ TokenFlags.Keyword, /* Default */ TokenFlags.Keyword, + /* Cleanup */ TokenFlags.Keyword | TokenFlags.ScriptBlockBlockName, #endregion Flags for keywords }; @@ -1147,6 +1150,7 @@ public static class TokenTraits /* Hidden */ "hidden", /* Base */ "base", /* Default */ "default", + /* Cleanup */ "cleanup", #endregion Text for keywords }; @@ -1154,10 +1158,12 @@ public static class TokenTraits #if DEBUG static TokenTraits() { - Diagnostics.Assert(s_staticTokenFlags.Length == ((int)TokenKind.Default + 1), - "Table size out of sync with enum - _staticTokenFlags"); - Diagnostics.Assert(s_tokenText.Length == ((int)TokenKind.Default + 1), - "Table size out of sync with enum - _tokenText"); + Diagnostics.Assert( + s_staticTokenFlags.Length == ((int)TokenKind.Cleanup + 1), + "Table size out of sync with enum - _staticTokenFlags"); + Diagnostics.Assert( + s_tokenText.Length == ((int)TokenKind.Cleanup + 1), + "Table size out of sync with enum - _tokenText"); // Some random assertions to make sure the enum and the traits are in sync Diagnostics.Assert(GetTraits(TokenKind.Begin) == (TokenFlags.Keyword | TokenFlags.ScriptBlockBlockName), "Table out of sync with enum - flags Begin"); diff --git a/src/System.Management.Automation/engine/parser/tokenizer.cs b/src/System.Management.Automation/engine/parser/tokenizer.cs index 161e53cef0c7..fa0b1966008d 100644 --- a/src/System.Management.Automation/engine/parser/tokenizer.cs +++ b/src/System.Management.Automation/engine/parser/tokenizer.cs @@ -635,23 +635,23 @@ internal class Tokenizer /*A*/ "configuration", "public", "private", "static", /*A*/ /*B*/ "interface", "enum", "namespace", "module", /*B*/ /*C*/ "type", "assembly", "command", "hidden", /*C*/ - /*D*/ "base", "default", /*D*/ + /*D*/ "base", "default", "cleanup", /*D*/ }; private static readonly TokenKind[] s_keywordTokenKind = new TokenKind[] { - /*1*/ TokenKind.ElseIf, TokenKind.If, TokenKind.Else, TokenKind.Switch, /*1*/ - /*2*/ TokenKind.Foreach, TokenKind.From, TokenKind.In, TokenKind.For, /*2*/ - /*3*/ TokenKind.While, TokenKind.Until, TokenKind.Do, TokenKind.Try, /*3*/ - /*4*/ TokenKind.Catch, TokenKind.Finally, TokenKind.Trap, TokenKind.Data, /*4*/ - /*5*/ TokenKind.Return, TokenKind.Continue, TokenKind.Break, TokenKind.Exit, /*5*/ - /*6*/ TokenKind.Throw, TokenKind.Begin, TokenKind.Process, TokenKind.End, /*6*/ - /*7*/ TokenKind.Dynamicparam, TokenKind.Function, TokenKind.Filter, TokenKind.Param, /*7*/ - /*8*/ TokenKind.Class, TokenKind.Define, TokenKind.Var, TokenKind.Using, /*8*/ - /*9*/ TokenKind.Workflow, TokenKind.Parallel, TokenKind.Sequence, TokenKind.InlineScript, /*9*/ - /*A*/ TokenKind.Configuration, TokenKind.Public, TokenKind.Private, TokenKind.Static, /*A*/ - /*B*/ TokenKind.Interface, TokenKind.Enum, TokenKind.Namespace, TokenKind.Module, /*B*/ - /*C*/ TokenKind.Type, TokenKind.Assembly, TokenKind.Command, TokenKind.Hidden, /*C*/ - /*D*/ TokenKind.Base, TokenKind.Default, /*D*/ + /*1*/ TokenKind.ElseIf, TokenKind.If, TokenKind.Else, TokenKind.Switch, /*1*/ + /*2*/ TokenKind.Foreach, TokenKind.From, TokenKind.In, TokenKind.For, /*2*/ + /*3*/ TokenKind.While, TokenKind.Until, TokenKind.Do, TokenKind.Try, /*3*/ + /*4*/ TokenKind.Catch, TokenKind.Finally, TokenKind.Trap, TokenKind.Data, /*4*/ + /*5*/ TokenKind.Return, TokenKind.Continue, TokenKind.Break, TokenKind.Exit, /*5*/ + /*6*/ TokenKind.Throw, TokenKind.Begin, TokenKind.Process, TokenKind.End, /*6*/ + /*7*/ TokenKind.Dynamicparam, TokenKind.Function, TokenKind.Filter, TokenKind.Param, /*7*/ + /*8*/ TokenKind.Class, TokenKind.Define, TokenKind.Var, TokenKind.Using, /*8*/ + /*9*/ TokenKind.Workflow, TokenKind.Parallel, TokenKind.Sequence, TokenKind.InlineScript, /*9*/ + /*A*/ TokenKind.Configuration, TokenKind.Public, TokenKind.Private, TokenKind.Static, /*A*/ + /*B*/ TokenKind.Interface, TokenKind.Enum, TokenKind.Namespace, TokenKind.Module, /*B*/ + /*C*/ TokenKind.Type, TokenKind.Assembly, TokenKind.Command, TokenKind.Hidden, /*C*/ + /*D*/ TokenKind.Base, TokenKind.Default, TokenKind.Cleanup, /*D*/ }; internal static readonly string[] _operatorText = new string[] { diff --git a/src/System.Management.Automation/engine/pipeline.cs b/src/System.Management.Automation/engine/pipeline.cs index 78fb19dbc653..4ed5118d2fbb 100644 --- a/src/System.Management.Automation/engine/pipeline.cs +++ b/src/System.Management.Automation/engine/pipeline.cs @@ -618,6 +618,10 @@ private void DoCompleteCore(CommandProcessorBase commandRequestingUpstreamComman commandRequestingUpstreamCommandsToStop = stopUpstreamCommandsException.RequestingCommandProcessor; } } + finally + { + commandProcessor.Dispose(); + } EtwActivity.SetActivityId(commandProcessor.PipelineActivityId); @@ -1321,12 +1325,16 @@ private void DisposeCommands() // exemption. catch (Exception e) // Catch-all OK, 3rd party callout. { + if (_firstTerminatingError != null) + { + _firstTerminatingError.Throw(); + } + InvocationInfo myInvocation = null; if (commandProcessor.Command != null) myInvocation = commandProcessor.Command.MyInvocation; - ProviderInvocationException pie = - e as ProviderInvocationException; + ProviderInvocationException pie = e as ProviderInvocationException; if (pie != null) { e = new CmdletProviderInvocationException( diff --git a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs index 2e6db041126a..aa4cce37662e 100644 --- a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs +++ b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs @@ -12,11 +12,9 @@ using System.Management.Automation.Runspaces; using System.Management.Automation.Tracing; using System.Reflection; -using System.Runtime.CompilerServices; using System.Runtime.Serialization; using System.Security.Cryptography.X509Certificates; using System.Text; -using System.Threading.Tasks; #if LEGACYTELEMETRY using Microsoft.PowerShell.Telemetry.Internal; #endif @@ -35,6 +33,7 @@ internal enum ScriptBlockClauseToInvoke Begin, Process, End, + Cleanup, ProcessBlockOnly, } @@ -247,6 +246,7 @@ bool IsScriptBlockInFactASafeHashtable() if (scriptBlockAst.BeginBlock != null || scriptBlockAst.ProcessBlock != null + || scriptBlockAst.CleanupBlock != null || scriptBlockAst.ParamBlock != null || scriptBlockAst.DynamicParamBlock != null || scriptBlockAst.ScriptRequirements != null @@ -331,6 +331,8 @@ private IParameterMetadataProvider DelayParseScriptText() internal Action EndBlock { get; set; } internal Action UnoptimizedEndBlock { get; set; } + internal Action CleanupBlock { get; set; } + internal Action UnoptimizedCleanupBlock { get; set; } internal IScriptExtent[] SequencePoints { get; set; } @@ -753,7 +755,7 @@ private PipelineAst GetSimplePipeline(Func errorHandler) { errorHandler ??= (static _ => null); - if (HasBeginBlock || HasProcessBlock) + if (HasBeginBlock || HasProcessBlock || HasCleanupBlock) { return errorHandler(AutomationExceptions.CanConvertOneClauseOnly); } @@ -891,7 +893,10 @@ internal bool SkipLogging Parser parser = new Parser(); var ast = AstInternal; - if (HasBeginBlock || HasProcessBlock || ast.Body.ParamBlock != null) + if (HasBeginBlock + || HasProcessBlock + || HasCleanupBlock + || ast.Body.ParamBlock != null) { Ast errorAst = ast.Body.BeginBlock ?? (Ast)ast.Body.ProcessBlock ?? ast.Body.ParamBlock; parser.ReportError( @@ -976,7 +981,8 @@ internal bool SkipLogging { if ((clauseToInvoke == ScriptBlockClauseToInvoke.Begin && !HasBeginBlock) || (clauseToInvoke == ScriptBlockClauseToInvoke.Process && !HasProcessBlock) - || (clauseToInvoke == ScriptBlockClauseToInvoke.End && !HasEndBlock)) + || (clauseToInvoke == ScriptBlockClauseToInvoke.End && !HasEndBlock) + || (clauseToInvoke == ScriptBlockClauseToInvoke.Cleanup && !HasCleanupBlock)) { return; } @@ -1362,7 +1368,7 @@ internal static void SetAutomaticVariable(AutomaticVariable variable, object val private Action GetCodeToInvoke(ref bool optimized, ScriptBlockClauseToInvoke clauseToInvoke) { if (clauseToInvoke == ScriptBlockClauseToInvoke.ProcessBlockOnly - && (HasBeginBlock || (HasEndBlock && HasProcessBlock))) + && (HasBeginBlock || HasCleanupBlock || (HasEndBlock && HasProcessBlock))) { throw PSTraceSource.NewInvalidOperationException(AutomationExceptions.ScriptBlockInvokeOnOneClauseOnly); } @@ -1379,6 +1385,8 @@ private Action GetCodeToInvoke(ref bool optimized, ScriptBlockC return _scriptBlockData.ProcessBlock; case ScriptBlockClauseToInvoke.End: return _scriptBlockData.EndBlock; + case ScriptBlockClauseToInvoke.Cleanup: + return _scriptBlockData.CleanupBlock; default: return HasProcessBlock ? _scriptBlockData.ProcessBlock : _scriptBlockData.EndBlock; } @@ -1392,6 +1400,8 @@ private Action GetCodeToInvoke(ref bool optimized, ScriptBlockC return _scriptBlockData.UnoptimizedProcessBlock; case ScriptBlockClauseToInvoke.End: return _scriptBlockData.UnoptimizedEndBlock; + case ScriptBlockClauseToInvoke.Cleanup: + return _scriptBlockData.UnoptimizedCleanupBlock; default: return HasProcessBlock ? _scriptBlockData.UnoptimizedProcessBlock : _scriptBlockData.UnoptimizedEndBlock; } @@ -2147,11 +2157,17 @@ internal static void LogScriptBlockEnd(ScriptBlock scriptBlock, Guid runspaceId) internal Action UnoptimizedEndBlock { get => _scriptBlockData.UnoptimizedEndBlock; } + internal Action CleanupBlock { get => _scriptBlockData.CleanupBlock; } + + internal Action UnoptimizedCleanupBlock { get => _scriptBlockData.UnoptimizedCleanupBlock; } + internal bool HasBeginBlock { get => AstInternal.Body.BeginBlock != null; } internal bool HasProcessBlock { get => AstInternal.Body.ProcessBlock != null; } internal bool HasEndBlock { get => AstInternal.Body.EndBlock != null; } + + internal bool HasCleanupBlock { get => AstInternal.Body.CleanupBlock != null; } } [Serializable] @@ -2514,18 +2530,134 @@ public void Dispose() return; } - this.DisposingEvent.SafeInvoke(this, EventArgs.Empty); - commandRuntime = null; - currentObjectInPipeline = null; - _input.Clear(); - // _scriptBlock = null; - // _localsTuple = null; - // _functionContext = null; + ExecutionContext currentContext = LocalPipeline.GetExecutionContextFromTLS(); + if (Context != currentContext && _scriptBlock.HasCleanupBlock) + { + // Something went wrong; Dispose{} is being called from the wrong thread. + // Create an event to call it from the correct thread. + InvokeCleanupInOriginalContext(); + return; + } + + var oldOutputPipe = _functionContext?._outputPipe; + + try + { + if (_scriptBlock.HasCleanupBlock) + { + var disposeBlock = _runOptimized ? _scriptBlock.CleanupBlock : _scriptBlock.UnoptimizedCleanupBlock; + if (_functionContext != null) + { + _functionContext._outputPipe = new Pipe + { + NullPipe = true + }; + } + + RunClause( + disposeBlock, + AutomationNull.Value, + AutomationNull.Value); + } + } + finally + { + if (_functionContext != null) + { + _functionContext._outputPipe = oldOutputPipe; + } + + this.DisposingEvent.SafeInvoke(this, EventArgs.Empty); - base.InternalDispose(true); - _disposed = true; + commandRuntime = null; + currentObjectInPipeline = null; + _input.Clear(); + + base.InternalDispose(true); + _disposed = true; + } } #endregion IDispose + + #region Eventing for Cleanup + + private void InvokeCleanupInOriginalContext() + { + Context.Events.SubscribeEvent( + source: null, + eventName: PSEngineEvent.OnScriptCommandCleanup, + sourceIdentifier: PSEngineEvent.OnScriptCommandCleanup, + data: null, + handlerDelegate: new PSEventReceivedEventHandler(OnCleanupInvocationEventHandler), + supportEvent: true, + forwardEvent: false, + shouldQueueAndProcessInExecutionThread: true, + maxTriggerCount: 1); + + var cleanupInvocationEventArgs = new CleanupInvocationEventArgs(this); + + Context.Events.GenerateEvent( + sourceIdentifier: PSEngineEvent.OnScriptCommandCleanup, + sender: null, + args: new object[] { cleanupInvocationEventArgs }, + extraData: null, + processInCurrentThread: true, + waitForCompletionInCurrentThread: true); + } + + /// + /// Handles OnDisposeInvoke event, this is called by the event manager. + /// + /// The source of the event. + /// + /// Arguments to be passed to the event, which must be of type + /// + /// + private static void OnCleanupInvocationEventHandler(object sender, PSEventArgs args) + { + var eventArgs = (object)args.SourceEventArgs as CleanupInvocationEventArgs; + Diagnostics.Assert( + eventArgs?.ScriptCmdlet != null, + $"Event Arguments to {nameof(OnCleanupInvocationEventHandler)} should not be null"); + + bool wasAlreadyStopping = ExceptionHandlingOps.SuspendStoppingPipeline(eventArgs.ScriptCmdlet.Context); + try + { + eventArgs.ScriptCmdlet.Dispose(); + } + finally + { + ExceptionHandlingOps.RestoreStoppingPipeline(eventArgs.ScriptCmdlet.Context, wasAlreadyStopping); + } + } + + #endregion Eventing for Cleanup + } + + /// + /// Defines Event arguments passed to OnDisposeInvocationEventHandler. + /// + internal sealed class CleanupInvocationEventArgs : EventArgs + { + /// + /// Initializes a new instance of the class. + /// + /// The command processor to dispose. + /// is null. + internal CleanupInvocationEventArgs(PSScriptCmdlet cmdlet) + { + if (cmdlet == null) + { + throw new ArgumentNullException(nameof(cmdlet)); + } + + ScriptCmdlet = cmdlet; + } + + /// + /// Gets the script cmdlet to be disposed. + /// + internal PSScriptCmdlet ScriptCmdlet { get; } } } diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index de9ef697fc61..9e9d075f30d0 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -1578,11 +1578,13 @@ private static int FindMatchingHandlerByType(Type exceptionType, Type[] types) internal static bool SuspendStoppingPipeline(ExecutionContext context) { - LocalPipeline lpl = (LocalPipeline)context.CurrentRunspace.GetCurrentlyRunningPipeline(); - if (lpl != null) + var pipeline = (LocalPipeline)context.CurrentRunspace.GetCurrentlyRunningPipeline(); + if (pipeline != null) { - bool oldIsStopping = lpl.Stopper.IsStopping; - lpl.Stopper.IsStopping = false; + bool oldIsStopping = pipeline.Stopper.IsStopping; + pipeline.Stopper.IsStopping = false; + pipeline.Stopper.EnterCriticalRegion(); + return oldIsStopping; } @@ -1591,10 +1593,11 @@ internal static bool SuspendStoppingPipeline(ExecutionContext context) internal static void RestoreStoppingPipeline(ExecutionContext context, bool oldIsStopping) { - LocalPipeline lpl = (LocalPipeline)context.CurrentRunspace.GetCurrentlyRunningPipeline(); - if (lpl != null) + var pipeline = (LocalPipeline)context.CurrentRunspace.GetCurrentlyRunningPipeline(); + if (pipeline != null) { - lpl.Stopper.IsStopping = oldIsStopping; + pipeline.Stopper.IsStopping = oldIsStopping; + pipeline.Stopper.ExitCriticalRegion(); } } diff --git a/src/System.Management.Automation/resources/ParserStrings.resx b/src/System.Management.Automation/resources/ParserStrings.resx index d21719157ac9..bacfb336120a 100644 --- a/src/System.Management.Automation/resources/ParserStrings.resx +++ b/src/System.Management.Automation/resources/ParserStrings.resx @@ -497,7 +497,7 @@ The correct form is: foreach ($a in $b) {...} Script command clause '{0}' has already been defined. - unexpected token '{0}', expected 'begin', 'process', 'end', or 'dynamicparam'. + unexpected token '{0}', expected 'begin', 'process', 'end', 'cleanup', or 'dynamicparam'. Missing closing '}' in statement block or type definition. diff --git a/test/powershell/Language/Parser/Parser.Tests.ps1 b/test/powershell/Language/Parser/Parser.Tests.ps1 index 0f8a6c74726a..9afdce4ab7d4 100644 --- a/test/powershell/Language/Parser/Parser.Tests.ps1 +++ b/test/powershell/Language/Parser/Parser.Tests.ps1 @@ -67,6 +67,8 @@ Describe "ParserTests (admin\monad\tests\monad\src\engine\core\ParserTests.cs)" } end {} + + cleanup {} } '@ $functionDefinition>$functionDefinitionFile diff --git a/test/powershell/Language/Parser/Parsing.Tests.ps1 b/test/powershell/Language/Parser/Parsing.Tests.ps1 index c764830954b9..d6643fbf4a1f 100644 --- a/test/powershell/Language/Parser/Parsing.Tests.ps1 +++ b/test/powershell/Language/Parser/Parsing.Tests.ps1 @@ -148,17 +148,21 @@ Describe 'named blocks parsing' -Tags "CI" { ShouldBeParseError 'begin' MissingNamedStatementBlock 5 ShouldBeParseError 'process' MissingNamedStatementBlock 7 ShouldBeParseError 'end' MissingNamedStatementBlock 3 + ShouldBeParseError 'cleanup' MissingNamedStatementBlock 7 ShouldBeParseError 'dynamicparam' MissingNamedStatementBlock 12 ShouldBeParseError 'begin process {}' MissingNamedStatementBlock 6 -CheckColumnNumber ShouldBeParseError 'end process {}' MissingNamedStatementBlock 4 -CheckColumnNumber + ShouldBeParseError 'cleanup process {}' MissingNamedStatementBlock 8 -CheckColumnNumber ShouldBeParseError 'dynamicparam process {}' MissingNamedStatementBlock 13 -CheckColumnNumber ShouldBeParseError 'process begin {}' MissingNamedStatementBlock 8 -CheckColumnNumber - ShouldBeParseError 'begin process end' MissingNamedStatementBlock,MissingNamedStatementBlock,MissingNamedStatementBlock 6,14,18 -CheckColumnNumber + ShouldBeParseError 'begin process end cleanup' MissingNamedStatementBlock, MissingNamedStatementBlock, MissingNamedStatementBlock, MissingNamedStatementBlock 6, 14, 18, 26 -CheckColumnNumber Test-Ast 'begin' 'begin' 'begin' Test-Ast 'begin end' 'begin end' 'begin' 'end' Test-Ast 'begin end process' 'begin end process' 'begin' 'end' 'process' Test-Ast 'begin {} end' 'begin {} end' 'begin {}' 'end' + Test-Ast 'begin process end cleanup' 'begin process end cleanup' 'begin' 'cleanup' 'end' 'process' + Test-Ast 'begin {} process end cleanup {}' 'begin {} process end cleanup {}' 'begin {}' 'cleanup {}' 'end' 'process' } # diff --git a/test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1 b/test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1 new file mode 100644 index 000000000000..5616ea0cd79b --- /dev/null +++ b/test/powershell/Language/Scripting/PipelineBehaviour.Tests.ps1 @@ -0,0 +1,122 @@ +Describe 'Function Pipeline Behaviour' -Tag 'CI' { + + Context 'Output from Named Blocks' { + + $Cases = @( + @{ Script = { begin { 10 } }; ExpectedResult = 10 } + @{ Script = { process { 15 } }; ExpectedResult = 15 } + @{ Script = { end { 22 } }; ExpectedResult = 22 } + ) + It 'permits output from named block: