Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Engine event processing bypasses "ShouldQueueAndProcessInExecutionThread" causing state corruption and crash due to Runspace affinity #4003

Closed
PaulHigin opened this issue Jun 13, 2017 · 5 comments
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-No Activity Issue has had no activity for 6 months or more Size-Week WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@PaulHigin
Copy link
Contributor

PaulHigin commented Jun 13, 2017

This was found while investigating a PowerShell class static/instance method concurrency bug.

Background

When invoking a script block using InvokeWithPipe, if the script block is bound to a different Runspace (e.g. created in a different Runspace), then the script block will be marshaled to that Runspace using the EventManager and is supposed to be executed by the main pipeline thread of that Runspace.

This is how it's done:

  1. The thread that is calling InvokeWithPipe finds that the script block is bound to another Runsapce and needs to run in that Runspace. (code here)
  2. The thread queues an event on the EventManager of the target Runspace, wishing the main pipeline thread of the target Runspace would pick up the event and run the script block. (code here)
  3. The thread waits for the main pipeline thread of the target Runspace to finish running the script block. (code here)

Issues

The problem is that, to avoid a possible hang, the thread, which waits for the main pipeline thread of the target Runspace, will pick up the event and execute it (not the required main pipeline thread) after waiting for 250 mSec (See the code here). This will result in 2 threads running in the same Runspace and changing its state simultaneously. This would cause:

  1. Deadlock.
  2. Runspace state corruption and process crash.

Repro Steps

Deadlock

Both arguments for '-sb' and '-arg' are script blocks that are created in the powershell console session. So when bar runs $sb.InvokeReturnAsIs($arg) in a new Runspace, it needs to marshal it back to the powershell console session. This is what happens:

  1. The new Runspace thread (aka. requesting thread) cannot execute the script block because it has to run in the powershell console session (aka. target Runspace) which is bound with the script block, so it queues an event for the pipeline thread of the target Runspace to run the script block;
  2. However, the target Runspace is completely unresponsive because it’s blocked on $ps.Invoke();
  3. So, after 250ms, the requesting thread go ahead to process the event itself to run the script block. Note that – an event is now executing;
  4. Again, the requesting thread finds it cannot run $arg and goes back to step (1). However, this time when it comes to step (3), an event is already executing. So this.ProcessPendingActions() will immediately return, and the requesting thread will be stuck in the while loop.
## The deadlock happens on PowerShell Core RC builds
$ps = [powershell]::Create()
$ps.AddScript('function bar { param([scriptblock]$sb, [scriptblock]$arg) $sb.InvokeReturnAsIs($arg) }').Invoke()
$ps.Commands.Clear()
$ps.AddCommand("bar").AddParameter('sb', {param([scriptblock] $s) $s.InvokeReturnAsIs()}).AddParameter('arg', {[Console]::WriteLine("blah")}) > $null
$ps.Invoke()

Runspace state corruption and process crash

Note: this doesn't repro on latest PowerShell Core anymore because we have fixed the PowerShell class static method to not route method execution to other Runspaces incorrectly. But the underlying problem in EventManager is still there. You can run this repro in Windows PowerShell v5.1 to see the results.

This repro creates a script DoInvokeInParallel.ps1 that dot-sources an Invoker.ps1 file which defines a class with a static method. Run DoInvokeInParallel.ps1 in multiple Runspaces via a RunspacePool to use the class static method concurrently.

Invoker.ps1 file

class Invoker
{
    static [object[]] Invoke(
        [scriptblock] $scriptToInvoke,
        [object[]] $args)
    {
        return $scriptToInvoke.Invoke($args)
    }
}

DoInvokeInParallel.ps1 file

$invokerPath = Join-Path $PSScriptRoot Invoker.ps1
. $invokerPath

$rsp = [runspacefactory]::CreateRunspacePool(1, 10, $host)
$rsp.Open()

$scriptTemplate = @'
    . {0}
    1..100 | foreach {{
        $results = [Invoker]::Invoke({{ "RS_{1} Loop $_ `r`n" }}, $null)
        Write-Output $results
    }}
'@

class Task
{
    [powershell] $powershell
    [System.IAsyncResult] $Async
}

$tasks = @()

1..10 | foreach {
    $task = [Task]::new()
    $script = $scriptTemplate -f $invokerPath,$_
    $task.powershell = [powershell]::Create()
    $null = $task.powershell.AddScript($script)
    $task.powershell.RunspacePool = $rsp
    $task.Async = $task.powershell.BeginInvoke()
    $tasks += $task
}

foreach ($task in $tasks)
{
    $results = $task.powershell.EndInvoke($task.Async)
    Write-Host $results
    Write-Host $task.powershell.Streams.Error
    $task.powershell.Dispose()
}

$rsp.Dispose()

Run DoInvokeInParallel.ps1. The result is multiple "invalid session state" asserts if you are using a debug flavor Windows PowerShell. Eventually, the process will crash.

@PaulHigin PaulHigin added WG-Engine core PowerShell engine, interpreter, and runtime Issue-Bug Issue has been identified as a bug in the product Size-Week labels Jun 13, 2017
@PaulHigin PaulHigin added this to the 6.0.0-Pending milestone Jun 13, 2017
@joeyaiello
Copy link
Contributor

Repro no longer fails.

@joeyaiello joeyaiello added the Resolution-Fixed The issue is fixed. label Sep 19, 2017
@daxian-dbw daxian-dbw reopened this Dec 12, 2017
@daxian-dbw daxian-dbw removed this from the 6.0.0-Pending milestone Dec 12, 2017
@SteveL-MSFT
Copy link
Member

@daxian-dbw did you reopen this as it's not fixed? if so, you should also remove the fixed label

@daxian-dbw daxian-dbw removed the Resolution-Fixed The issue is fixed. label Dec 12, 2017
@daxian-dbw
Copy link
Member

daxian-dbw commented Dec 12, 2017

@SteveL-MSFT This is not fixed. I will update the repro steps. The whole issue description has been updated.

@SteveL-MSFT SteveL-MSFT added this to the Future milestone Aug 18, 2018
@BrucePay BrucePay self-assigned this Aug 25, 2018
@daxian-dbw daxian-dbw changed the title Engine event processing bypasses "ShouldQueueAndProcessInExecutionThread" causing state corruption and crash Engine event processing bypasses "ShouldQueueAndProcessInExecutionThread" causing state corruption and crash due to Runspace affinity Nov 12, 2021
Copy link
Contributor

This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.

Copy link
Contributor

This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-No Activity Issue has had no activity for 6 months or more Size-Week WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Development

No branches or pull requests

5 participants