Dongbo Wang (@daxian-dbw) thanks for the review, I've just rebased on the latest master and made the change to move the cancellation token to the pipeline processor. There are probably a few things we want to decide on:

• Should the token be cancelled before or after StopProcessing is called on each command in the pipeline
  • Currently it will cancel it after
• Should the stop token be added the ICommandRuntime interface through a new ICommandRuntime3 and that is used as the check
  • Currently it just verifies the CommandRuntime of the PSCmdlet is MshCommandRuntime
  • This should always be the case so it's a simple Debug assertion but maybe that will change in the future
  • A new interface seems like a drastic change so I didn't go ahead with it

/azp run

Should the token be cancelled before or after StopProcessing is called on each command in the pipeline

Cancelling the token after StopProcessing would result in fewer branches in user PowerShell code to achieve correct results, I think. That would be better ergonomics for authors of Advanced Functions.

I'm assuming here that token cancellation and StopProcessing both happen on a different thread from the execution of the PowerShell instance being stopped. If StopProcessing occurs after cancelling the token then the PowerShell thread can continue to execute user PowerShell code after token cancellation but before StopProcessing whenever unfortunate thread scheduling occurs. That would mean PowerShell authors have to consider cases where the PowerShell engine has canceled the token but not yet stopped the pipeline. I expect that would necessarily lead to checks of $PSCmdlet.PipelineStopToken.IsCancellationRequested in user code to avert undesired behavior and extraneous errors arising from that condition.

Consider the following user code:

function Invoke-SomeCancellableWorkload {
    [CmdletBinding()]
    param()
    $result = [MyCancellableWorkload]::Invoke($PSCmdlet.PipelineStopToken)
    $result | ConvertTo-SomeOtherFormat
}

If the token is cancelled after StopProcessing, then the PowerShell engine will always interrupt PowerShell execution before $result | ConvertTo-SomeOtherFormat is executed when Invoke() is canceled. In that scenario, the engine currently completes assignment to $result despite the pipeline already being in the Stopping state, but does not execute $result | ConvertTo-SomeOtherFormat. (The behavior of assignments to PowerShell variables from .Net methods while stopping is contemplated in #24658. There are empirical test results demonstrating the behavior in this answer).

If, on the other hand, the token is cancelled before StopProcessing, then $result would have the return value of a cancelled call to Invoke() assigned to it. Let's say that value is $null. That then means that the author of Invoke-SomeCancellableWorkload has to consider the possibility of $result being $null when the line $result | ConvertTo-SomeOtherFormat is executed after token cancellation but before StopProcessing. That scenario is apt to cause at least an extraneous parameter binding error that would likely be observed by users whenever that unfortunate thread scheduling occurs. But the author might also deem that inputting $null to ConvertTo-SomeOtherFormat is unnacceptable because that results in unknown, undesired, or destructive behavior. Then the author would probably have to resort to something like the following:

function Invoke-SomeCancellableWorkload {
    [CmdletBinding()]
    param()
    $result = [MyCancellableWorkload]::Invoke($PSCmdlet.PipelineStopToken)
    . {}
    if ($PSCmdlet.PipelineStopToken.IsCancellationRequested) {
        throw [StopTokenCanceledBeforeStopProcessingException]::new()
        return
    }
    $result | ConvertTo-SomeOtherFormat
}

I think the former is more readable. But the latter would be necessary if cancellation occurs before StopProcessing.

alx9r in the script based functions it won't matter because script functions cannot implement StopProcessing, so whether the token is cancelled before or after the effectively no-op StopProcessing function is called won't make a different. The question is more important for compiled cmdlets as they can have their own StopProcessing implementation. Even then I don't think it matters too much.

Please keep in mind this PR is not trying to deal with any scenarios like variable assignment or the like. It is focused specifically on just exposing a CancellationToken for functions or cmdlets to use when calling methods that support a CancellationToken so it can stop its execution. In the majority of cases the cancellation will result in an OperationCanceledException meaning callers don't have to worry about checking the cancellation status of the token anyway.

Jordan Borean (@jborean93)

in the script based functions it won't matter because script functions cannot implement StopProcessing, so whether the token is cancelled before or after the effectively no-op StopProcessing function is called won't make a different.

I think I might be miscommunicating, then. For script functions, how do you refer to the point in execution on the thread that invokes stopping that causes flow of control of an advanced function to jump to clean{}? I was referring to that as StopProcessing because in all testing I have done, the invocation of StopProcessing on a compiled Cmdlet corresponds to the same timing as flow of control jump to clean{} (and $PSCmdlet.Stopping changing from false to true). Whatever that event is called, the argument I made here for StopProcessing surely applies to that event. In other words, whatever you call that event, there is a significant difference when authoring Advanced Functions whether that event happens before or after $PSCmdlet.PipelineStopToken is canceled.

I think you might be focusing too much on this cancellation/cleanup side. This PR enables two things for end users:

• Removes the need for common boilerplate done in compiled cmdlets that manually setup a CancellationToken
• Exposes the same functionality present in a compiled function into a script function
  • They can now call async functions with a cancellation token and do a blocking wait and it will respond to stop events
  • Currently you need to do a loop that frees up the engine every few ms to respond to a cancellation event

The talk about when clean is called in relation to stop is unrelated to this PR.

The talk about when clean is called in relation to stop is unrelated to this PR.

My concern is not about when clean{} is called in relation to stop. My concern is about when the $PSCmdlet.PipelineStopToken available in an Advanced Function is canceled in relation to when pipeline stopping affects the execution of the PowerShell code of that Advanced Function.

My concern is about when the $PSCmdlet.PipelineStopToken available in an Advanced Function is canceled in relation to when pipeline stopping affects the execution of the PowerShell code of that Advanced Function.

It's exactly the same as it is today, the only difference is that now there is a way to call .NET functions that accept a CancellationToken and have it respond to the stop request.

Normally this works just fine because a task that accepts a CancellationToken will raise the OperationCanceledException if set. For example

& {
    [CmdletBinding()] param()

    [System.Threading.Tasks.Task]::Delay(-1, $PSCmdlet.PipelineStopToken).GetAwaiter().GetResult()
    [Console]::WriteLine("Will not be called")
}

If the method does not raise the exception or you catch it yourself then it'll continue to run until it reaches a point where the engine checks if it is stopped. For example when calling a cmdlet, entering a loop.

& {
    [CmdletBinding()] param()

    try {
        [System.Threading.Tasks.Task]::Delay(-1, $PSCmdlet.PipelineStopToken).GetAwaiter().GetResult()
    }
    catch {
        [Console]::WriteLine("Exception $_")
    }

    [Console]::WriteLine("Will be called")
    Write-Host test  # PowerShell checks here if it is stopped or not
    [Console]::WriteLine("Will not be called")
}

# Exception Exception calling "GetResult" with "0" argument(s): "A task was canceled."
# Will be called

If the API doesn't use that token properly then you need to handle that accordingly. This works exactly the same as in a compiled cmdlet, if you don't release control back to the engine somehow then things will continue to run.

But once again, this is off topic to this PR. This is not about the stop implementation but rather about adding the CancellationToken to the PSCmdlet to allow cmdlets and now script functions to use that. The former removing the boilerplate needed, the latter actually making it possible.

Jordan Borean (@jborean93)

I think we are still talking past one another. I'll try a different approach. With $PSCmdlet.PipelineStopToken there are now two separate events that can affect the execution of user code when stopping an Advanced Function:

• cancellation of the token
• interruption of executing PowerShell code

Which event happens first?

The pipeline is marked as stopped before the pipeline calls StopProcessing on each cmdlet and thus sets the token to be cancelled. This PR doesn't change any of that and this is why I'm trying to say this is off topic and just adds noise to the PR.

In Stop() the _stopping field is set to true before pp.Stop() (what called StopProcessing on each cmdlet) is called.

The pipeline is marked as stopped before the pipeline calls StopProcessing on each cmdlet and thus sets the token to be cancelled.
...
In Stop() the _stopping field is set to true before pp.Stop() (what called StopProcessing on each cmdlet) is called.

Based on this and my attempt to trace through the source I think the thread invoking stopping completes the events in question in the following order:

1. _stopping is set to true. This has the effect that flow of user PowerShell code of the pipeline undergoing stopping will subsequently be controlled according to the stopping rules.
2. PipelineProcessor.Stop() is invoked which calls .Cancel() on the CancellationTokenSource from which $PSCmdlet.PipelineStopToken was produced.

Can you confirm that this is correct?

If that is correct, then my concern is addressed since that is the order that avoids the races I was concerned about.

I'm assuming here that token cancellation and StopProcessing both happen on a different thread from the execution of the PowerShell instance being stopped.
... that causes flow of control of an advanced function to jump to clean{}? I was referring to that as StopProcessing because ...

alx9r If I understand correctly, you refer to clean {} as StopProcessing. clean {} will always be invoked on the pipeline thread, which is the same thread that runs begin {}, process {}, and end {}. When pipeline is being stopped, clean {} runs after all running scripts are actually stopped. PipelineProcessor.Stop, on the other hand, runs on a separate thread.

Can you confirm that this is correct?

The order is correct.

Thanks for the reviews and comments on this one!