Skip to content

A job that is stopped while executing clean{} does not complete executing clean{} #24679

Open

Description

@alx9r

A job that is stopped while it is executing clean{} interrupts execution of that clean{} block. This appears to contradict the documentation

The clean block is a convenient way for users to clean up resources... Resource cleanup is enforced for the following scenarios:

...
4. when the pipeline is being stopped by StopProcessing()

It seems that when StopProcessing() occurs while clean{} is executing, resource cleanup is interrupted rather than enforced.

Steps to reproduce

$log = [System.Collections.Concurrent.ConcurrentQueue[string]]::new()

$job =
    Start-ThreadJob  `
        -ArgumentList $log `
        -ScriptBlock {
            param($log)
            begin {}
            clean {
                $log.Enqueue('clean{} start')
                Start-Sleep 1
                $log.Enqueue('clean{} end')
            }
        }

Start-Sleep -Milliseconds 100
$job.StopJobAsync()
Start-Sleep -Milliseconds 100
$job | Receive-Job -Wait
$log

Expected output

clean{} start
clean{} end

perhaps with

OperationStopped: The pipeline has been stopped.

mentioned.

Actual output

OperationStopped: The pipeline has been stopped.
clean{} start

Related Issues

This issue arose from Provide a way to assign a returned object to a variable that is explicitly atomic when Stopping PowerShell #24658.

There are a number of other related issues as summarized in the table below. Note that none of those issues are the same as this because of the following distinctions:

  • The issues in the table are only reproducible with Ctrl+C, and not by stopping a job. This issue is reproducible by stopping a job.
  • None of the issues below contemplate the scenario where execution has already entered clean{} when execution is interrupted. This issue is about interrupting execution in clean{}.
Issue Shortened Description Ctrl+C
repro 1.
Job.Stopping
repro
#23786 Finally block isn't executed upon Ctrl+C yes no
#19988 Attempts to write to success in finally causes termination yes no
#19987 statements in finally fail with Ctrl+C and Read-Host OP N/A 2.
#24241 "Clean" block does not run on Ctrl+C yes no

Notes:

  1. "yes" and "no" in the repro columns are indicated only if I was able to successfully repeat a check for the behavior reported in the issue.
  2. Write-Host statements, file redirections in a finally block intermittently fail quietly on Windows when Ctrl+C is used to terminate a Read-Host call #19987 seems to hinge on Read-Host in a way that doesn't have an obvious analog in a job where there is no console.

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.6
PSEdition                      Core
GitCommitId                    7.4.6
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Activity

  1. added
    Needs-TriageThe issue is new and needs to be triaged by a work group.
    on Dec 16, 2024
  2. alx9r commented on Dec 16, 2024

    @alx9r
    Author

    Observation of StopProcessing() and Stopping

    Invocation of StopProcessing() and setting of Stopping can be observed occurring in clean{} per this comment:

    Consider the following setup using that tracing Cmdlet from this comment:

    Add-Type -Path .\traceCmdletInternalsCommand.cs
    
    $job =
       Start-ThreadJob                                `
           -init   { [TraceCmdletInternalsCommand] |
                     % Assembly                    |
                     Import-Module                  } `
           -script { begin {}
                     clean { . { 1; Start-Sleep 1 } |
                             Trace-CmdletInternals    } }
    
    Start-Sleep -Milliseconds 100
    $job | Stop-Job
    $job | Wait-Job | Receive-Job
    
    [TraceCmdletInternalsCommand]::Log

    That outputs

    OperationStopped: The pipeline has been stopped.
    TraceCmdletInternalsCommand() Stopping: False
    BeginProcessing() Stopping: False
    set InputObject '1'
    ProcessRecord() Stopping: False
    StopProcessing() Stopping: True
    Dispose() Stopping: True
    

    which shows that StopProcessing() is called and Stopping is set even though execution is in the clean{} block.

  3. alx9r commented on Dec 16, 2024

    @alx9r
    Author

    Workaround

    Consider the following test which records the flow control of a script block whose execution is stopped at different stop: points:

    test code
    # define a type with the following features:
    #    - can wait for a signal to complete contruction
    #    - records constructed and disposed flags such that it is beyond object lifetime
    Add-Type @'
    public class WaitingDisposableMock : System.IDisposable {
        // global statics for tracking object lifetime
        public static int  Disposed    = 0;
        public static int  CleanedUp   = 0;
        public static bool Constructed = false;
        public WaitingDisposableMock(System.Threading.ManualResetEventSlim waitSignal) {
            Disposed    += 1;
            Constructed =  true;
            // Wait to complete construction if the signal exists.
            if (null != waitSignal) {
                waitSignal.Wait();
            }
        }
        public void Cleanup() {
            CleanedUp++;
        }
        public void Dispose() {
            Disposed++;
        }
    }
    public static class WaitSignal {
        public static void Await(System.Threading.ManualResetEventSlim waitSignal) {
            if (null != waitSignal) {
                waitSignal.Wait();
            }
        }
    }
    '@
    
    function Invoke-TestRun {
        <#
            .SYNOPSIS
                Stop a partly-executed scriptblock and output information about progress, object lifetime, and cleanup.
        #>
        param(
            [Parameter(Mandatory,ValueFromPipeline)]
            [ValidateSet('begin{} before ctor','begin{} inside ctor','begin{} after ctor','clean{} before try{}','clean{try{}} before =','clean{try{}} after =','clean{finally{}}')]
            [string]
            # The point in the scriptblock at which to way for .StopProcessing().
            $StopPoint
        )
        process {
            # initialize the mock's globals
            [WaitingDisposableMock]::Disposed    = 0
            [WaitingDisposableMock]::CleanedUp   = 0
            [WaitingDisposableMock]::Constructed = $false
    
            # set up the signals to wait for stop
            $stopPoints = [System.Collections.Concurrent.ConcurrentDictionary[string,System.Threading.ManualResetEventSlim]]::new()
            $stopPoints[$StopPoint] = [System.Threading.ManualResetEventSlim]::new()
    
            # create the log
            $log = [System.Collections.Concurrent.ConcurrentQueue[string]]::new()
    
            $job =
                Start-ThreadJob                 `
                    -argu     $stopPoints,$log `
                    -script {
                        param($stopPoints,$log)
                        begin {
                            # Write progress to the log.
                            $log.Enqueue( 'begin{} before ctor')
                            # If the signal exists, wait for it.  . {} ensures execution is interrupted here on .StopProcessing()
                            [WaitSignal]::Await($stopPoints['begin{} before ctor'])                            ; . {}
    
                            # Most of the remainder of this scriptblock are pairs of log/wait line like above.
    
                            $log.Enqueue(                                    'begin{} inside ctor')
                            # Construct the mock object.  Waiting occurs inside the constructor when that signal exists.
                            $obj = [WaitingDisposableMock]::new($stopPoints['begin{} inside ctor']) ; . {}
    
                            $log.Enqueue( 'begin{} after ctor')
                            [WaitSignal]::Await($stopPoints['begin{} after ctor'])                  ; . {}
    
                            $log.Enqueue( 'begin{} complete')
                        }
                        clean {
                            $log.Enqueue( 'clean{} before try{}')
                            [WaitSignal]::Await($stopPoints['clean{} before try{}'])
                            <#
                            Workaround clean{} block stopping if the runspace is stopped in the clean block.
                            Note the following:
                             - Interruption can only begin at one, specific point.
                             - The pipeline that is running when stop is invoked is stopped even if it is clean{} or finally{}.
                             - When that happens in clean{try{}}, it's a problem because cleanup is interrupted. So it is also run in clean{try{}}.
                             - If $clean is interrupted during try{}, then it will run again in finally{}.
                             - If $clean is interrupted during finally{}, then it has already run without interruption in try{}.
                            #>
                            try {
                                $log.Enqueue( 'clean{try{}} before =')
                                [WaitSignal]::Await($stopPoints['clean{try{}} before =']) # omit . {} here to make this and the next line atomic
    
                                # define cleanup
                                $clean = { $obj | ? {$_} | % Cleanup }
    
                                $log.Enqueue( 'clean{try{}} after =')
                                [WaitSignal]::Await($stopPoints['clean{try{}} after ='])            ; . {}
    
                                # invoke cleanup, this might be interrupted
                                . $clean
                                $log.Enqueue('clean{try{}} complete')
                            }
                            finally {
                                $log.Enqueue( 'clean{finally{}}')
                                [WaitSignal]::Await($stopPoints['clean{finally{}}'])                ; . {}
    
                                # invoke cleanup, this might be interrupted
                                . $clean
                                $log.Enqueue('clean{finally{}} complete')
                            }
                            $log.Enqueue('clean{} complete')
                        }
                    }
            # Wait for the job to start.
            Start-Sleep -Milliseconds 100
            # By this point, the job should have executed to the StopPoint.
            # Signal the job to stop.
            $job.StopJobAsync()
            # Wait for Stopping to begin.
            Start-Sleep -Milliseconds 100
            # By this point, the Stopping should have begun.
            # Signal execution to continue from the StopPoint.
            $stopPoints[$StopPoint].Set()
            # Wait for the job to complete.
            $job | Wait-Job | Out-Null
            # Clean up.
            $job | Remove-Job
    
            # Output the results of the test run.
    
            $cleanTryProgress     = $log | ? {$_ -like 'clean{try{}}*'}     | Select-Object -Last 1
            $cleanFinallyProgress = $log | ? {$_ -like 'clean{finally{}}*'} | Select-Object -Last 1
            [pscustomobject]@{
                CleanupOK                   = if ([WaitingDisposableMock]::CleanedUp -xor [WaitingDisposableMock]::Constructed) { '❌' } else { '✅' }
                'stop:'                     = $StopPoint
                Constructed                 = [WaitingDisposableMock]::Constructed
                Disposed                    = [WaitingDisposableMock]::Disposed
                CleanedUp                   = [WaitingDisposableMock]::CleanedUp
                                              # Compute the latest point reach in each of the begin{} and clean{} blocks.
                'begin{} progress'          = $log | ? {$_ -like 'begin{}*'}          | Select-Object -Last 1
                'clean{try{}} progress'     = $cleanTryProgress
                'clean{finally{}} progress' = $cleanFinallyProgress
                'clean{try{}} complete'     = if ($cleanTryProgress -eq 'clean{try{}} complete') { '✅' } else { '❌' }
                'clean{finally{}} complete' = if ($cleanFinallyProgress -eq 'clean{finally{}} complete') { '✅' } else { '❌' }
                                              # The results are OK if neither or both of Constructed and Disposed completed.
            }
        }
    }
    
    # Invoke and format the test output for each of the StopPoints.
    'begin{} before ctor'  ,
    'begin{} inside ctor'  ,
    'begin{} after ctor'   ,
    'clean{} before try{}' ,
    'clean{try{}} before =',
    'clean{try{}} after =' ,
    'clean{finally{}}'     |
        Invoke-TestRun     |
        Format-Table -AutoSize

    An analogous script block with points annotated with stop: where the stop signal arrives follows:

    begin {
        # stop: begin{} before ctor
        $obj = [SomeObject]::new() # stop: begin{} inside ctor
        # stop: begin{} after ctor
    }
    clean {
        # stop: 'clean{} before try{}'
        try {
            # stop: clean{try{}} before =
            $clean = { $obj | ? {$_} | % CleanUp }
            # stop: clean{try{}} after =
            # invoke cleanup, this might be interrupted
            . $clean
        }
        finally {
            # stop: clean{finally{}}
            # invoke cleanup, this might be interrupted
            . $clean
        }
    }

    The following table is output by that test:

    CleanupOK stop:                 Constructed Disposed CleanedUp begin{} progress    clean{try{}} progress clean{finally{}} progress clean{try{}} complete clean{finally{}} complete
    --------- -----                 ----------- -------- --------- ----------------    --------------------- ------------------------- --------------------- -------------------------
    ✅         begin{} before ctor         False        0         0 begin{} before ctor clean{try{}} complete clean{finally{}} complete ✅                     ✅
    ✅         begin{} inside ctor          True        1         2 begin{} inside ctor clean{try{}} complete clean{finally{}} complete ✅                     ✅
    ✅         begin{} after ctor           True        1         2 begin{} after ctor  clean{try{}} complete clean{finally{}} complete ✅                     ✅
    ✅         clean{} before try{}         True        1         1 begin{} complete    clean{try{}} after =  clean{finally{}} complete ❌                     ✅
    ✅         clean{try{}} before =        True        1         1 begin{} complete    clean{try{}} after =  clean{finally{}} complete ❌                     ✅
    ✅         clean{try{}} after =         True        1         1 begin{} complete    clean{try{}} after =  clean{finally{}} complete ❌                     ✅
    ✅         clean{finally{}}             True        1         1 begin{} complete    clean{try{}} complete clean{finally{}}          ✅                     ❌

    Analysis

    The test demonstrates the following key observations:

    • Either clean{try{}} or clean{finally{}} or both executes to completion regardless of the stop: point.
    • $clean is executed either once or twice regardless of the stop: point.

    This implies that the interruption of clean{} can be recovered from using the above clean{try{}finally{}} pattern for any cleanup that can safely be invoked multiple times.

    Note the following nuances about this workaround:

    • The entry into clean{} and assignment of $clean must contain only statements that are atomic 1.. Which statements are atomic seems to be incidental. That is, atomicity is not by design. This means whether a statement is (or a series of statements are) atomic is an empirical question. A statement's atomicity can be tested using one of the techniques mentioned in this answer.
    • The atomicity requirement precludes simplifying the call site of the above clean{try{}finally{}} into a PowerShell command that accepts arguments since parameter binding seems to be interrupted by Stopping.
    • The obvious probable solutions for simplifying the call site given the so-far characterized and documented behavior of this part of PowerShell are as follows (Edit: These have been ruled out as demonstrated by the failure to cleanup here):
      • A parameterless command that does not check Stopping and thus reliably executes when it is the first command in clean{}. Out-Null seems to be an example of such a command that reliably executes in that position. Such a call site might look like clean {$cleanup = {...}; CleanUp;} where CleanUp invokes $cleanup by way of Cmdlet.SessionState.PSVariable.
      • A .Net method that accepts $cleanup scriptblock and, possibly, $PSCmdlet. Such a call site might look like [workaround]::clean($PSCmdlet,{...}) where clean() injects the requisite try{}finally{} to ensure complete execution in all cases.
    • A standardized pattern with acceptable behavior should probably take into consideration the following:
      • similar problems with finally{}
      • the problems arising from outputting on some streams when Stopping is initiated from Ctrl+C as mentioned in the related issues in OP here

    Footnotes:

    1. Atomic here has the meaning from the working definition specified in this question.

  4. alx9r commented on Dec 16, 2024

    @alx9r
    Author

    .Net Method Workaround Proof-of-Concept

    By the same doctrine as the test for the clean{try{}finally{}} workaround, the following test records the flow control of a script block whose execution is stopped at different stop: points:

    test code
    Add-Type -Path .\workaround24679.cs # https://gist.github.com/alx9r/f68f06828f33b6e611840fc5ce180ae7
    Add-Type @'
    public class WaitingDisposableMock : System.IDisposable {
        // global statics for tracking object lifetime
        public static int  Disposed    = 0;
        public static int  CleanedUp   = 0;
        public static bool Constructed = false;
        public WaitingDisposableMock(System.Threading.ManualResetEventSlim waitSignal) {
            Disposed    += 1;
            Constructed =  true;
            // Wait to complete construction if the signal exists.
            if (null != waitSignal) {
                waitSignal.Wait();
            }
        }
        public void Cleanup() {
            CleanedUp++;
        }
        public void Dispose() {
            Disposed++;
        }
    }
    public static class WaitSignal {
        public static void Await(System.Threading.ManualResetEventSlim waitSignal) {
            if (null != waitSignal) {
                waitSignal.Wait();
            }
        }
    }
    '@
    
    function Invoke-TestRun {
        param(
            [Parameter(Mandatory,ValueFromPipeline)]
            [ValidateSet('begin{} before ctor','begin{} inside ctor','begin{} after ctor','clean{} before shim','clean{clean(1)} before cleanup','clean{clean(1)} after cleanup','clean{clean(2)} before cleanup','clean{clean(2)} after cleanup')]
            [string]
            $StopPoint
        )
        process {
            [WaitingDisposableMock]::Disposed    = 0
            [WaitingDisposableMock]::CleanedUp   = 0
            [WaitingDisposableMock]::Constructed = $false
            $stopPoints = [System.Collections.Concurrent.ConcurrentDictionary[string,System.Threading.ManualResetEventSlim]]::new()
            $stopPoints[$StopPoint] = [System.Threading.ManualResetEventSlim]::new()
            $log = [System.Collections.Concurrent.ConcurrentQueue[string]]::new()
            $job =
                Start-ThreadJob                 `
                    -argu     $stopPoints,$log `
                    -script {
                        param($stopPoints,$log)
                        begin {
                            $log.Enqueue( 'begin{} before ctor')
                            [WaitSignal]::Await($stopPoints['begin{} before ctor'])                ; . {}
                            $log.Enqueue(                                    'begin{} inside ctor')
                            $obj = [WaitingDisposableMock]::new($stopPoints['begin{} inside ctor'])         ; . {}
                            $log.Enqueue( 'begin{} after ctor')
                            [WaitSignal]::Await($stopPoints['begin{} after ctor'])                 ; . {}
                            $log.Enqueue( 'begin{} complete')
                        }
                        clean {
                            $log.Enqueue('clean{} before shim')
                            [WaitSignal]::Await($stopPoints['clean{} before shim'])
                            [workaround]::clean({
                                param($i)
                                $log.Enqueue("clean{clean($i)} before cleanup")
                                [WaitSignal]::Await($stopPoints["clean{clean($i)} before cleanup"]); . {}
                                $obj | ? {$_} | % Cleanup
                                $log.Enqueue("clean{clean($i)} after cleanup")
                                [WaitSignal]::Await($stopPoints["clean{clean($i)} after cleanup"]) ; . {}
                                $log.Enqueue("clean{clean($i)} complete")
                        })}
                    }
            Start-Sleep -Milliseconds 100
            $job.StopJobAsync()
            Start-Sleep -Milliseconds 100
            $stopPoints[$StopPoint].Set()
            $job | Wait-Job | Out-Null
            $job | Remove-Job
            $clean1Progress = $log | ? {$_ -like 'clean{clean(1)*'} | Select-Object -Last 1
            $clean2Progress = $log | ? {$_ -like 'clean{clean(2)*'} | Select-Object -Last 1
            [pscustomobject]@{
                CleanupOK                   = if ([WaitingDisposableMock]::CleanedUp -xor [WaitingDisposableMock]::Constructed) { '❌' } else { '✅' }
                'stop:'                     = $StopPoint
                Constructed                 = [WaitingDisposableMock]::Constructed
                Disposed                    = [WaitingDisposableMock]::Disposed
                CleanedUp                   = [WaitingDisposableMock]::CleanedUp
                'begin{} progress'          = $log | ? {$_ -like 'begin{}*'}          | Select-Object -Last 1
                'clean{clean(1)}'           = $clean1Progress
                'clean{clean(2)}'           = $clean2Progress
                'clean{clean(1)} complete'  = if ($clean1Progress -eq 'clean{clean(1)} complete') { '✅' } else { '❌' }
                'clean{clean(2)} complete'  = if ($clean2Progress -eq 'clean{clean(2)} complete') { '✅' } else { '❌' }
            }
        }
    }
    
    'begin{} before ctor'           ,
    'begin{} inside ctor'           ,
    'begin{} after ctor'            ,
    'clean{} before shim'           ,
    'clean{clean(1)} before cleanup',
    'clean{clean(1)} after cleanup' ,
    'clean{clean(2)} before cleanup',
    'clean{clean(2)} after cleanup' |
        Invoke-TestRun              |
        Format-Table -AutoSize

    The analogous script block with stop: points annotated is as follows:

    begin {
        # stop: begin{} before ctor
        $obj = [SomeObject]::new() # stop: begin{} inside ctor
        # stop: begin{} after ctor
    }
    clean {
        # stop: clean{} before shim
        [workaround]::clean({
        # stop: clean{clean()} before cleanup
        $obj | ? {$_} | % Cleanup
        # stop: clean{clean($i)} after cleanup
    })}

    The following table is output by that test:

    CleanupOK stop:                          Constructed Disposed CleanedUp begin{} progress    clean{clean(1)}                clean{clean(2)}          clean{clean(1)} complete clean{clean(2)} complete
    --------- -----                          ----------- -------- --------- ----------------    ---------------                ---------------          ------------------------ ------------------------
    ✅         begin{} before ctor                  False        0         0 begin{} before ctor clean{clean(1)} complete                                ✅                        ❌
    ✅         begin{} inside ctor                   True        1         1 begin{} inside ctor clean{clean(1)} complete                                ✅                        ❌
    ✅         begin{} after ctor                    True        1         1 begin{} after ctor  clean{clean(1)} complete                                ✅                        ❌
    ❌         clean{} before shim                   True        1         0 begin{} complete                                                            ❌                        ❌
    ✅         clean{clean(1)} before cleanup        True        1         1 begin{} complete    clean{clean(1)} before cleanup clean{clean(2)} complete ❌                        ✅
    ✅         clean{clean(1)} after cleanup         True        1         2 begin{} complete    clean{clean(1)} after cleanup  clean{clean(2)} complete ❌                        ✅
    ✅         clean{clean(2)} before cleanup        True        1         1 begin{} complete    clean{clean(1)} complete                                ✅                        ❌
    ✅         clean{clean(2)} after cleanup         True        1         1 begin{} complete    clean{clean(1)} complete                                ✅                        ❌

    This demonstrates the following:

    • The machinery required to implement the workaround can be encapsulated in a .Net method.
    • Cleanup completes in all cases. Cleanup does not complete when the timing of the stop signal occurs after clean{} has been entered but before the call to ScriptBlock.Invoke().
    • If the job is stopped amidst execution of the [workaround]::clean({}) script block, $obj has CleanUp() invoked twice.

    This workaround also prevents writing to the error and success streams. That seems to abate problems associated with those streams when stopping is inititiated by Ctrl+C like in #19988.

  5. jborean93 commented on Dec 16, 2024

    @jborean93
    Collaborator

    This was an explicit decision during the implementation of the clean block to have it reflect the behaviour of finally. The PR that was merged #15177 stated

    • The Ctrl+c behavior is still pending. The Update the resource cleanup RFC PowerShell-RFC#294 discusses two different behaviors in the event of pipeline stopping in details in the Pipeline Stopping Behavior section. The current implementation in this PR mimics the finally clause behavior, but it keeps the changes in Stopper that would be needed to support non-cancellable clean block execution.

    The original PR had some more background information on both sides of the debate but in the end having finally/clean be cancellable was desired from a shell perspective over a consistency of state perspective. I think over time I have changed my mind to be in favour of the final choice but I'm sure there are pros and cons to both sides of the debate but in the end the current behaviour of clean and the finally block was explicitly chosen to be by design.

  6. alx9r commented on Dec 17, 2024

    @alx9r
    Author

    Jordan Borean (@jborean93) Thank you for that incite. That is very helpful.

    I see now the applicable deliberation in the RFC's Pipeline Stopping Behavior section:

    Pipeline Stopping Behavior

    Today, the way how the finally clause behaves in case of pipeline stopping is inconsistent:

    • If the pipeline stopping happens in the try clause, then the corresponding finally clause is guaranteed to run by suspending the stopping pipeline.
    • If the pipeline stopping happens in the finally clause, then the finally clause will be terminated immediately.

    ...

    ...the finally clause does not reliably execute in the event of pipeline stopping.
    So, when it comes to the clean block, we have a debate on two different behaviors:

    1. Mimic the current behavior of the finally clause.
    2. The clean block always runs to the end regardless of pipeline stopping, no matter it happens before or during its operation.

    Indeed there is substantial debate about those behaviors in the RFC and PR review. Evidently the current design was chosen and it led to me opening this issue. I certainly can see now how and why that decision was reached. I would say though, that given that the current behavior is an intentional design decision, the documentation for clean{} currently overstates its abilities in this statement:

    Resource cleanup is enforced for the following scenarios:

    1. when the pipeline is being stopped by Ctrl+c or StopProcessing()

    As it stands, out-of-the-box, a plain-old clean{} block cannot enforce cleanup as described by that statement without extraordinary measures like those described in this thread.

    I think that passage would more accurately be written as follows:

    Resource cleanup is enforced for the following scenarios:

    1. when the pipeline execution finishes normally without terminating error
    2. when the pipeline execution is interrupted due to terminating error
    3. when the pipeline is halted by Select-Object -First

    The clean block also enables users to write code that can enforce resource cleanup when the pipeline is being stopped by Ctrl+c or StopProcessing().

  7. SeeminglyScience commented on Dec 20, 2024

    @SeeminglyScience
    Contributor

    As it stands, out-of-the-box, a plain-old clean{} block cannot enforce cleanup as described by that statement without extraordinary measures like those described in this thread.

    The wording there is still correct, with an added caveat that if clean is running when Ctrl + C is pressed for the first time in that command invocation, it will be interrupted. Typically that'll be quite rare as most cleanup operations are very fast, but definitely still possible.

    That's just my opinion though, you're welcome to see if the docs folks would be down to add more details (MicrosoftDocs/PowerShell-Docs).

  8. added
    Issue-Discussionthe issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif
    and removed
    Needs-TriageThe issue is new and needs to be triaged by a work group.
    on Dec 20, 2024
  9. alx9r commented on Dec 20, 2024

    @alx9r
    Author

    ...you're welcome to see if the docs folks would be down to add more details...

    Patrick Meinecke (@SeeminglyScience) I'll consider doing so. Could you direct me to whatever guidance there is to accomplish that? I've had some unproductive experiences attempting to improve similar documentation, so I think I must be missing something about how to do this effectively.

  10. SeeminglyScience commented on Dec 21, 2024

    @SeeminglyScience
    Contributor

    They have a new issue template, afraid I don't have any additional information to share but that's where I'd suggest starting.

  11. iSazonov commented on Dec 21, 2024

    @iSazonov
    Collaborator

    By-design? I wonder why did we spend a lot of time to implement clean {}? The main intention was to clear resources when it's not possible in end {} block. In other words, Ctrl-C interrupting the work of all (begin {}, process {}, end {}) blocks should still ensure the execution of clean {} blocks in the pipeline (just as finally {}).

  12. alx9r commented on Dec 21, 2024

    @alx9r
    Author

    Ilya (@iSazonov)

    By-design?

    Strictly speaking, I think the behavior is by design. Arguably that design has shortcomings. But this part of PowerShell does seem to behave the way it does by design.

    I do have a related quibble about tagging this issue with "Resolution-By Design", though: Adding that tag marks the issue as "By Design". That is accurate. But, it also marks this issue as "Resolved". From my perspective this issue is not resolved. Perhaps the implementation of clean{} and finally{} should remain as designed, but in that case the problem of unreliable cleanup from my OP remains for me and anyone else facing this issue to solve.

    (As an aside I've disliked the "Resolution-By Design" tag since I first saw it because it conflates two orthogonal concepts: 1. Whether the issue reflects PowerShell behaving as designed. 2. Whether the issue is resolved. I'm not sure what to do with that grievance since there doesn't seem to be a meta forum for the PowerShell project like stackoverflow, for example, has.)

    I wonder why did we spend a lot of time to implement clean {}? The main intention was to clear resources when it's not possible in end {} block. In other words, Ctrl-C interrupting the work of all (begin {}, process {}, end {}) blocks should still ensure the execution of clean {} blocks in the pipeline (just as finally {}).

    Based on the historical documents I have reviewed, I think that last sentence reflects what the original intention of clean{} seems to have been. But it looks like that intention was departed from to achieve the current behavior to make it more consistent with finally{}. I don't think there's much point revisiting that decision if changing it would be a breaking change. Recently I've been assuming that changing clean{} behavior to address this issue would be a breaking change but I haven't put much thought into that question.

    For what it's worth, I think the Ctrl+C scenario is difficult to deliberate about because of how easily cleanup is curtailed by the console after interruption (per #23786). But your point, I think, applies equally to stopping jobs where there is no such complication.

    For me a similar question to yours comes to mind: After so much has been invested in establishing a reliable cleanup mechanism in PowerShell, why wouldn't we want to abate these last few ways PowerShell itself deters cleanup?

    I don't know yet what the best abatement for this issue would be, but I've seen no arguments made for why such an abatement shouldn't at least be explored.

  13. iSazonov commented on Dec 21, 2024

    @iSazonov
    Collaborator

    For me a similar question to yours comes to mind: After so much has been invested in establishing a reliable cleanup mechanism in PowerShell, why wouldn't we want to abate these last few ways PowerShell itself deters cleanup?

    I don't know yet what the best abatement for this issue would be, but I've seen no arguments made for why such an abatement shouldn't at least be explored.

    I wasn't a fan of the appearance of this block as in my opinion binary cmdlets are simpler and more reliable in that sense. But definitely the original request was "to release resources like dispose in C#". If it was transformed into something else and this block doesn't clear resources, why is it needed at all? If it transformed into “do as finally”, then that highlights the problem that finally also doesn't work as well as it should (as expected as in C#). All this will work if terminate (soft kill) is used instead of kill semantics (in process terms).

  14. 5 remaining items

  15. SeeminglyScience commented on Dec 23, 2024

    @SeeminglyScience
    Contributor

    I agree and don't think see anything in the RFC and PR that suggests this implication was clearly understood at the time.

    There are two PRs, the first was created by Rain who was the original author of the change. The second was created by Dongbo after the discussion was already completed. He decided to take over the PR, presumably in order to not force Rain to complete a lot of extra work that they didn't agree with (and it had been discussed for a large chunk of time at that point).

    You can see more of the back and forth in the original PR: #9900

    But yes, it was indeed understood and discussed at length.

  16. alx9r commented on Dec 24, 2024

    @alx9r
    Author

    There are two PRs, the first was created by Rain who was the original author of the change.
    ...
    You can see more of the back and forth in the original PR: #9900

    Patrick Meinecke (@SeeminglyScience) Thank you for referring me to that earlier PR. I was not aware of that history. Somehow I didn't see it mentioned in the more recent records. I have reviewed #9900 as well. My understanding of that thread as it relates to this issue follows.

    Two distinct scenarios requiring decisions

    I see two distinct scenarios that emerged that required a decision about their respective behaviors:

    • User presses Ctrl+C for the first or nth time when execution in the console runspace is in clean{}
    • Automation invokes Stop-Job for the first and only time when execution in the job runspace is in clean{}

    For brevity I propose referring to these as the Ctrl+C and Stop-Job scenarios, respectively.

    Ctrl+C: User presses Ctrl+C for the first or nth time when execution in the console runspace is in clean{}

    This is not the scenario that corresponds to the issue described in my OP.

    This is the scenario about which much of the discussion in #9900 is centered.

    There are many arguments on both sides of this debate, but ultimately, for this scenario it seems to have been accepted that regardless of any other consideration when a user presses Ctrl+C while clean{} is executing in the console runspace, the executing pipeline is de facto interrupted. I reviewed the deliberations about this as it was represented in the RFC and the latter PR #15177 in a prior comment. When I wrote that I was nearly persuaded that this behavior was accurately characterized as a design decision. But after reviewing #9900 I think it's more accurate to say that consensus about changing the behavior from that of finally could not be reached. So clean was implemented to mimick finally for this scenario. That's understandable and is the obvious safe course of action when consensus to make a change cannot be reached.

    Stop-Job: Automation invokes Stop-Job for the first and only time when execution in the job runspace is in clean{}

    This is the scenario that corresponds to the issue described in my OP.

    This scenario is only contemplated in passing in #9900. The first description of this scenario in #9900 seems to have been "hosted PowerShell runtime" with "Stop() called...from another thread." Eventually this scenario came to be referred to as the "automation" scenario in #9900.

    The behavior I expected for this scenario is described by Rob Holt here (my emphasis added):

    I'd say this is most consistent with the general scenario where this will occur, which is where a hosted PowerShell runtime will have Stop() called on it from another thread. The cleanup block could be doing anything at that point, so throwing an exception could be impossible or could do anything, which is basically what we're trying to avoid.

    So I think it makes sense to block the Stop() call on the cleanup block completing.

    It appears to me that behavior was taken as given from there on.

    Dongbo seems to first set out preferences about this behavior here:

    1. Clean block should be defined as it's guaranteed to get a chance to run, but no guarantee that it will run to the end

      • Guaranteed to get a chance to run:
        • The Clean block will run when the normal pipeline execution was successful
        • The Clean block will run when the normal pipeline execution was stopped by either a terminating error or Ctrl+c
      • No guarantee that it will run to the end:
        • The Clean block can be stopped by Ctrl+c, just like finally block can be stopped by Ctrl+c today.
          • The end user should have the power to cancel Clean block execution. When Ctrl+c happens during the normal running of the pipeline (Begin/Process/End), the normal pipeline execution should be stopped, but Clean blocks get their chance to run. However, when Ctrl+c happens again while a Clean block is running, the Clean block should be stopped and the rest to-be-run Clean blocks should be skipped.
          • This is very similar to the current try/finally behavior. When Ctrl+c happens while the try block is running, the try block execution will be stopped, but we temporarily suspend the stopping pipeline and give the finally block a chance to run. If Ctrl+c happens again while in finally, the finally block will be stopped.
      • The Clean block can be stopped by a terminating error thrown within it, but it shouldn't affect any other commands in the pipeline.

    There is no mention of stopping the Clean block using .Stop() in the No guarantee that it will run to the end section. If the intention at that time was that .Stop() should interrupt Clean I would expected that to be mentioned in that section.

    Dongbo's later comment seems to presume the "block the Stop() call" behavior for this scenario:

    So, I'm not super sure if the "non-interruptible" behavior is really needed. For automation scenario, it will work fine as Ctrl+c is not in the picture.

    There are a few more replies to that here, here, and here. But all of those replies are consistent with the original suggestion that "it makes sense to block the Stop() call on the cleanup block completing."

    I think there are two interpretations about what was decided about this scenario:

    1. This scenario fell through the cracks of the other deliberations and was never decided.
    2. This scenario was decided as "block the Stop() call on the cleanup block" but that hasn't yet been implemented.

    I the record does not seem to support

    • that the behavior in this scenario was explicitly decided, or
    • that the behavior in this scenario was decided to be as it is currently implemented.

    The neither-was-decided interpretation

    For what it's worth, I think the record also supports the interpretation (perhaps better than any other interpretation) that behavior of neither of the above scenarios was decided.

    This is story that fits best to me after having read through all of these deliberations: Discussion around clean{} bogged down in the Ctrl+C debate in March 2021 with no resolution. Development on everything else related to the feature continued until a prototype by Dongbo was announced in April. That announcement included the following statement:

    The prototype doesn't do anything about Ctrl+c handling, since we don't yet have an agreement there.

    So somehow the prototype was made in a way that "doesn't do anything about Ctrl+C handling." That's completely understandable given the lack of consensus. I expect I'd have done the same if I were in that situation. Development then continued into October 2021 with no further discussion about Ctrl+c and still no explicit decision and also no change to behavior. The feature was released in December 2021's v7.3.0-preview.1 with no explicit decision ever having been made about the Ctrl+C scenario.

    As for the Stop-Job scenario, that is, the scenario that corresponds to this issue: That scenario just incidentally ended up with the same behavior as the Ctrl+C scenario.

  17. SeeminglyScience commented on Dec 24, 2024

    @SeeminglyScience
    Contributor

    I see two distinct scenarios that emerged that required a decision about their respective behaviors:

    • User presses Ctrl+C for the first or nth time when execution in the console runspace is in clean{}
    • Automation invokes Stop-Job for the first and only time when execution in the job runspace is in clean{}

    They are the same mechanism, there just isn't a straight forward catch all term so ctrl + c was used as it's the primary use case. References you see to a Stop method are talking about REPL implementations, as we will call PowerShell.Stop() when ctrl + c is pressed.

  18. iSazonov commented on Dec 24, 2024

    @iSazonov
    Collaborator

    By-design? I wonder why did we spend a lot of time to implement clean {}? The main intention was to clear resources when it's not possible in end {} block. In other words, Ctrl-C interrupting the work of all (begin {}, process {}, end {}) blocks should still ensure the execution of clean {} blocks in the pipeline (just as finally {}).

    I'm a little confused here, the behavior we're discussing is what you argued for in the original PR #9900 (comment). Maybe I'm reading something wrong.

    I mean, don't get me wrong I don't agree with the decision and I voiced that at the time, but a decision was made.

    This comment was about the fact that even C# does not guarantee the release of resources and we should not expect that pwsh will be guaranteed to perform finally and clean.
    Otherwise, my expectations were as I described here. My only omission was that the Dongbo's example used Write-Host rather than Write-Output, and I didn't see the deviations that we were discussing now.
    And I think we should take the next step and improve finally and clean.

  19. alx9r commented on Dec 25, 2024

    @alx9r
    Author

    From my earlier comment:

    I do have a related quibble about tagging this issue with "Resolution-By Design", though: Adding that tag marks the issue as "By Design". That is accurate. But, it also marks this issue as "Resolved". From my perspective this issue is not resolved. Perhaps the implementation of clean{} and finally{} should remain as designed, but in that case the problem of unreliable cleanup from my OP remains for me and anyone else facing this issue to solve.

    This is no longer merely a quibble since using this overloaded tag leads to prejudicial closing of issues. I recognize that this might be an oversight, but it does leaves me wondering whether this policy remains such in good faith.

    (As an aside I've disliked the "Resolution-By Design" tag since I first saw it because it conflates two orthogonal concepts: 1. Whether the issue reflects PowerShell behaving as designed. 2. Whether the issue is resolved. I'm not sure what to do with that grievance since there doesn't seem to be a meta forum for the PowerShell project like stackoverflow, for example, has.)

    I no longer consider this an aside. I'm now looking for the appropriate forum in which to properly voice this grievance.

  20. added
    WG-Enginecore PowerShell engine, interpreter, and runtime
    WG-NeedsReviewNeeds a review by the labeled Working Group
    and removed on Dec 25, 2024
  21. iSazonov commented on Dec 25, 2024

    @iSazonov
    Collaborator

    I think we all want these blocks to work better than they do now.

  22. alx9r commented on Dec 26, 2024

    @alx9r
    Author

    Shims aren't reliable workarounds

    The stop-safe shims did not survive testing. In particular, if Stop occurs after clean{} is entered but before the shim invokes its workaround script block, invoking the workaround script block throws an exception. The test results are summarized in the following table:

    CleanupAlwaysOK CleanupBlock
    Technique clean finally
    cleanup{try{}finally{}} $${\color{green}✓}$$ $${\color{green}✓}$$
    shim $${\color{red}✕}$$ $${\color{green}✓}$$
    details

    The following is a summary of testing cleanup when interrupted by a Stop signal. Two techniques that attempt to ensure cleanup occurs, cleanup{try{}finally{}} and shim, are tested. The script blocks under test are analogous to the following script blocks . The script blocks are annotated with the locations at corresponding to the various point at which the stop signal was received during testing.

    cleanup{try{}finally{}}

    begin {
        # stop: begin{} before ctor
        $obj = [SomeObject]::new() # stop: begin{} inside ctor
        # stop: begin{} after ctor
    }
    clean {
        # stop: 'clean{} before try{}'
        try {
            # stop: clean{try{}} before =
            $clean = { $obj | ? {$_} | % CleanUp }
            # stop: clean{try{}} after =
            # invoke cleanup, this might be interrupted
            . $clean
        }
        finally {
            # stop: clean{finally{}}
            # invoke cleanup, this might be interrupted
            . $clean
        }
    }

    shim

    begin {
        # stop: begin{} before ctor
        $obj = [SomeObject]::new() # stop: begin{} inside ctor
        # stop: begin{} after ctor
    }
    clean {
        # stop: clean{} before shim
        [workaround]::clean({
        # stop: clean{clean()} before cleanup
        $obj | ? {$_} | % Cleanup
        # stop: clean{clean($i)} after cleanup
    })}

    Details

    CleanupOK combination
    StopPoint cleanup{try{}finally{}}
    finally
    cleanup{try{}finally{}}
    clean
    shim
    finally
    shim
    clean
    cleanup{cleanup(1)} after cleanup $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    cleanup{cleanup(1)} before cleanup $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    cleanup{cleanup(1)} complete $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    cleanup{cleanup(2))} complete $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    cleanup{cleanup(2)} after cleanup $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    cleanup{cleanup(2)} before cleanup $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    cleanup{cleanup{}}} before shim $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{red}✕}$$
    interrupted{} after ctor $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    interrupted{} before ctor $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    interrupted{} complete $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$
    interrupted{} inside ctor $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$ $${\color{green}✓}$$

    $PSVersionTable

    Name                           Value
    ----                           -----
    PSVersion                      7.4.6
    PSEdition                      Core
    GitCommitId                    7.4.6
    OS                             Microsoft Windows 10.0.22631
    Platform                       Win32NT
    PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
    PSRemotingProtocolVersion      2.3
    SerializationVersion           1.1.0.1
    WSManStackVersion              3.0

    This leaves the following cleanup{try{}finally{}} pattern as the only known (to me, at least) way to structure a script block such that it achieves reliable cleanup after Stop-Job:

    begin   {<#...#>}
    process {<#...#>}
    end     {<#...#>}
    clean {
        $clean = {<#...#>}
        try     { . $clean }
        finally { . $clean }
    }

    <#...#> indicates user code.

    Note the following known (to me, at least) restrictions that apply to this pattern in order to accomplish reliable cleanup:

    • $clean will run to completion either once or twice after one of the other blocks has been entered. Care should be taken when authoring $clean that invoking it twice does not cause other problems.
    • Either the first or second invocation of $clean itself (but not both invocations) can be interrupted at any point by Stop-Job. Care should be taken when authoring $clean to ensure the following:
      • interruption at any point of the first invocation does not prevent the second invocation from successfully cleaning up
      • interruption of the second invocation after the first invocation has run to completion does not itself leave things in a state still requiring cleaned up
    • Reliable assignment of variables involved in cleanup is sometimes necessary. Accomplishing that reliably when execution is interrupted by Stop-Job such that cleanup occurs is a separate issue and is covered in Provide a way to assign a returned object to a variable that is explicitly atomic when Stopping PowerShell #24658.
    • This technique does not abate the curtailment of cleanup associated with Ctrl+C described in Finally block isn't executed upon Ctrl+C  #23786. In particular, an error that results in writing to the error stream from $clean results in curtailment of cleanup after Ctrl+C (but not after Stop-Job).
  23. daxian-dbw commented on Jan 7, 2025

    @daxian-dbw
    Member

    The .Stop method was discussed in the original PR. I didn't have a thorough search, but here is one that I found #9900 (comment):

    As you brought up, after suspending a stopping pipeline, the finally block cannot be interrupted by subsequent Ctrl+c. From the implementation perspective, this is because ConsoleHost today simply ignores the signal when it finds there is already one stopping thread on-going, and thus the _pipelineStopper.IsStopping cannot be flipped back to true. However, in case that the stop is initiated through API PowerShell.Stop, then it's possible to call PowerShell.Stop again on a different thread to stop the finally block. Nevertheless, the behavior of finally with regarding to Ctrl+c is inconsistent today because of how ConsoleHost handles the signal.

    As Patrick Meinecke (@SeeminglyScience) mentioned, the Stop/StopAsync APIs are with the same mechanism as ctrl+c. The APIs are mentioned in the RFC but was not discussed in detail as a separate topic.

    When the clean block PR #15177 was merged, the main purpose was to have a finally-like thing that covers all Begin/Process/End blocks, so that terminating errors thrown from any of those blocks can be handled without needing try/finally redundantly in those blocks with the same or similar cleanup code. The discussion about the pipeline-stopping behavior didn't reach a conclusion at the time of the PR. The PR followed the finally behavior for obvious reasons -- consistency, safety, and simplicity, but by no means that's the end of discussion or the final decision. I explicitly called out in #15177 (comment):

    ... In a nutshell, there was no conclusion in the previous committee discussion.
    For this PR, I follow the current finally behavior, and we can leave the change of that behavior to a separate PR once there is a conclusion.

    I think the "Pipeline Stopping Behavior" section of the Clean block RFC captures the problems and concerns pretty well.

  24. alx9r commented on Jan 15, 2025

    @alx9r
    Author

    For what it's worth, the test fixture used to reproduce #24782 (fixture particulars) can be used to reproduce the behavior that is the subject of this issue using the following commands:

    Import-Module .\Assuage.PowerShell.Diagnostics.ControlFlow\
    
    Trace-ControlFlow                         `
            -WaitPoint 'stop command clean{}' `
            -NoCommand dot-source             `
            -DryRun    $false                 |
        % Log

    That code outputs the trace

    upstream begin{}
    command begin{}
    downstream begin{}
    upstream process{1}
    command process{1}
    downstream process{1}
    upstream process{2}
    command process{2}
    downstream process{2}
    upstream end{}
    command end{}
    downstream end{}
    upstream clean{}
    command clean{} start

    which is missing command clean{} completed.

    In contrast, Trace-ControlFlow -NoCommand dot-source -Pipeline 'Normal Upstream' traces the the same pipeline without the stop signal and shows commmand clean{} completed in its trace log.

    The good news is that that the behavior described in this issue seems to be the only scenario where a script block invoked by the . dot-source operator does not complete clean{} as a result of either an external stop signal or error.

    Using the same test fixture module,

    Get-ControlFlowTestParameters `
            -Arm ScriptBlock      |
        Test-ControlFlow          |
        ? {-not $_.CleanupOk}     |
        Format-List Name,Parameters

    outputs only

    Name       : stop command clean{}
    Parameters : {[NoCommand, dot-source],
                  [DryRun, False],
                  [WaitPoint, stop command clean{}]}

    That tests more than 30 other distinct scenarios all of which clean up OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Issue-Discussionthe issue may not have a clear classification yet. The issue may generate an RFC or may be reclassifNeeds-TriageThe issue is new and needs to be triaged by a work group.WG-Enginecore PowerShell engine, interpreter, and runtimeWG-NeedsReviewNeeds a review by the labeled Working Group

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions