From 87b3c9ce6006ccef72f9e030f7e95cdb092aeef1 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 27 Apr 2022 12:22:38 -0700 Subject: [PATCH 1/2] Fix extra prompting and manual debugger commands This requires that we set the REPL to exit correctly. Note that we don't have a notion of "disconnect" so we removed that logic. We are assuming the debugger is "active" a little earlier now, that is, also when we've received an LSP notification to start it (via a launch or attach handler). Because we receive a notification on "disconnect" even for a script just finishing, we need to know that the server is not active and so not cancel the current prompt. --- .../Handlers/LaunchAndAttachHandler.cs | 19 +++++--- .../Debugging/PowerShellDebugContext.cs | 47 +++++++++++-------- .../PowerShell/Host/PsesInternalHost.cs | 22 +++++---- 3 files changed, 54 insertions(+), 34 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs index 2849fee4d..8e5e758f2 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs @@ -11,15 +11,16 @@ using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Logging; using Microsoft.PowerShell.EditorServices.Services; -using OmniSharp.Extensions.DebugAdapter.Protocol.Events; -using OmniSharp.Extensions.JsonRpc; -using Microsoft.PowerShell.EditorServices.Services.TextDocument; -using OmniSharp.Extensions.DebugAdapter.Protocol.Requests; -using OmniSharp.Extensions.DebugAdapter.Protocol.Server; using Microsoft.PowerShell.EditorServices.Services.PowerShell; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Context; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace; +using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.DebugAdapter.Protocol.Events; +using OmniSharp.Extensions.DebugAdapter.Protocol.Requests; +using OmniSharp.Extensions.DebugAdapter.Protocol.Server; +using OmniSharp.Extensions.JsonRpc; namespace Microsoft.PowerShell.EditorServices.Handlers { @@ -122,6 +123,9 @@ public LaunchAndAttachHandler( public async Task Handle(PsesLaunchRequestArguments request, CancellationToken cancellationToken) { + // The debugger has officially started. We use this to later check if we should stop it. + ((PsesInternalHost)_executionService).DebugContext.IsActive = true; + _debugEventHandlerService.RegisterEventHandlers(); // Determine whether or not the working directory should be set in the PowerShellContext. @@ -213,6 +217,9 @@ public async Task Handle(PsesLaunchRequestArguments request, Can public async Task Handle(PsesAttachRequestArguments request, CancellationToken cancellationToken) { + // The debugger has officially started. We use this to later check if we should stop it. + ((PsesInternalHost)_executionService).DebugContext.IsActive = true; + _debugStateService.IsAttachSession = true; _debugEventHandlerService.RegisterEventHandlers(); diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs index 878caf18d..a66b0d93b 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs @@ -94,7 +94,7 @@ public void EnableDebugMode() _psesHost.Runspace.Debugger.SetDebugMode(DebugModes.LocalScript | DebugModes.RemoteScript); } - public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop, isDisconnect: true); + public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop); public void BreakExecution() => _psesHost.Runspace.Debugger.SetDebuggerStepMode(enabled: true); @@ -106,10 +106,12 @@ public void EnableDebugMode() public void StepOver() => SetDebugResuming(DebuggerResumeAction.StepOver); - public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction, bool isDisconnect = false) + public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction) { - // NOTE: We exit because the paused/stopped debugger is currently in a prompt REPL, and - // to resume the debugger we must exit that REPL. + // We exit because the paused/stopped debugger is currently in a prompt REPL, and to + // resume the debugger we must exit that REPL. If we're continued from 'c' or 's', this + // is already set and so is a no-op; but if the user clicks the continue or step button, + // then this came over LSP and we need to set it. _psesHost.SetExit(); if (LastStopEventArgs is not null) @@ -127,23 +129,31 @@ public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction, bool isD return; } - if (debuggerResumeAction is DebuggerResumeAction.Stop) + // If we're stopping (or disconnecting, which is the same thing in LSP-land), then we + // want to cancel any debug prompts, remote prompts, debugged scripts, etc. However, if + // the debugged script has exited normally (or was quit with 'q'), we still get an LSP + // notification that eventually lands here with a stop event. In this case, the debug + // context is NOT active and we do not want to cancel the regular REPL. + if (!_psesHost.DebugContext.IsActive) { - // If we're disconnecting we want to unwind all the way back to the default, local - // state. So we use UnwindCallStack here to ensure every context frame is cancelled. - if (isDisconnect) - { - _psesHost.UnwindCallStack(); - return; - } + return; + } - _psesHost.CancelIdleParentTask(); + // If the debugger is active and we're stopping, we need to unwind everything. + if (debuggerResumeAction is DebuggerResumeAction.Stop) + { + // TODO: We need to assign cancellation tokens to each frame, because the current + // logic results in a deadlock here when we try to cancel the scopes...which + // includes ourself (hence running it in a separate thread). + Task.Run(() => _psesHost.UnwindCallStack()); return; } + // Otherwise we're continuing or stepping (i.e. resuming) so we need to cancel the + // debugger REPL. if (_psesHost.CurrentFrame.IsRepl) { - _psesHost.CancelCurrentTask(); + _psesHost.CancelIdleParentTask(); } } @@ -166,15 +176,14 @@ public void ProcessDebuggerResult(DebuggerCommandResults debuggerResult) { if (debuggerResult?.ResumeAction is not null) { - SetDebugResuming(debuggerResult.ResumeAction.Value); - - // If a debugging command like `c` is specified in a nested remote - // debugging prompt we need to unwind the nested execution loop. - if (_psesHost.CurrentFrame.IsRemote) + // Since we're processing a command like 'c' or 's' remotely, we need to tell the + // host to stop the remote REPL loop. + if (debuggerResult.ResumeAction is not DebuggerResumeAction.Stop || _psesHost.CurrentFrame.IsRemote) { _psesHost.ForceSetExit(); } + SetDebugResuming(debuggerResult.ResumeAction.Value); RaiseDebuggerResumingEvent(new DebuggerResumingEventArgs(debuggerResult.ResumeAction.Value)); // The Terminate exception is used by the engine for flow control diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 8697f45ab..d47990351 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -724,6 +724,19 @@ private void DoOneRepl(CancellationToken cancellationToken) return; } + // TODO: We must remove this awful logic, it causes so much pain. The StopDebugContext() + // requires that we're not in a prompt that we're skipping, otherwise the debugger is + // "active" but we haven't yet hit a breakpoint. + // + // 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. + if (_skipNextPrompt) + { + _skipNextPrompt = false; + return; + } + // We use the REPL as a poll to check if the debug context is active but PowerShell // indicates we're no longer debugging. This happens when PowerShell was used to start // the debugger (instead of using a Code launch configuration) via Wait-Debugger or @@ -736,15 +749,6 @@ private void DoOneRepl(CancellationToken cancellationToken) StopDebugContext(); } - // 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. - if (_skipNextPrompt) - { - _skipNextPrompt = false; - return; - } - try { string prompt = GetPrompt(cancellationToken); From 3cba30c95c5d0667bd485990bc1244ceefb728f7 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 26 Apr 2022 12:25:19 -0700 Subject: [PATCH 2/2] Remove `WriteWithPrompt` workaround --- .../Execution/SynchronousPowerShellTask.cs | 2 +- .../PowerShell/Host/PsesInternalHost.cs | 39 +++++++------------ 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs index 71accf212..52ea70efe 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousPowerShellTask.cs @@ -62,7 +62,7 @@ public override IReadOnlyList Run(CancellationToken cancellationToken) if (PowerShellExecutionOptions.WriteInputToHost) { - _psesHost.WriteWithPrompt(_psCommand, cancellationToken); + _psesHost.UI.WriteLine(_psCommand.GetInvocationText()); } return _pwsh.Runspace.Debugger.InBreakpoint diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index d47990351..1140f5e65 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -757,18 +757,22 @@ private void DoOneRepl(CancellationToken cancellationToken) // If the user input was empty it's because: // - the user provided no input - // - the readline task was canceled - // - CtrlC was sent to readline (which does not propagate a cancellation) + // - the ReadLine task was canceled + // - CtrlC was sent to ReadLine (which does not propagate a cancellation) // - // In any event there's nothing to run in PowerShell, so we just loop back to the prompt again. - // However, we must distinguish the last two scenarios, since PSRL will not print a new line in those cases. + // In any event there's nothing to run in PowerShell, so we just loop back to the + // prompt again. However, PSReadLine will not print a newline for CtrlC, so we print + // one, but we do not want to print one if the ReadLine task was canceled. if (string.IsNullOrEmpty(userInput)) { - if (cancellationToken.IsCancellationRequested || LastKeyWasCtrlC()) + if (LastKeyWasCtrlC()) { UI.WriteLine(); } - return; + // Propogate cancellation if that's what happened, since ReadLine won't. + // TODO: We may not need to do this at all. + cancellationToken.ThrowIfCancellationRequested(); + return; // Task wasn't canceled but there was no input. } InvokeInput(userInput, cancellationToken); @@ -782,10 +786,8 @@ private void DoOneRepl(CancellationToken cancellationToken) { throw; } - catch (FlowControlException) - { - // Do nothing, a break or continue statement was used outside of a loop. - } + // Do nothing, a break or continue statement was used outside of a loop. + catch (FlowControlException) { } catch (Exception e) { UI.WriteErrorLine($"An error occurred while running the REPL loop:{Environment.NewLine}{e}"); @@ -829,25 +831,14 @@ private string GetPrompt(CancellationToken cancellationToken) return prompt; } - /// - /// This is used to write the invocation text of a command with the user's prompt so that, - /// for example, F8 (evaluate selection) appears as if the user typed it. Used when - /// 'WriteInputToHost' is true. - /// - /// The PSCommand we'll print after the prompt. - /// - public void WriteWithPrompt(PSCommand command, CancellationToken cancellationToken) - { - UI.Write(GetPrompt(cancellationToken)); - UI.WriteLine(command.GetInvocationText()); - } - private string InvokeReadLine(CancellationToken cancellationToken) { - cancellationToken.ThrowIfCancellationRequested(); try { + // TODO: If we can pass the cancellation token to ReadKey directly in PSReadLine, we + // can remove this logic. _readKeyCancellationToken = cancellationToken; + cancellationToken.ThrowIfCancellationRequested(); return _readLineProvider.ReadLine.ReadLine(cancellationToken); } finally