From 04e942ed4e4a2494786ab58e5fe714c35776798d Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 29 Jul 2021 16:11:39 -0700 Subject: [PATCH 1/2] Set `AllThreadsStopped` appropriately --- .../Server/PsesDebugServer.cs | 1 + .../DebugAdapter/BreakpointService.cs | 3 +- .../DebugAdapter/DebugEventHandlerService.cs | 53 ++++++++----------- .../Debugging/BreakpointDetails.cs | 7 ++- .../DebugAdapter/Handlers/ThreadsHandler.cs | 2 +- 5 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/PowerShellEditorServices/Server/PsesDebugServer.cs b/src/PowerShellEditorServices/Server/PsesDebugServer.cs index c09a54f14..e4327cfbe 100644 --- a/src/PowerShellEditorServices/Server/PsesDebugServer.cs +++ b/src/PowerShellEditorServices/Server/PsesDebugServer.cs @@ -137,6 +137,7 @@ public async Task StartAsync() public void Dispose() { _powerShellContextService.IsDebugServerActive = false; + // TODO: If the debugger has stopped, should we clear the breakpoints? _debugAdapterServer.Dispose(); _inputStream.Dispose(); _outputStream.Dispose(); diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs index ab49bd21a..01de55b5a 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs @@ -135,7 +135,8 @@ public async Task> SetBreakpointsAsync(string esc IEnumerable setBreakpoints = await _powerShellContextService.ExecuteCommandAsync(psCommand).ConfigureAwait(false); configuredBreakpoints.AddRange( - setBreakpoints.Select(BreakpointDetails.Create)); + setBreakpoints.Select((breakpoint) => BreakpointDetails.Create(breakpoint)) + ); } return configuredBreakpoints; diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs index 205885889..13491bb77 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs @@ -81,6 +81,7 @@ e.OriginalEvent.Breakpoints[0] is CommandBreakpoint new StoppedEvent { ThreadId = 1, + AllThreadsStopped = true, Reason = debuggerStoppedReason }); } @@ -117,59 +118,47 @@ private void PowerShellContext_DebuggerResumed(object sender, DebuggerResumeActi _debugAdapterServer.SendNotification(EventNames.Continued, new ContinuedEvent { - AllThreadsContinued = true, - ThreadId = 1 + ThreadId = 1, + AllThreadsContinued = true }); } private void DebugService_BreakpointUpdated(object sender, BreakpointUpdatedEventArgs e) { - string reason = "changed"; - + // Don't send breakpoint update notifications when setting + // breakpoints on behalf of the client. if (_debugStateService.IsSetBreakpointInProgress) { - // Don't send breakpoint update notifications when setting - // breakpoints on behalf of the client. return; } - switch (e.UpdateType) - { - case BreakpointUpdateType.Set: - reason = "new"; - break; - - case BreakpointUpdateType.Removed: - reason = "removed"; - break; - } - - var breakpoint = new OmniSharp.Extensions.DebugAdapter.Protocol.Models.Breakpoint - { - Verified = e.UpdateType != BreakpointUpdateType.Disabled - }; - if (e.Breakpoint is LineBreakpoint) { - breakpoint = LspDebugUtils.CreateBreakpoint(BreakpointDetails.Create(e.Breakpoint)); + var breakpoint = LspDebugUtils.CreateBreakpoint( + BreakpointDetails.Create(e.Breakpoint, e.UpdateType) + ); + + string reason = (e.UpdateType) switch { + BreakpointUpdateType.Set => "new", + BreakpointUpdateType.Removed => "removed", + BreakpointUpdateType.Enabled => "changed", + BreakpointUpdateType.Disabled => "changed", + _ => "unknown" + }; + + _debugAdapterServer.SendNotification( + EventNames.Breakpoint, + new BreakpointEvent { Reason = reason, Breakpoint = breakpoint } + ); } else if (e.Breakpoint is CommandBreakpoint) { _logger.LogTrace("Function breakpoint updated event is not supported yet"); - return; } else { _logger.LogError($"Unrecognized breakpoint type {e.Breakpoint.GetType().FullName}"); - return; } - - _debugAdapterServer.SendNotification(EventNames.Breakpoint, - new BreakpointEvent - { - Reason = reason, - Breakpoint = breakpoint - }); } #endregion diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointDetails.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointDetails.cs index 43d67cf13..f1c4c7bbe 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointDetails.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointDetails.cs @@ -77,8 +77,11 @@ internal static BreakpointDetails Create( /// PowerShell Breakpoint object. /// /// The Breakpoint instance from which details will be taken. + /// The BreakpointUpdateType to determine if the breakpoint is verified. /// A new instance of the BreakpointDetails class. - internal static BreakpointDetails Create(Breakpoint breakpoint) + internal static BreakpointDetails Create( + Breakpoint breakpoint, + BreakpointUpdateType updateType = BreakpointUpdateType.Set) { Validate.IsNotNull("breakpoint", breakpoint); @@ -91,7 +94,7 @@ internal static BreakpointDetails Create(Breakpoint breakpoint) var breakpointDetails = new BreakpointDetails { Id = breakpoint.Id, - Verified = true, + Verified = updateType != BreakpointUpdateType.Disabled, Source = lineBreakpoint.Script, LineNumber = lineBreakpoint.Line, ColumnNumber = lineBreakpoint.Column, diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ThreadsHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ThreadsHandler.cs index b1c2faba3..ca6b41e25 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ThreadsHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ThreadsHandler.cs @@ -20,7 +20,7 @@ public Task Handle(ThreadsArguments request, CancellationToken { // TODO: OmniSharp supports multithreaded debugging (where // multiple threads can be debugged at once), but we don't. This - // means we always need to set AllThreadsStoppped and + // means we always need to set AllThreadsStopped and // AllThreadsContinued in our events. But if we one day support // multithreaded debugging, we'd need a way to associate // debugged runspaces with .NET threads in a consistent way. From 6a19daf5bc3e8547fbdc10f351ac2a81d76f9cec Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 29 Jul 2021 16:27:14 -0700 Subject: [PATCH 2/2] Make `ExecuteCommandAsync` cancellable This lets us cancel the debugger more reliably. --- .../DebugAdapter/DebugEventHandlerService.cs | 12 +++++----- .../Handlers/LaunchAndAttachHandler.cs | 12 ++++++---- .../Console/ConsoleReadLine.cs | 9 ++++--- .../Handlers/ExpandAliasHandler.cs | 3 ++- .../Handlers/GetCommandHandler.cs | 3 ++- .../PSHostProcessAndRunspaceHandlers.cs | 3 ++- .../Handlers/ShowHelpHandler.cs | 3 ++- .../PowerShellContextService.cs | 24 ++++++++++++------- .../Host/EditorServicesPSHostUserInterface.cs | 3 ++- .../Session/PSReadLinePromptContext.cs | 3 ++- 10 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs index 13491bb77..a83e3e87d 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugEventHandlerService.cs @@ -139,16 +139,16 @@ private void DebugService_BreakpointUpdated(object sender, BreakpointUpdatedEven ); string reason = (e.UpdateType) switch { - BreakpointUpdateType.Set => "new", - BreakpointUpdateType.Removed => "removed", - BreakpointUpdateType.Enabled => "changed", - BreakpointUpdateType.Disabled => "changed", - _ => "unknown" + BreakpointUpdateType.Set => BreakpointEventReason.New, + BreakpointUpdateType.Removed => BreakpointEventReason.Removed, + BreakpointUpdateType.Enabled => BreakpointEventReason.Changed, + BreakpointUpdateType.Disabled => BreakpointEventReason.Changed, + _ => "InvalidBreakpointUpdateTypeEnum" }; _debugAdapterServer.SendNotification( EventNames.Breakpoint, - new BreakpointEvent { Reason = reason, Breakpoint = breakpoint } + new BreakpointEvent { Breakpoint = breakpoint, Reason = reason } ); } else if (e.Breakpoint is CommandBreakpoint) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs index 51581c968..040bb4a3f 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs @@ -303,11 +303,13 @@ await _powerShellContextService.ExecuteScriptStringAsync( string debugRunspaceCmd; if (request.RunspaceName != null) { - IEnumerable ids = await _powerShellContextService.ExecuteCommandAsync(new PSCommand() - .AddCommand("Microsoft.PowerShell.Utility\\Get-Runspace") - .AddParameter("Name", request.RunspaceName) - .AddCommand("Microsoft.PowerShell.Utility\\Select-Object") - .AddParameter("ExpandProperty", "Id")).ConfigureAwait(false); + IEnumerable ids = await _powerShellContextService.ExecuteCommandAsync( + new PSCommand() + .AddCommand("Microsoft.PowerShell.Utility\\Get-Runspace") + .AddParameter("Name", request.RunspaceName) + .AddCommand("Microsoft.PowerShell.Utility\\Select-Object") + .AddParameter("ExpandProperty", "Id"), cancellationToken: cancellationToken).ConfigureAwait(false); + foreach (var id in ids) { _debugStateService.RunspaceId = id; diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Console/ConsoleReadLine.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Console/ConsoleReadLine.cs index 8e51b3133..145cf7360 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Console/ConsoleReadLine.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Console/ConsoleReadLine.cs @@ -208,9 +208,8 @@ internal async Task InvokeLegacyReadLineAsync(bool isCommandLine, Cancel command.AddParameter("CursorColumn", currentCursorIndex); command.AddParameter("Options", null); - var results = await this.powerShellContext - .ExecuteCommandAsync(command, sendOutputToHost: false, sendErrorToHost: false) - .ConfigureAwait(false); + var results = await this.powerShellContext.ExecuteCommandAsync( + command, sendOutputToHost: false, sendErrorToHost: false, cancellationToken).ConfigureAwait(false); currentCompletion = results.FirstOrDefault(); } @@ -327,8 +326,8 @@ internal async Task InvokeLegacyReadLineAsync(bool isCommandLine, Cancel PSCommand command = new PSCommand(); command.AddCommand("Get-History"); - currentHistory = await this.powerShellContext.ExecuteCommandAsync(command, sendOutputToHost: false, sendErrorToHost: false) - .ConfigureAwait(false) + currentHistory = await this.powerShellContext.ExecuteCommandAsync( + command, sendOutputToHost: false, sendErrorToHost: false, cancellationToken).ConfigureAwait(false) as Collection; if (currentHistory != null) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ExpandAliasHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ExpandAliasHandler.cs index f7c8988ea..08da235c8 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ExpandAliasHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ExpandAliasHandler.cs @@ -69,7 +69,8 @@ function __Expand-Alias { .AddStatement() .AddCommand("__Expand-Alias") .AddArgument(request.Text); - var result = await _powerShellContextService.ExecuteCommandAsync(psCommand).ConfigureAwait(false); + var result = await _powerShellContextService.ExecuteCommandAsync( + psCommand, cancellationToken: cancellationToken).ConfigureAwait(false); return new ExpandAliasResult { diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommandHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommandHandler.cs index cbc32b645..91931a3bf 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommandHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommandHandler.cs @@ -53,7 +53,8 @@ public async Task> Handle(GetCommandParams request, Cance .AddCommand("Microsoft.PowerShell.Utility\\Sort-Object") .AddParameter("Property", "Name"); - IEnumerable result = await _powerShellContextService.ExecuteCommandAsync(psCommand).ConfigureAwait(false); + IEnumerable result = await _powerShellContextService.ExecuteCommandAsync( + psCommand, cancellationToken: cancellationToken).ConfigureAwait(false); var commandList = new List(); if (result != null) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/PSHostProcessAndRunspaceHandlers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/PSHostProcessAndRunspaceHandlers.cs index b6b7a375d..77a47f845 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/PSHostProcessAndRunspaceHandlers.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/PSHostProcessAndRunspaceHandlers.cs @@ -87,7 +87,8 @@ public async Task Handle(GetRunspaceParams request, Cancella var psCommand = new PSCommand().AddCommand("Microsoft.PowerShell.Utility\\Get-Runspace"); var sb = new StringBuilder(); // returns (not deserialized) Runspaces. For simpler code, we use PSObject and rely on dynamic later. - runspaces = await _powerShellContextService.ExecuteCommandAsync(psCommand, sb).ConfigureAwait(false); + runspaces = await _powerShellContextService.ExecuteCommandAsync( + psCommand, sb, cancellationToken: cancellationToken).ConfigureAwait(false); } var runspaceResponses = new List(); diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ShowHelpHandler.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ShowHelpHandler.cs index 09efe1952..76a69c1a8 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ShowHelpHandler.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Handlers/ShowHelpHandler.cs @@ -72,7 +72,8 @@ public async Task Handle(ShowHelpParams request, CancellationToken cancell // TODO: Rather than print the help in the console, we should send the string back // to VSCode to display in a help pop-up (or similar) - await _powerShellContextService.ExecuteCommandAsync(checkHelpPSCommand, sendOutputToHost: true).ConfigureAwait(false); + await _powerShellContextService.ExecuteCommandAsync( + checkHelpPSCommand, sendOutputToHost: true, cancellationToken: cancellationToken).ConfigureAwait(false); return Unit.Value; } } diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs b/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs index 18c7c5314..ff2db32a7 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs @@ -519,9 +519,11 @@ public Task GetRunspaceHandleAsync(CancellationToken cancellatio public Task> ExecuteCommandAsync( PSCommand psCommand, bool sendOutputToHost = false, - bool sendErrorToHost = true) + bool sendErrorToHost = true, + CancellationToken cancellationToken = default) { - return ExecuteCommandAsync(psCommand, errorMessages: null, sendOutputToHost, sendErrorToHost); + return this.ExecuteCommandAsync( + psCommand, errorMessages: null, sendOutputToHost, sendErrorToHost, cancellationToken: cancellationToken); } /// @@ -549,7 +551,8 @@ public Task> ExecuteCommandAsync( StringBuilder errorMessages, bool sendOutputToHost = false, bool sendErrorToHost = true, - bool addToHistory = false) + bool addToHistory = false, + CancellationToken cancellationToken = default) { return this.ExecuteCommandAsync( @@ -560,7 +563,8 @@ public Task> ExecuteCommandAsync( WriteOutputToHost = sendOutputToHost, WriteErrorsToHost = sendErrorToHost, AddToHistory = addToHistory - }); + }, + cancellationToken); } @@ -581,7 +585,8 @@ public Task> ExecuteCommandAsync( public async Task> ExecuteCommandAsync( PSCommand psCommand, StringBuilder errorMessages, - ExecutionOptions executionOptions) + ExecutionOptions executionOptions, + CancellationToken cancellationToken = default) { Validate.IsNotNull(nameof(psCommand), psCommand); Validate.IsNotNull(nameof(executionOptions), executionOptions); @@ -759,8 +764,11 @@ public async Task> ExecuteCommandAsync( return shell.Invoke(null, invocationSettings); } - // May need a cancellation token here - return await Task.Run>(() => shell.Invoke(input: null, invocationSettings), CancellationToken.None).ConfigureAwait(false); + // This is the primary reason that ExecuteCommandAsync takes a CancellationToken + cancellationToken.Register(() => shell.Stop()); + return await Task.Run>( + () => shell.Invoke(input: null, invocationSettings), cancellationToken) + .ConfigureAwait(false); } finally { @@ -872,7 +880,7 @@ public async Task> ExecuteCommandAsync( // will exist already so we need to create one and then use it if (runspaceHandle == null) { - runspaceHandle = await this.GetRunspaceHandleAsync().ConfigureAwait(false); + runspaceHandle = await this.GetRunspaceHandleAsync(cancellationToken).ConfigureAwait(false); } sessionDetails = this.GetSessionDetailsInRunspace(runspaceHandle.Runspace); diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Session/Host/EditorServicesPSHostUserInterface.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Session/Host/EditorServicesPSHostUserInterface.cs index 0df1224dc..744cd0c41 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Session/Host/EditorServicesPSHostUserInterface.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Session/Host/EditorServicesPSHostUserInterface.cs @@ -731,7 +731,8 @@ private async Task WritePromptStringToHostAsync(CancellationToken cancellationTo cancellationToken.ThrowIfCancellationRequested(); string promptString = - (await this.powerShellContext.ExecuteCommandAsync(promptCommand, false, false).ConfigureAwait(false)) + (await this.powerShellContext.ExecuteCommandAsync( + promptCommand, false, false, cancellationToken).ConfigureAwait(false)) .Select(pso => pso.BaseObject) .OfType() .FirstOrDefault() ?? "PS> "; diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Session/PSReadLinePromptContext.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Session/PSReadLinePromptContext.cs index d58afc7f1..e74519913 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Session/PSReadLinePromptContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Session/PSReadLinePromptContext.cs @@ -143,7 +143,8 @@ public async Task InvokeReadLineAsync(bool isCommandLine, CancellationTo IEnumerable readLineResults = await _powerShellContext.ExecuteCommandAsync( readLineCommand, errorMessages: null, - s_psrlExecutionOptions).ConfigureAwait(false); + s_psrlExecutionOptions, + cancellationToken).ConfigureAwait(false); string line = readLineResults.FirstOrDefault();