Revamp pipeline thread handling

Rob Holt (@rjmholt)

A lot of the problems we face is based around handling of the pipeline thread. In order to invoke something on it, we need to interact with the PowerShell class, making us invoke at least a small bit of script in most cases. The reason for this is that the actual thread used by PowerShell is internal to the runspace by default. The only way to access it is to invoke a command of some sort.

This is the default experience, but we can change it. Runspace has a property called ThreadOptions. One of the choices for that enum is UseCurrentThread. So what we can do is start our own thread, create the runspace there with that option, and never give up that thread.

One of the biggest wins here would be that we could call PSConsoleReadLine.ReadLine directly without having to invoke something. We could also ditch using the thread pool to wait for PowerShell.Invoke to finish (which probably causes some dead locks). We could get rid of a lot of the more complicated code in PowerShellContext.

I'm pretty confident that if this doesn't outright solve a lot of the sluggishness and dead locks, it'll at the very least be significantly easier to debug.

~~The rest of this post is taken from #980 since the idea is the same:~~ Nvm, just look at Rob Holt (@rjmholt)'s code and the rest of conversation. The linked post is pretty outdated.

Patrick Meinecke (@SeeminglyScience)

I've been thinking about this too, and wrote a simple consumer that allows asynchronous dispatch of PowerShell commands like this:

    public class AsyncPowerShell
    {
        private readonly PowerShell _pwsh;

        private readonly Runspace _runspace;

        private readonly BlockingCollection<Tuple<PSCommand, TaskCompletionSource<IReadOnlyCollection<PSObject>>>> _commandQueue;

        private readonly Thread _pwshThread;

        private readonly CancellationTokenSource _cancellationSource;

        public AsyncPowerShell()
        {
            _commandQueue = new BlockingCollection<Tuple<PSCommand, TaskCompletionSource<IReadOnlyCollection<PSObject>>>>();
            _cancellationSource = new CancellationTokenSource();
            _runspace = RunspaceFactory.CreateRunspace();
            _runspace.Open();
            _pwsh = PowerShell.Create();
            _pwsh.Runspace = _runspace;
            _runspace.Debugger.DebuggerStop += OnDebuggerStop;
            _pwshThread = new Thread(RunPwshConsumer);
            _pwshThread.Start();
        }

        public Task<IReadOnlyCollection<PSObject>> ExecutePowerShellAsync(string command)
        {
            var completionSource = new TaskCompletionSource<IReadOnlyCollection<PSObject>>();
            _commandQueue.Add(new Tuple<PSCommand, TaskCompletionSource<IReadOnlyCollection<PSObject>>>(new PSCommand().AddCommand(command), completionSource));
            return completionSource.Task;
        }

        public void Stop()
        {
            _commandQueue.CompleteAdding();
            _cancellationSource.Cancel();
            _pwshThread.Join();
        }

        private void RunPwshConsumer()
        {
            try
            {
                foreach (Tuple<PSCommand, TaskCompletionSource<IReadOnlyCollection<PSObject>>> executionRequest in _commandQueue.GetConsumingEnumerable(_cancellationSource.Token))
                {
                    try
                    {
                        _pwsh.Commands = executionRequest.Item1;
                        executionRequest.Item2.SetResult(_pwsh.Invoke());
                    }
                    catch (OperationCanceledException)
                    {
                        executionRequest.Item2.SetCanceled();
                    }
                    catch (Exception e)
                    {
                        executionRequest.Item2.SetException(e);
                    }
                    finally
                    {
                        _pwsh.Commands.Clear();
                    }
                }
            }
            catch (OperationCanceledException)
            {
                // End nicely
            }
        }

        private void OnDebuggerStop(object sender, DebuggerStopEventArgs args)
        {
            Console.WriteLine("Debugger stopped");
        }
    }

I think with some added sophistication, we could build off such a model to add:

Nested prompt and debugging support
Per-execution cancellations, allowing us to skip particular requests
An implementation based on PowerShell's BeginInvoke and EndInvoke methods
Cancellation that works into PowerShell's own pipeline cancellation mechanisms

The only part I don't quite understand in your description Patrick Meinecke (@SeeminglyScience) is why we need another thread for message dispatch.

Dongbo Wang (@daxian-dbw) might also be interested in this discussion

Patrick Meinecke (@SeeminglyScience)

The only part I don't quite understand in your description Patrick Meinecke (@SeeminglyScience) is why we need another thread for message dispatch.

Oops that made sense in the context of the original thread, but OmniSharp already does that. I'll rip that out

Love the code btw ❤️. Basically the same thing I was picturing, except include an option to queue a Action<EngineIntrinsics, CancellationToken> that sets Runspace.DefaultRunspace before invocation (maybe needed maybe not). That way we can maybe run ReadLine, CommandInvocationIntrinsics.GetCommand, and/or CommandCompletion.CompleteInput (we might already do this?) without a command processor.

Would also maybe make accessing some internals via reflection a bit safer. I think there have been a few situations where it would have been worth it to use an internal API, but threading concerns made it really risky.

~~The other thought I had was to also include a method to queue a non-PowerShell action, so we could run something on the pipeline thread without having the overhead of executing PowerShell~~

This is exactly what you're saying...

Rob Holt (@rjmholt)

One of the choices for that enum is UseCurrentThread.

I think setting the Runspace.DefaultRunspace should be enough to directly calling ReadLine without requiring __Invoke-ReadLineForEditorService.
But we will need to make sure the Runspace is not used by anything else when PSReadLine is doing its work.

Setting UseCurrentThread in ThreadOptions only indicates that when invoking a script/command with that PowerShell instance (say ps), the actual invocation happens on the calling thread.
So if you later call ps.Invoke() to a different thread, then that thread will be used instead.
Also, a gotcha is that PowerShell's BeginInvoke and EndInvoke methods will cause a deadlock when using UseCurrentThread. It's more like a bug to me.

Rob Holt (@rjmholt) Patrick Meinecke (@SeeminglyScience) Do you mean the Dispatch pool section in the PR description is not applicable anymore?

Rob Holt (@rjmholt)

One of the choices for that enum is UseCurrentThread.

I think setting the Runspace.DefaultRunspace should be enough to directly calling ReadLine without requiring __Invoke-ReadLineForEditorService.
But we will need to make sure the Runspace is not used by anything else when PSReadLine is doing its work.

I know there are a few other things that are thread static though. Drawing a blank on them, but iirc one has to do with DSC cache. Also I do think it would easier to manage access to the thread than it is to manage what is using the runspace (that's more or less what we do today).

Setting UseCurrentThread in ThreadOptions only indicates that when invoking a script/command with that PowerShell instance (say ps), the actual invocation happens on the calling thread.
So if you later call ps.Invoke() to a different thread, then that thread will be used instead.

Yeah, the idea being is that ps.Invoke() will only be called on the thread we create.

Also, a gotcha is that PowerShell's BeginInvoke and EndInvoke methods will cause a deadlock when using UseCurrentThread. It's more like a bug to me.

That's interesting... I kind of expected that would throw. Similar to the "cannot invoke async on nested pipeline" exception.

Rob Holt (@rjmholt) Patrick Meinecke (@SeeminglyScience) Do you mean the Dispatch pool section in the PR description is not applicable anymore?

He was talking about a different part I already removed, but yeah that one kind of isn't either. That's also already mostly handled by omnisharp. I went ahead and just removed all that since Rob Holt (@rjmholt)'s code shows what I'm talking about better than I was explaining it.

The parts I'm not quite clear on here are:

How do we handle when the debugger gets dropped into?
Can we use this design to fix other debugger issues like Wait-Debugger in an argument completer?
How do we programmatically trigger a cancellation in the current PowerShell execution and what do we do if we are unable to cancel it? Do we try to abort it, clean it up and start a new one, or is that going to usually leave things in a bad state anyway?
Is there a simple way we can traverse the queue and sift out things that have been cancelled ahead of time? Or perhaps it's enough to just evaluate if something's been cancelled before we evaluate it? If so, do we set an exception or return some indicative value?

fix other debugger issues like Wait-Debugger in an argument completer?

That sounds like an unexpected use to me. What is the scenario?

I looked into the PSES source code recently, and get the idea of how script execution for intellisense is done via the OnIdle event.
The logic might not work if PSES starts to call ReadLine directly instead of from a pipeline. Or to be more precise, the OnIdle event might not work with PSReadLine, because the Runspace will be idle for most of the time (PSReadLine spinning in ReadKey), so a PulsePipeline will be automatically started by the Runspace from time to time, but ReadLine doesn't expect that something is running in the pipeline, and may invoke script/command in a nested pipeline (PowerShell.Create(CurrentRunspace)), and result in conflicts and failures. Never mind, PipelineBase.DoConcurrentCheck handles the case where a pulse pipeline is running. So this is probably not a problem.

What does the existing ThreadController do in PSES?

Dongbo Wang (@daxian-dbw)

That sounds like an unexpected use to me. What is the scenario?

[Powershell Preview regression bug] Debugger hangs when a breakpoint inside a scriptblock is hit vscode-powershell#2448
Integrated console hangs if PSReadline sets AddToHistoryHandler vscode-powershell#2246

Dongbo Wang (@daxian-dbw) do you know if PSReadLine handles either of those?

For PowerShell/vscode-powershell#2448, PSReadLine doesn't handle breaking into debugger or the nested debugger prompt. The debugger prompt is handled by the PowerShell host. With ConsoleHost, it will call into PSConsoleHostReadLine function again from the nested prompt.

For PowerShell/vscode-powershell#2246, the script block is converted to a delegate which calls ScriptBlock.InvokeAsDelegateHelper to execute the script in the DefaultRunspace. BTW, I cannot reproduce the hang.

Just a reminder if anyone is still working on this currently, if y'all get stuck, want advise, or would like me to work on something for this, let me know. DM's are open on the PS discord same name there (there's also a #vscode-contributors channel). Always down to chat PSES! 🙂

Rob Holt (@rjmholt)

There's a Discord too??? Rob Holt (@rjmholt) is currently working on this, and when I'm done getting my feet wet (read: setting up PSES so I can actually debug it all and get the OmniSharp update done) he's going to transition it over to me (read: work on finishing it together).

Andy Jordan (@andschwa)

Andy Jordan (@andschwa) yes the PS discord is quite popular :)

Andy Jordan (@andschwa)

Andy Jordan (@andschwa) welcome back !

Big day! Congrats and thank you for all the hard word to get this done.

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

Revamp pipeline thread handling #1295

123 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Sunbelt Computer Software

PL/B Language Development and Support

Revamp pipeline thread handling #1295

Description

Activity

rjmholt commented on May 21, 2020

SeeminglyScience commented on May 21, 2020

SeeminglyScience commented on May 21, 2020

rjmholt commented on May 21, 2020

daxian-dbw commented on May 21, 2020

SeeminglyScience commented on May 21, 2020

rjmholt commented on May 21, 2020

daxian-dbw commented on May 21, 2020

rjmholt commented on May 21, 2020

daxian-dbw commented on May 21, 2020

123 remaining items

SeeminglyScience commented on Mar 24, 2021

andyleejordan commented on Mar 24, 2021

JustinGrote commented on Mar 24, 2021

fMichaleczek commented on Mar 27, 2021

JustinGrote commented on Oct 29, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions