From 70d100b32e08e5587808da45ceda1670c737d8f1 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 26 Jan 2022 10:16:00 -0800 Subject: [PATCH 1/5] Move `startDebugger` notification to `PsesInternalHost.OnDebuggerStopped` --- .../Debugging/PowerShellDebugContext.cs | 24 +++++++++---------- .../PowerShell/Host/PsesInternalHost.cs | 13 ++++++++-- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs index 74e088d47..bae9e61ed 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs @@ -4,10 +4,9 @@ using System; using System.Management.Automation; using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; -using OmniSharp.Extensions.LanguageServer.Protocol.Server; -using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging { @@ -38,24 +37,30 @@ internal class PowerShellDebugContext : IPowerShellDebugContext { private readonly ILogger _logger; - private readonly ILanguageServerFacade _languageServer; - private readonly PsesInternalHost _psesHost; public PowerShellDebugContext( ILoggerFactory loggerFactory, - ILanguageServerFacade languageServer, PsesInternalHost psesHost) { _logger = loggerFactory.CreateLogger(); - _languageServer = languageServer; _psesHost = psesHost; } public bool IsStopped { get; private set; } + /// + /// Tracks the state of the LSP debug server (not the PowerShell debugger). + /// public bool IsDebugServerActive { get; set; } + /// + /// Tracks if the PowerShell session started the debug server itself (true), or if it was + /// started by an LSP notification (false). Essentially, this marks if we're responsible for + /// stopping the debug server (and thus need to send a notification to do so). + /// + public bool OwnsDebugServerState { get; set; } + public DebuggerStopEventArgs LastStopEventArgs { get; private set; } public event Action DebuggerStopped; @@ -165,13 +170,6 @@ public void HandleBreakpointUpdated(BreakpointUpdatedEventArgs breakpointUpdated private void RaiseDebuggerStoppedEvent() { - if (!IsDebugServerActive) - { - // NOTE: The language server is not necessarily connected, so this must be - // conditional access. This shows up in unit tests. - _languageServer?.SendNotification("powerShell/startDebugger"); - } - DebuggerStopped?.Invoke(this, LastStopEventArgs); } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index a1ccee7b5..99a2e4a1e 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; @@ -112,7 +112,7 @@ public PsesInternalHost( Name = hostInfo.Name; Version = hostInfo.Version; - DebugContext = new PowerShellDebugContext(loggerFactory, languageServer, this); + DebugContext = new PowerShellDebugContext(loggerFactory, this); UI = hostInfo.ConsoleReplEnabled ? new EditorServicesConsolePSHostUserInterface(loggerFactory, _readLineProvider, hostInfo.PSHost.UI) : new NullPSHostUI(); @@ -852,7 +852,16 @@ private bool LastKeyWasCtrlC() private void OnDebuggerStopped(object sender, DebuggerStopEventArgs debuggerStopEventArgs) { + if (!DebugContext.IsDebugServerActive) + { + // If the we've hit a breakpoint and the debug server is not active, then we need to + // start it (and own stopping it later). + DebugContext.OwnsDebugServerState = true; + _languageServer?.SendNotification("powerShell/startDebugger"); + } + DebugContext.SetDebuggerStopped(debuggerStopEventArgs); + try { CurrentPowerShell.WaitForRemoteOutputIfNeeded(); From 8df688c07275c52478883c7367890c31634585fa Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 26 Jan 2022 10:16:21 -0800 Subject: [PATCH 2/5] Clean up `PowerShellDebugContext.cs` --- .../Debugging/PowerShellDebugContext.cs | 95 ++++++------------- 1 file changed, 30 insertions(+), 65 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs index bae9e61ed..aa2037035 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs @@ -15,22 +15,22 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging /// /// /// - /// Debugging through a PowerShell Host is implemented by registering a handler - /// for the event. - /// Registering that handler causes debug actions in PowerShell like Set-PSBreakpoint - /// and Wait-Debugger to drop into the debugger and trigger the handler. - /// The handler is passed a mutable object - /// and the debugger stop lasts for the duration of the handler call. - /// The handler sets the property - /// when after it returns, the PowerShell debugger uses that as the direction on how to proceed. + /// Debugging through a PowerShell Host is implemented by registering a handler for the event. Registering that handler causes debug actions in + /// PowerShell like Set-PSBreakpoint and Wait-Debugger to drop into the debugger and trigger the + /// handler. The handler is passed a mutable object and the + /// debugger stop lasts for the duration of the handler call. The handler sets the property when after it returns, the PowerShell + /// debugger uses that as the direction on how to proceed. /// /// - /// When we handle the event, - /// we drop into a nested debug prompt and execute things in the debugger with , - /// which enables debugger commands like l, c, s, etc. - /// saves the event args object in its state, - /// and when one of the debugger commands is used, the result returned is used to set - /// on the saved event args object so that when the event handler returns, the PowerShell debugger takes the correct action. + /// When we handle the event, we drop into a nested debug + /// prompt and execute things in the debugger with , which enables debugger commands like l, c, + /// s, etc. saves the event args object in its + /// state, and when one of the debugger commands is used, the result returned is used to set + /// on the saved event args object so that when + /// the event handler returns, the PowerShell debugger takes the correct action. /// /// internal class PowerShellDebugContext : IPowerShellDebugContext @@ -72,42 +72,24 @@ public Task GetDscBreakpointCapabilityAsync(Cancellatio return _psesHost.CurrentRunspace.GetDscBreakpointCapabilityAsync(_logger, _psesHost, cancellationToken); } + // This is required by the PowerShell API so that remote debugging works. Without it, a + // runspace may not have these options set and attempting to set breakpoints remotely fails. public void EnableDebugMode() { - // This is required by the PowerShell API so that remote debugging works. - // Without it, a runspace may not have these options set and attempting to set breakpoints remotely can fail. _psesHost.Runspace.Debugger.SetDebugMode(DebugModes.LocalScript | DebugModes.RemoteScript); } - public void Abort() - { - SetDebugResuming(DebuggerResumeAction.Stop); - } + public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop); - public void BreakExecution() - { - _psesHost.Runspace.Debugger.SetDebuggerStepMode(enabled: true); - } + public void BreakExecution() => _psesHost.Runspace.Debugger.SetDebuggerStepMode(enabled: true); - public void Continue() - { - SetDebugResuming(DebuggerResumeAction.Continue); - } + public void Continue() => SetDebugResuming(DebuggerResumeAction.Continue); - public void StepInto() - { - SetDebugResuming(DebuggerResumeAction.StepInto); - } + public void StepInto() => SetDebugResuming(DebuggerResumeAction.StepInto); - public void StepOut() - { - SetDebugResuming(DebuggerResumeAction.StepOut); - } + public void StepOut() => SetDebugResuming(DebuggerResumeAction.StepOut); - public void StepOver() - { - SetDebugResuming(DebuggerResumeAction.StepOver); - } + public void StepOver() => SetDebugResuming(DebuggerResumeAction.StepOver); public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction) { @@ -132,27 +114,19 @@ public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction) } // This must be called AFTER the new PowerShell has been pushed - public void EnterDebugLoop() - { - RaiseDebuggerStoppedEvent(); - } + public void EnterDebugLoop() => RaiseDebuggerStoppedEvent(); // This must be called BEFORE the debug PowerShell has been popped [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "This method may acquire an implementation later, at which point it will need instance data")] - public void ExitDebugLoop() - { - } + public void ExitDebugLoop() { } - public void SetDebuggerStopped(DebuggerStopEventArgs debuggerStopEventArgs) + public void SetDebuggerStopped(DebuggerStopEventArgs args) { IsStopped = true; - LastStopEventArgs = debuggerStopEventArgs; + LastStopEventArgs = args; } - public void SetDebuggerResumed() - { - IsStopped = false; - } + public void SetDebuggerResumed() { IsStopped = false; } public void ProcessDebuggerResult(DebuggerCommandResults debuggerResult) { @@ -163,19 +137,10 @@ public void ProcessDebuggerResult(DebuggerCommandResults debuggerResult) } } - public void HandleBreakpointUpdated(BreakpointUpdatedEventArgs breakpointUpdatedEventArgs) - { - BreakpointUpdated?.Invoke(this, breakpointUpdatedEventArgs); - } + public void HandleBreakpointUpdated(BreakpointUpdatedEventArgs args) => BreakpointUpdated?.Invoke(this, args); - private void RaiseDebuggerStoppedEvent() - { - DebuggerStopped?.Invoke(this, LastStopEventArgs); - } + private void RaiseDebuggerStoppedEvent() => DebuggerStopped?.Invoke(this, LastStopEventArgs); - private void RaiseDebuggerResumingEvent(DebuggerResumingEventArgs debuggerResumingEventArgs) - { - DebuggerResuming?.Invoke(this, debuggerResumingEventArgs); - } + private void RaiseDebuggerResumingEvent(DebuggerResumingEventArgs args) => DebuggerResuming?.Invoke(this, args); } } From 20aaa02ec6edea79a0c5d9af8739f14e9f7216ea Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 26 Jan 2022 10:18:39 -0800 Subject: [PATCH 3/5] Clean up `PsesInternalHost` --- .../PowerShell/Host/PsesInternalHost.cs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 99a2e4a1e..3646153b1 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -32,7 +32,7 @@ internal class PsesInternalHost : PSHost, IHostSupportsInteractiveSession, IRuns private static string s_bundledModulePath = Path.GetFullPath(Path.Combine( Path.GetDirectoryName(typeof(PsesInternalHost).Assembly.Location), "..", "..", "..")); - private static string s_commandsModulePath => Path.GetFullPath(Path.Combine( + private static string CommandsModulePath => Path.GetFullPath(Path.Combine( s_bundledModulePath, "PowerShellEditorServices", "Commands", "PowerShellEditorServices.Commands.psd1")); private readonly ILoggerFactory _loggerFactory; @@ -513,8 +513,7 @@ private void PopPowerShell(RunspaceChangeAction runspaceChangeAction = RunspaceC { // If we're changing runspace, make sure we move the handlers over. If we just // popped the last frame, then we're exiting and should pop the runspace too. - if (_psFrameStack.Count == 0 - || _runspaceStack.Peek().Runspace != _psFrameStack.Peek().PowerShell.Runspace) + if (_psFrameStack.Count == 0 || CurrentRunspace.Runspace != CurrentPowerShell.Runspace) { RunspaceFrame previousRunspaceFrame = _runspaceStack.Pop(); RemoveRunspaceEventHandlers(previousRunspaceFrame.Runspace); @@ -566,7 +565,6 @@ private void RunTopLevelExecutionLoop() _stopped.SetResult(true); } - private void RunDebugExecutionLoop() { try @@ -584,16 +582,14 @@ private void RunExecutionLoop() { while (!ShouldExitExecutionLoop) { - using (CancellationScope cancellationScope = _cancellationContext.EnterScope(isIdleScope: false)) - { - DoOneRepl(cancellationScope.CancellationToken); + using CancellationScope cancellationScope = _cancellationContext.EnterScope(isIdleScope: false); + DoOneRepl(cancellationScope.CancellationToken); - while (!ShouldExitExecutionLoop - && !cancellationScope.CancellationToken.IsCancellationRequested - && _taskQueue.TryTake(out ISynchronousTask task)) - { - task.ExecuteSynchronously(cancellationScope.CancellationToken); - } + while (!ShouldExitExecutionLoop + && !cancellationScope.CancellationToken.IsCancellationRequested + && _taskQueue.TryTake(out ISynchronousTask task)) + { + task.ExecuteSynchronously(cancellationScope.CancellationToken); } } } @@ -742,7 +738,7 @@ private static PowerShell CreatePowerShellForRunspace(Runspace runspace) pwsh.SetCorrectExecutionPolicy(_logger); } - pwsh.ImportModule(s_commandsModulePath); + pwsh.ImportModule(CommandsModulePath); if (hostStartupInfo.AdditionalModules?.Count > 0) { @@ -884,7 +880,7 @@ private void OnRunspaceStateChanged(object sender, RunspaceStateEventArgs runspa if (!ShouldExitExecutionLoop && !_resettingRunspace && !runspaceStateEventArgs.RunspaceStateInfo.IsUsable()) { _resettingRunspace = true; - PopOrReinitializeRunspaceAsync().HandleErrorsAsync(_logger); + Task _ = PopOrReinitializeRunspaceAsync().HandleErrorsAsync(_logger); } } @@ -898,7 +894,7 @@ private Task PopOrReinitializeRunspaceAsync() return ExecuteDelegateAsync( nameof(PopOrReinitializeRunspaceAsync), new ExecutionOptions { InterruptCurrentForeground = true }, - (cancellationToken) => + (_) => { while (_psFrameStack.Count > 0 && !_psFrameStack.Peek().PowerShell.Runspace.RunspaceStateInfo.IsUsable()) From d52caec108798ee7b7dbdc6aaf31b621cfd9643a Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 26 Jan 2022 11:59:54 -0800 Subject: [PATCH 4/5] Send `stopDebugger` during REPL if no longer debugging --- .../Services/PowerShell/Host/PsesInternalHost.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 3646153b1..2e9299a88 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; @@ -601,6 +601,14 @@ private void DoOneRepl(CancellationToken cancellationToken) return; } + // If we started the debug server, then on each REPL we need to check if we're still + // actively debugging, and if not, stop the server. + if (DebugContext.OwnsDebugServerState && !CurrentRunspace.Runspace.Debugger.InBreakpoint) + { + DebugContext.OwnsDebugServerState = false; + _languageServer?.SendNotification("powerShell/stopDebugger"); + } + // 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. From 0f52aa2fe126add8f44852a21e3d3ee454bd8f18 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 26 Jan 2022 16:28:47 -0800 Subject: [PATCH 5/5] Stop debugger on task cancellation too This synchronizes the `DebugContext` and LSP debug server state during both edge cases: 1. A REPL happened, the debugger was marked active, and the debugger is now no longer running (it was quit, detached, or continued to the end of the script or function). 2. A user manually cancelled a task, and it was running under the "debugger" (whether or not the PowerShell debugger was active). --- .../Debugging/PowerShellDebugContext.cs | 19 ++++++--- .../PowerShell/Host/PsesInternalHost.cs | 39 ++++++++++++++----- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs index aa2037035..a6c1d27e1 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs @@ -47,19 +47,26 @@ public PowerShellDebugContext( _psesHost = psesHost; } + /// + /// Tracks if the debugger is currently stopped at a breakpoint. + /// public bool IsStopped { get; private set; } /// - /// Tracks the state of the LSP debug server (not the PowerShell debugger). + /// Tracks the state of the PowerShell debugger. This is NOT the same as , which is true whenever breakpoints are set. Instead, this is + /// set to true when the first event is + /// fired, and set to false in when is false. This is used to send the + /// 'powershell/stopDebugger' notification to the LSP debug server in the cases where the + /// server was started or ended by the PowerShell session instead of by Code's GUI. /// - public bool IsDebugServerActive { get; set; } + public bool IsActive { get; set; } /// - /// Tracks if the PowerShell session started the debug server itself (true), or if it was - /// started by an LSP notification (false). Essentially, this marks if we're responsible for - /// stopping the debug server (and thus need to send a notification to do so). + /// Tracks the state of the LSP debug server (not the PowerShell debugger). /// - public bool OwnsDebugServerState { get; set; } + public bool IsDebugServerActive { get; set; } public DebuggerStopEventArgs LastStopEventArgs { get; private set; } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs index 2e9299a88..1901dabf4 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs @@ -601,12 +601,14 @@ private void DoOneRepl(CancellationToken cancellationToken) return; } - // If we started the debug server, then on each REPL we need to check if we're still - // actively debugging, and if not, stop the server. - if (DebugContext.OwnsDebugServerState && !CurrentRunspace.Runspace.Debugger.InBreakpoint) + // 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 + // simply hitting a PSBreakpoint. We need to synchronize the state and stop the debug + // context (and likely the debug server). + if (DebugContext.IsActive && !CurrentRunspace.Runspace.Debugger.InBreakpoint) { - DebugContext.OwnsDebugServerState = false; - _languageServer?.SendNotification("powerShell/stopDebugger"); + StopDebugContext(); } // When a task must run in the foreground, we cancel out of the idle loop and return to the top level. @@ -633,8 +635,7 @@ private void DoOneRepl(CancellationToken cancellationToken) // However, we must distinguish the last two scenarios, since PSRL will not print a new line in those cases. if (string.IsNullOrEmpty(userInput)) { - if (cancellationToken.IsCancellationRequested - || LastKeyWasCtrlC()) + if (cancellationToken.IsCancellationRequested || LastKeyWasCtrlC()) { UI.WriteLine(); } @@ -834,7 +835,12 @@ private void OnPowerShellIdle(CancellationToken idleCancellationToken) private void OnCancelKeyPress(object sender, ConsoleCancelEventArgs args) { + // We need to cancel the current task. _cancellationContext.CancelCurrentTask(); + + // If the current task was running under the debugger, we need to synchronize the + // cancelation with our debug context (and likely the debug server). + StopDebugContext(); } private ConsoleKeyInfo ReadKey(bool intercept) @@ -854,13 +860,26 @@ private bool LastKeyWasCtrlC() && _lastKey.Value.IsCtrlC(); } + private void StopDebugContext() + { + // We are officially stopping the debugger. + DebugContext.IsActive = false; + + // If the debug server is active, we need to synchronize state and stop it. + if (DebugContext.IsDebugServerActive) + { + _languageServer?.SendNotification("powerShell/stopDebugger"); + } + } + private void OnDebuggerStopped(object sender, DebuggerStopEventArgs debuggerStopEventArgs) { + // The debugger has officially started. We use this to later check if we should stop it. + DebugContext.IsActive = true; + + // If the debug server is NOT active, we need to synchronize state and start it. if (!DebugContext.IsDebugServerActive) { - // If the we've hit a breakpoint and the debug server is not active, then we need to - // start it (and own stopping it later). - DebugContext.OwnsDebugServerState = true; _languageServer?.SendNotification("powerShell/startDebugger"); }