From eeae6ef3c85cddbda4f95be3e6fbef7cf52a2250 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 26 Apr 2022 12:21:14 -0700 Subject: [PATCH 1/4] Set execution options consistently --- .../Handlers/ConfigurationDoneHandler.cs | 4 ++ .../PowerShell/Console/PsrlReadLine.cs | 6 ++- .../Execution/BlockingConcurrentDeque.cs | 2 +- .../PowerShell/Host/PsesInternalHost.cs | 41 ++++++++++--------- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs index 74595831e..ed683eaaa 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs @@ -27,6 +27,10 @@ internal class ConfigurationDoneHandler : IConfigurationDoneHandler // instead hide from the user with pretty strings (or perhaps not write out at all). private static readonly PowerShellExecutionOptions s_debuggerExecutionOptions = new() { + // NOTE: We want to interrupt the current foreground task because otherwise we won't run + // this until the idle handler processes it, but we also don't want it to run under the + // idle handler, so both of these are true. + InterruptCurrentForeground = true, MustRunInForeground = true, WriteInputToHost = true, WriteOutputToHost = true, diff --git a/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs b/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs index 64e3627fa..ec3823429 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Console/PsrlReadLine.cs @@ -32,7 +32,11 @@ public PsrlReadLine( _psrlProxy.OverrideIdleHandler(onIdleAction); } - public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate(representation: "ReadLine", new ExecutionOptions { MustRunInForeground = true }, InvokePSReadLine, cancellationToken); + public override string ReadLine(CancellationToken cancellationToken) => _psesHost.InvokeDelegate( + representation: "ReadLine", + new ExecutionOptions { MustRunInForeground = true, InterruptCurrentForeground = true, }, + InvokePSReadLine, + cancellationToken); protected override ConsoleKeyInfo ReadKey(CancellationToken cancellationToken) => _psesHost.ReadKey(intercept: true, cancellationToken); diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs index 035437dd3..2e5b4f733 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/BlockingConcurrentDeque.cs @@ -65,7 +65,7 @@ public bool TryTake(out T item) public IDisposable BlockConsumers() => PriorityQueueBlockLifetime.StartBlocking(_blockConsumersEvent); - public void Dispose() => ((IDisposable)_blockConsumersEvent).Dispose(); + public void Dispose() => _blockConsumersEvent.Dispose(); private class PriorityQueueBlockLifetime : IDisposable { diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 8697f45ab..99d09c6f4 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -949,6 +949,7 @@ private Runspace CreateInitialRunspace(InitialSessionState initialSessionState) return runspace; } + // NOTE: This token is received from PSReadLine, and it _is_ the ReadKey cancellation token! private void OnPowerShellIdle(CancellationToken idleCancellationToken) { IReadOnlyList eventSubscribers = _mainRunspaceEngineIntrinsics.Events.Subscribers; @@ -1102,27 +1103,27 @@ private void OnDebuggerStopped(object sender, DebuggerStopEventArgs debuggerStop void OnDebuggerStoppedImpl(object sender, DebuggerStopEventArgs debuggerStopEventArgs) { - // If the debug server is NOT active, we need to synchronize state and start it. - if (!DebugContext.IsDebugServerActive) - { - _languageServer?.SendNotification("powerShell/startDebugger"); - } + // If the debug server is NOT active, we need to synchronize state and start it. + if (!DebugContext.IsDebugServerActive) + { + _languageServer?.SendNotification("powerShell/startDebugger"); + } - DebugContext.SetDebuggerStopped(debuggerStopEventArgs); + DebugContext.SetDebuggerStopped(debuggerStopEventArgs); - try - { - CurrentPowerShell.WaitForRemoteOutputIfNeeded(); - PowerShellFrameType frameBase = CurrentFrame.FrameType & PowerShellFrameType.Remote; - PushPowerShellAndRunLoop( - CreateNestedPowerShell(CurrentRunspace), - frameBase | PowerShellFrameType.Debug | PowerShellFrameType.Nested | PowerShellFrameType.Repl); - CurrentPowerShell.ResumeRemoteOutputIfNeeded(); - } - finally - { - DebugContext.SetDebuggerResumed(); - } + try + { + CurrentPowerShell.WaitForRemoteOutputIfNeeded(); + PowerShellFrameType frameBase = CurrentFrame.FrameType & PowerShellFrameType.Remote; + PushPowerShellAndRunLoop( + CreateNestedPowerShell(CurrentRunspace), + frameBase | PowerShellFrameType.Debug | PowerShellFrameType.Nested | PowerShellFrameType.Repl); + CurrentPowerShell.ResumeRemoteOutputIfNeeded(); + } + finally + { + DebugContext.SetDebuggerResumed(); + } } } From cd52c3a9aafdfc4eee80a2c28c028b6e613aeab8 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 26 Apr 2022 12:21:35 -0700 Subject: [PATCH 2/4] Don't cancel on disposal of `CancellationScope` --- .../Services/PowerShell/Utility/CancellationContext.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Utility/CancellationContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Utility/CancellationContext.cs index daee301ba..10dba79a3 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Utility/CancellationContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Utility/CancellationContext.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -109,8 +109,10 @@ internal CancellationScope( public void Dispose() { + // TODO: This is whack. It used to call `Cancel` on the cancellation source, but we + // shouldn't do that! + _cancellationSource.Dispose(); _cancellationStack.TryPop(out CancellationScope _); - _cancellationSource.Cancel(); } } } From c1906ce90af2f7f97d0a6c816ac69bfe60d88ace Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 26 Apr 2022 12:23:26 -0700 Subject: [PATCH 3/4] Add sleep to REPL loop to temporarily workaround race condition --- .../Services/PowerShell/Host/PsesInternalHost.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 99d09c6f4..8eb1723bc 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -683,6 +683,9 @@ private void RunExecutionLoop(bool isForDebug = false) try { + // TODO: This works around a race condition around executing our interactive + // tasks, and is a HIGH PRIORITY to fix. + Thread.Sleep(200); DoOneRepl(cancellationScope.CancellationToken); } catch (OperationCanceledException) From c349bafc12818a10a74cd7b9f85e2985d23ed4cb Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Tue, 26 Apr 2022 17:28:47 -0400 Subject: [PATCH 4/4] Fix broken prompt (PSIC) on F5 --- .../Services/PowerShell/Host/PsesInternalHost.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 8eb1723bc..558043f32 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System; @@ -739,6 +739,11 @@ private void DoOneRepl(CancellationToken cancellationToken) StopDebugContext(); } + if (cancellationToken.IsCancellationRequested) + { + return; + } + // When a task must run in the foreground, we cancel out of the idle loop and return to the top level. // At that point, we would normally run a REPL, but we need to immediately execute the task. // So we set _skipNextPrompt to do that.