Revamp pipeline thread handling

[SeeminglyScience]

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 @rjmholt's code and the rest of conversation. The linked post is pretty outdated.

[rjmholt]

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 @SeeminglyScience is why we need another thread for message dispatch.

@daxian-dbw might also be interested in this discussion

[SeeminglyScience]

The only part I don't quite understand in your description @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

[SeeminglyScience]

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.

[rjmholt]

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...

[daxian-dbw]

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.

@rjmholt @SeeminglyScience Do you mean the Dispatch pool section in the PR description is not applicable anymore?

[SeeminglyScience]

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.

@rjmholt @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 @rjmholt's code shows what I'm talking about better than I was explaining it.

[rjmholt]

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?

[daxian-dbw]

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?

[rjmholt]

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

@daxian-dbw do you know if PSReadLine handles either of those?

[daxian-dbw]

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.