PowerShell / PowerShell

PowerShell for every system!
https://microsoft.com/PowerShell
MIT License
45.2k stars 7.28k forks source link

"Clean" block does not run or partially runs when using Ctrl+C #24241

Open FinneganMacePike opened 1 month ago

FinneganMacePike commented 1 month ago

Prerequisites

Steps to reproduce

Code to reproduce error:

function Test-Function {
    param(
        [Parameter(Mandatory = $true, ValueFromPipeline = $true)]
        [string]$InputTst,
        [Parameter(Mandatory = $false)]
        [switch]$ShowInput
    )
    if ($ShowInput) {
         Write-Host "You wrote: $( $InputTst )"
    }
}

function Start-Main {
    [CmdletBinding()]
    param()
    begin {}
    process {
        $Close = $false
        Write-Host "Process block runs."
        # Test 1 - Does not work.
        # $Testing = Read-Host "Please type something: "
        # Test 2 - Does not work.
        # Test-Function -ShowInput
        # Test 3 - Works properly.
        # Write-Host "Start Sleep"
        # Start-Sleep("10")
    }
    end {}
    clean {
        Write-Host "Clean block runs."
        Write-Host "Cleaning...."
        $Close = $true
        Write-Host "Value: $( $Close )"
    }
}

try {
    Start-Main
}
finally {
    Write-Host "Finally block runs."
    Write-Host "Finally...."
    $Close = 8
    Write-Host "Value: $( $Close )"
}

Test 1

Uncomment the "Test 1" part of the process. Run the code. Ctrl+C when prompted for function parameter input. Sometimes, none of the "Clean" block output even shows at all. I have another personal script where this is the case. Other times, only the first Write-Host is run. However, in every case, the full "Clean Block" does not run. This seems to occur whenever Ctrl+C is used and the user is prompted for input.

Test 2

Comment the "Test 1" and uncomment the "Test 2" part of the process. Run the code. Ctrl+C when prompted for user input. Exactly the same issue as in "Test 1".

I questioned the Microsoft documentation that states: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_functions_advanced_methods?view=powershell-7.4 image

Test 3

Comment the "Test 2" and uncomment the "Test 3" part of the process. Run the code. Ctrl+C when the program is sleeping. The output is correct. It seems that Microsoft's documentation is in general correct, but fails specifically when terminated from a user input. I hope that this can be fixed, so that I can properly use the clean block.

Expected behavior

Test 1 >
Process block runs.

cmdlet Test-Function at command pipeline position 1
Supply values for the following parameters:
InputTst: Clean block runs.
Cleaning....
Value: True
Finally block runs.
Finally....
Value: 8

Test 2 >
Process block runs.
Please type something: : Clean block runs.
Cleaning....
Value: True
Finally block runs.
Finally....
Value: 8

Test 3 >
Process block runs.
Start Sleep
Clean block runs.
Cleaning....
Value: True
Finally block runs.
Finally....
Value: 8

Actual behavior

Test 1 >
cmdlet Test-Function at command pipeline position 1
Supply values for the following parameters:
InputTst: Clean block runs.
Finally block runs.
Finally....
Value: 8

Test 2 >
Process block runs.
Please type something: : Clean block runs.
Finally block runs.
Finally....
Value: 8

Test 3 >
Process block runs.
Start Sleep
Clean block runs.
Cleaning....
Value: True
Finally block runs.
Finally....
Value: 8

Error details

No response

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.5
PSEdition                      Core
GitCommitId                    7.4.5
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT

Visuals

No response

rhubarb-geek-nz commented 1 month ago

Might be worth checking with https://github.com/PowerShell/PowerShell/issues/23786 which shows similar behaviour.

In that situation Ctrl+C was stopping the pipeline, once the pipeline was stopped Write-Output operations failed. Might worth checking whether Write-Host is failing when the pipeline has stopped.

FinneganMacePike commented 1 month ago

Thank you @rhubarb-geek-nz for sharing the other issue. I decided to make a new issue because (in my experience), it was the clean block that was not working as expected, while the issue #23786 was about the finally block failing. The finally block was working properly for me. If you can find a case where the finally block also behaves unexpectedly, then I would be interested to know.

However, another reason why I decided to create a separate issue is that the Write-Output (alias echo) function seems to conflict with the finally block's purpose. The Write-Output function passes object(s) to the pipeline (without breaking like return) but was used in the finally block which should be used to cleanly terminate the script (hopefully accounting for termination errors as well). Usually returning something in the block that should, for example, close a network connection, may have some unintended consequences. Especially, if nothing is supposed to run after the finally block. I am relatively new to PowerShell scripting, so please correct me if I misunderstand something.

This is in contrast to the issue I created, where it seems to not work with just the Write-Host function and variable reassignment, which should just write to the console. It makes sense to write to the console in the finally or clean blocks, where you may want to gain insight into how your (again using the example) network connection is closing.

The last reason for why I created a separate issue is that it was really easy to reproduce this issue. I had a friend use my script and he had the same unexpected behavior occur. He will be sharing his Environment data, which I will post as soon as I can. (maybe an issue with a specific version, who knows?)

I hope both issues can be resolved, but I believe that this issue is different enough to warrant its own issue. If the issue causing this to happen comes from the same source, then I would be happy to have both issues resolved. Thank you again for sharing the #23786.

rhubarb-geek-nz commented 1 month ago

When writing a C# cmdlet the cleanup is performed in the IDisposable:Dispose() method, that is normally a "last chance saloon" to cleanup. If the cleanup for a function is implemented using similar mechanics (I don't know, I am speculating) there may not be much context of the cmdlet left, eg input and output pipelines are closed etc. For example it is standard behaviour to error when attempting to write to an output that is closed. Might be worth confirming the robustness of the cleanup by writing directly to files or variables, then we can see if the problem is writing to pipelines referenced by the cmdlet's context.

FinneganMacePike commented 1 month ago

@rhubarb-geek-nz Thank you for providing more information about the cleanup. For more context into how I found the error, I was actually using the clean block to close a FileStream. I Ctrl+C'd in a Read-Host, and then when I would run the script again, the file would still be considered locked by the prior script's process. Although not necessarily writing to a file, I think that should fulfill your question about robustness. I created the "Code to reproduce error" to provide a simpler and non-personal example.

FinneganMacePike commented 1 month ago

Also, my friend used the "Code to reproduce error", and also had the unexpected behavior occur. For him, it would work properly on the first run of the script, and then each subsequent run it would perform the unexpected behavior. He has a very similar environment to mine: Coworker Environment - True I wanted to reproduce the error before making an issue, so that I could at least say it is not my setup alone.

rhubarb-geek-nz commented 1 month ago

I was actually using the clean block to close a FileStream

I would recommend using try/finally rather than clean, effectively mimicking C#'s using.

$stream = [System.IO.File]::Open($fileName, [System.IO.FileMode]::Create )
try
{
    do stuff
}
finally
{
     $stream.Dispose()
}
FinneganMacePike commented 1 month ago

@rhubarb-geek-nz, I understand that you would recommend try, catch, finally, but that's not the issue. The issue is that the clean block is not working as intended. So, you are correct, in the meantime, finally can do what the clean block should.

It would really help if the new and really useful clean block could to be fixed. I really like the begin, process, end structure of functions, but having to use try, catch, finally in each of the input process blocks makes the code a lot harder to read. The alternative is ensuring that the function itself is always caught in a try, catch, finally, but then it HAS to be used like that each time. The begin, process, end blocks cannot be wrapped (to my knowledge) in try, catch, finally within the function definition.

Therefore, the clean block is the solution to my problems:

  1. It makes the code easier to read.
  2. Ensures that the function will always manage cleanup.
  3. Does not require the programmer to change how they use the function.

Thank you @rhubarb-geek-nz for providing a solution that could help solve a personal issue with cleanup, but my current worry is hopefully getting the clean functionality resolved.

rhubarb-geek-nz commented 1 month ago

It would really help if the new and really useful clean block could to be fixed.

Absolutely, or at least the known limitations documented.