-PipelineVariable is only assigned output from the first execution of the first function block

[bstrautin]

Steps to reproduce

function foo {
  [cmdletbinding()]
  Param([parameter(ValueFromPipeline)]$a)
  Begin   {1}
  Process {2}
  End     {3}
}
Write-Host -ForegroundColor Green 'Expected:'
1..3|foo|%{$_} -pv x -ov y |select {$x},{$_},{$y}|out-host

Write-Host -ForegroundColor Green 'Actual:'
1..3|foo       -pv x -ov y |select {$x},{$_},{$y}|out-host

Output:

Expected:

$x $_            $y
-- --            --
 1  1             1
 2  2        {1, 2}
 2  2     {1, 2, 2}
 2  2  {1, 2, 2, 2}
 3  3 {1, 2, 2, 2…}

Actual:

$x $_            $y
-- --            --
 1  1             1
    2        {1, 2}
    2     {1, 2, 2}
    2  {1, 2, 2, 2}
    3 {1, 2, 2, 2…}

Expected behavior

All output produced in any block (Begin, Process, or End) should be assigned to the PipelineVariable as it enters the pipeline. -OutVariable appears to behave correctly, i.e. it captures output that -PipelineVariable does not.

Actual behavior

Only output produced by the first block to exectute, and only on the first execution of that block is assigned to the PipelineVariable.

Examples:

• if a Begin block exists, the output from the Begin block will be assigned, but the output from the Process and End blocks will not.
• if a Begin block does not exists, the output from the first execution of the Process block will be assigned, but the output from subsequent executions of the Process block will not, and the output of the End block will not.

In the wild, on 5.1:

https://stackoverflow.com/questions/45782372/powershell-advanced-function-output-pipelinevariable-doesnt-work

https://www.reddit.com/r/PowerShell/comments/doboko/support_for_pipelinevariable_in_advanced_function/

Environment data

Name                           Value
----                           -----
PSVersion                      7.0.0-preview.5
PSEdition                      Core
GitCommitId                    7.0.0-preview.5
OS                             Microsoft Windows 10.0.18362
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[iSazonov]

Debug shows that we set the pipeline variable well.
Then I discovered that formatting doesn't work as expected:

1..3 | ft @{name="x"; Expression={Get-Date}}
1
2
3

[bstrautin]

After some debugging, and it appears that the problem is that the PipelineVariable is removed from the wrong scope when PSScriptCmdlet.RunClause calls ExitScope().

ExitScope() calls MshCommandRuntime.RemoveVariableListsInPipe(), which then calls _state?.PSVariable.Remove(this.PipelineVariable).

It looks like RemoveVariableListsInPipe is trying to make sure that PipelineVariable doesn't exist in the scope that is used to execute the function's functions (put in place by PSScriptCmdlet.EnterScope).

But the variable doesn't exist in that scope, so it ends up being removed from PSScriptCmdlet's Context.CurrentCommandProcessor._previousScope.Variables instead, and it's never re-added.

My guess for a fix would be relocating the calls to SetVariableListsInPipe() and RemoveVariableListsInPipe() from EnterScope() and ExitScope() to BeginProcessing() and DoEndProcessing(), respectively. I'd need to do more digging to understand whether that's reasonable or not.

[iSazonov]

Perhaps @SeeminglyScience and @vexx32 could have thoughts.

[thlac]

Ran into this issue yesterday with the cmdlet "Get-NetFirewallRule", and used this post to verify that the issue seems to be the same. For my own sake I added an expression to the repro script provided in the original post to match the findings about the variable being removed.

Script:

function foo {
  [cmdletbinding()]
  Param([parameter(ValueFromPipeline)]$a)
  Begin   {1}
  Process {2}
  End     {3}
}
Write-Host -ForegroundColor Green 'Expected:'
1..3|foo|%{$_} -pv x -ov y |select @{n="pv_set";e={$_ ? $true : $false}},{$x},{$_},{$y}|out-host

Write-Host -ForegroundColor Green 'Actual:'
1..3|foo       -pv x -ov y |select @{n="pv_set";e={$x ? $true : $false}},{$x},{$_},{$y}|out-host

Output:

Expected:

pv_set $x $_            $y
------ -- --            --
  True  1  1             1
  True  2  2        {1, 2}
  True  2  2     {1, 2, 2}
  True  2  2  {1, 2, 2, 2}
  True  3  3 {1, 2, 2, 2…}

Actual:

pv_set $x $_            $y
------ -- --            --
  True  1  1             1
 False     2        {1, 2}
 False     2     {1, 2, 2}
 False     2  {1, 2, 2, 2}
 False     3 {1, 2, 2, 2…}

[vexx32]

I've not really dug into how -PipelineVariable works, but this does pretty definitively seem broken to me 🤷

[vexx32]

@bstrautin's investigations seem promising to me. I've seen those code areas from working on #9900 and what they mention makes sense at least on first pass.

We have a few solid test cases here in this issue, so any potential solutions should be fairly easy to test out. Are you looking to make a PR for this one @bstrautin? If not I'm happy to take a stab, this should really be fixed sooner rather than later.

EDIT: Also, might be good to get @daxian-dbw and/or @rjmholt's perspective here, if they're at all familiar with any of this code. I know it's some of the older code in the repo, so modifying it can at times be a little hairy.

[bstrautin]

No imminent PR from me.

I haven't spent the time to research the "Why"s of the current code, and expect that a robust fix can only be made by someone who does have a full understanding.

[vexx32]

Further investigation:

1. EnterScope() and ExitScope() are called at the start and end of RunClause(), so the pipeline variable seems to be added and removed in the correct places, or at least the reference to it appears to be in the right place.
2. The pipeline variable implementation for compiled commands is slightly different, but it's not clear how much of it is really needed for script commands.

Currently have some promising results, I'll see if I can roll it in cleanly and get some tests added to cover this issue properly.

[vexx32]

This seems to be all that's needed to ensure this is working correctly: vexx32@8f13c8b

I'll get some tests written in this evening to verify all is in order and then push a PR. 🙂