Merge pull request #1685 from PowerShell/andschwa/stopDebugger

Synchronize PowerShell debugger and DAP server state

Author: andyleejordan

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
 {
@@ -16,44 +15,57 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging
     /// </summary>
     /// <remarks>
     /// <para>
-    /// Debugging through a PowerShell Host is implemented by registering a handler
-    /// for the <see cref="System.Management.Automation.Debugger.DebuggerStop"/> 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 <see cref="System.Management.Automation.DebuggerStopEventArgs"/> object
-    /// and the debugger stop lasts for the duration of the handler call.
-    /// The handler sets the <see cref="System.Management.Automation.DebuggerStopEventArgs.ResumeAction"/> 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 <see
+    /// cref="Debugger.DebuggerStop"/> 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 <see cref="DebuggerStopEventArgs"/> object and the
+    /// debugger stop lasts for the duration of the handler call. The handler sets the <see
+    /// cref="DebuggerStopEventArgs.ResumeAction"/> property when after it returns, the PowerShell
+    /// debugger uses that as the direction on how to proceed.
     /// </para>
     /// <para>
-    /// When we handle the <see cref="System.Management.Automation.Debugger.DebuggerStop"/> event,
-    /// we drop into a nested debug prompt and execute things in the debugger with <see cref="System.Management.Automation.Debugger.ProcessCommand(PSCommand, PSDataCollection{PSObject})"/>,
-    /// which enables debugger commands like <c>l</c>, <c>c</c>, <c>s</c>, etc.
-    /// <see cref="Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging.PowerShellDebugContext"/> saves the event args object in its state,
-    /// and when one of the debugger commands is used, the result returned is used to set <see cref="System.Management.Automation.DebuggerStopEventArgs.ResumeAction"/>
-    /// on the saved event args object so that when the event handler returns, the PowerShell debugger takes the correct action.
+    /// When we handle the <see cref="Debugger.DebuggerStop"/> event, we drop into a nested debug
+    /// prompt and execute things in the debugger with <see cref="Debugger.ProcessCommand(PSCommand,
+    /// PSDataCollection{PSObject})"/>, which enables debugger commands like <c>l</c>, <c>c</c>,
+    /// <c>s</c>, etc. <see cref="PowerShellDebugContext"/> saves the event args object in its
+    /// state, and when one of the debugger commands is used, the result returned is used to set
+    /// <see cref="DebuggerStopEventArgs.ResumeAction"/> on the saved event args object so that when
+    /// the event handler returns, the PowerShell debugger takes the correct action.
     /// </para>
     /// </remarks>
     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<PowerShellDebugContext>();
-            _languageServer = languageServer;
             _psesHost = psesHost;
         }
 
+        /// <summary>
+        /// Tracks if the debugger is currently stopped at a breakpoint.
+        /// </summary>
         public bool IsStopped { get; private set; }
 
+        /// <summary>
+        /// Tracks the state of the PowerShell debugger. This is NOT the same as <see
+        /// cref="Debugger.IsActive">, which is true whenever breakpoints are set. Instead, this is
+        /// set to true when the first <see cref="PsesInternalHost.OnDebuggerStopped"> event is
+        /// fired, and set to false in <see cref="PsesInternalHost.DoOneRepl"> when <see
+        /// cref="Debugger.IsInBreakpoint"> 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.
+        /// </summary>
+        public bool IsActive { get; set; }
+
+        /// <summary>
+        /// Tracks the state of the LSP debug server (not the PowerShell debugger).
+        /// </summary>
         public bool IsDebugServerActive { get; set; }
 
         public DebuggerStopEventArgs LastStopEventArgs { get; private set; }
@@ -67,42 +79,24 @@ public Task<DscBreakpointCapability> 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)
         {
@@ -127,27 +121,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)
         {
@@ -158,26 +144,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()
-        {
-            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");
-            }
+        private void RaiseDebuggerStoppedEvent() => DebuggerStopped?.Invoke(this, LastStopEventArgs);
 
-            DebuggerStopped?.Invoke(this, LastStopEventArgs);
-        }
-
-        private void RaiseDebuggerResumingEvent(DebuggerResumingEventArgs debuggerResumingEventArgs)
-        {
-            DebuggerResuming?.Invoke(this, debuggerResumingEventArgs);
-        }
+        private void RaiseDebuggerResumingEvent(DebuggerResumingEventArgs args) => DebuggerResuming?.Invoke(this, args);
     }
 }

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;
@@ -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();
@@ -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);
                 }
             }
         }
@@ -605,6 +601,16 @@ private void DoOneRepl(CancellationToken cancellationToken)
                 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
+            // 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)
+            {
+                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.
@@ -629,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();
                     }
@@ -742,7 +747,7 @@ private static PowerShell CreatePowerShellForRunspace(Runspace runspace)
                 pwsh.SetCorrectExecutionPolicy(_logger);
             }
 
-            pwsh.ImportModule(s_commandsModulePath);
+            pwsh.ImportModule(CommandsModulePath);
 
             if (hostStartupInfo.AdditionalModules?.Count > 0)
             {
@@ -830,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)
@@ -850,9 +860,31 @@ 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)
+            {
+                _languageServer?.SendNotification("powerShell/startDebugger");
+            }
+
             DebugContext.SetDebuggerStopped(debuggerStopEventArgs);
+
             try
             {
                 CurrentPowerShell.WaitForRemoteOutputIfNeeded();
@@ -875,7 +907,7 @@ private void OnRunspaceStateChanged(object sender, RunspaceStateEventArgs runspa
             if (!ShouldExitExecutionLoop && !_resettingRunspace && !runspaceStateEventArgs.RunspaceStateInfo.IsUsable())
             {
                 _resettingRunspace = true;
-                PopOrReinitializeRunspaceAsync().HandleErrorsAsync(_logger);
+                Task _ = PopOrReinitializeRunspaceAsync().HandleErrorsAsync(_logger);
             }
         }
 
@@ -889,7 +921,7 @@ private Task PopOrReinitializeRunspaceAsync()
             return ExecuteDelegateAsync(
                 nameof(PopOrReinitializeRunspaceAsync),
                 new ExecutionOptions { InterruptCurrentForeground = true },
-                (cancellationToken) =>
+                (_) =>
                 {
                     while (_psFrameStack.Count > 0
                         && !_psFrameStack.Peek().PowerShell.Runspace.RunspaceStateInfo.IsUsable())