Skip to content

Make ExecuteCommandAsync cancellable #1532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/PowerShellEditorServices/Server/PsesDebugServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as they're sent to us fresh in the next session, I think we should yeah

_debugAdapterServer.Dispose();
_inputStream.Dispose();
_outputStream.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc
IEnumerable<Breakpoint> setBreakpoints =
await _powerShellContextService.ExecuteCommandAsync<Breakpoint>(psCommand).ConfigureAwait(false);
configuredBreakpoints.AddRange(
setBreakpoints.Select(BreakpointDetails.Create));
setBreakpoints.Select((breakpoint) => BreakpointDetails.Create(breakpoint))
);
}

return configuredBreakpoints;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ e.OriginalEvent.Breakpoints[0] is CommandBreakpoint
new StoppedEvent
{
ThreadId = 1,
AllThreadsStopped = true,
Reason = debuggerStoppedReason
});
}
Expand Down Expand Up @@ -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 => BreakpointEventReason.New,
BreakpointUpdateType.Removed => BreakpointEventReason.Removed,
BreakpointUpdateType.Enabled => BreakpointEventReason.Changed,
BreakpointUpdateType.Disabled => BreakpointEventReason.Changed,
_ => "InvalidBreakpointUpdateTypeEnum"
};

_debugAdapterServer.SendNotification(
EventNames.Breakpoint,
new BreakpointEvent { Breakpoint = breakpoint, Reason = reason }
);
}
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ internal static BreakpointDetails Create(
/// PowerShell Breakpoint object.
/// </summary>
/// <param name="breakpoint">The Breakpoint instance from which details will be taken.</param>
/// <param name="updateType">The BreakpointUpdateType to determine if the breakpoint is verified.</param>
/// <returns>A new instance of the BreakpointDetails class.</returns>
internal static BreakpointDetails Create(Breakpoint breakpoint)
internal static BreakpointDetails Create(
Breakpoint breakpoint,
BreakpointUpdateType updateType = BreakpointUpdateType.Set)
{
Validate.IsNotNull("breakpoint", breakpoint);

Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,13 @@ await _powerShellContextService.ExecuteScriptStringAsync(
string debugRunspaceCmd;
if (request.RunspaceName != null)
{
IEnumerable<int?> ids = await _powerShellContextService.ExecuteCommandAsync<int?>(new PSCommand()
.AddCommand("Microsoft.PowerShell.Utility\\Get-Runspace")
.AddParameter("Name", request.RunspaceName)
.AddCommand("Microsoft.PowerShell.Utility\\Select-Object")
.AddParameter("ExpandProperty", "Id")).ConfigureAwait(false);
IEnumerable<int?> ids = await _powerShellContextService.ExecuteCommandAsync<int?>(
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public Task<ThreadsResponse> 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,8 @@ internal async Task<string> InvokeLegacyReadLineAsync(bool isCommandLine, Cancel
command.AddParameter("CursorColumn", currentCursorIndex);
command.AddParameter("Options", null);

var results = await this.powerShellContext
.ExecuteCommandAsync<CommandCompletion>(command, sendOutputToHost: false, sendErrorToHost: false)
.ConfigureAwait(false);
var results = await this.powerShellContext.ExecuteCommandAsync<CommandCompletion>(
command, sendOutputToHost: false, sendErrorToHost: false, cancellationToken).ConfigureAwait(false);

currentCompletion = results.FirstOrDefault();
}
Expand Down Expand Up @@ -327,8 +326,8 @@ internal async Task<string> InvokeLegacyReadLineAsync(bool isCommandLine, Cancel
PSCommand command = new PSCommand();
command.AddCommand("Get-History");

currentHistory = await this.powerShellContext.ExecuteCommandAsync<PSObject>(command, sendOutputToHost: false, sendErrorToHost: false)
.ConfigureAwait(false)
currentHistory = await this.powerShellContext.ExecuteCommandAsync<PSObject>(
command, sendOutputToHost: false, sendErrorToHost: false, cancellationToken).ConfigureAwait(false)
as Collection<PSObject>;

if (currentHistory != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ function __Expand-Alias {
.AddStatement()
.AddCommand("__Expand-Alias")
.AddArgument(request.Text);
var result = await _powerShellContextService.ExecuteCommandAsync<string>(psCommand).ConfigureAwait(false);
var result = await _powerShellContextService.ExecuteCommandAsync<string>(
psCommand, cancellationToken: cancellationToken).ConfigureAwait(false);

return new ExpandAliasResult
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public async Task<List<PSCommandMessage>> Handle(GetCommandParams request, Cance
.AddCommand("Microsoft.PowerShell.Utility\\Sort-Object")
.AddParameter("Property", "Name");

IEnumerable<CommandInfo> result = await _powerShellContextService.ExecuteCommandAsync<CommandInfo>(psCommand).ConfigureAwait(false);
IEnumerable<CommandInfo> result = await _powerShellContextService.ExecuteCommandAsync<CommandInfo>(
psCommand, cancellationToken: cancellationToken).ConfigureAwait(false);

var commandList = new List<PSCommandMessage>();
if (result != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ public async Task<RunspaceResponse[]> 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<PSObject>(psCommand, sb).ConfigureAwait(false);
runspaces = await _powerShellContextService.ExecuteCommandAsync<PSObject>(
psCommand, sb, cancellationToken: cancellationToken).ConfigureAwait(false);
}

var runspaceResponses = new List<RunspaceResponse>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ public async Task<Unit> 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<PSObject>(checkHelpPSCommand, sendOutputToHost: true).ConfigureAwait(false);
await _powerShellContextService.ExecuteCommandAsync<PSObject>(
checkHelpPSCommand, sendOutputToHost: true, cancellationToken: cancellationToken).ConfigureAwait(false);
return Unit.Value;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -519,9 +519,11 @@ public Task<RunspaceHandle> GetRunspaceHandleAsync(CancellationToken cancellatio
public Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
PSCommand psCommand,
bool sendOutputToHost = false,
bool sendErrorToHost = true)
bool sendErrorToHost = true,
CancellationToken cancellationToken = default)
{
return ExecuteCommandAsync<TResult>(psCommand, errorMessages: null, sendOutputToHost, sendErrorToHost);
return this.ExecuteCommandAsync<TResult>(
psCommand, errorMessages: null, sendOutputToHost, sendErrorToHost, cancellationToken: cancellationToken);
}

/// <summary>
Expand Down Expand Up @@ -549,7 +551,8 @@ public Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
StringBuilder errorMessages,
bool sendOutputToHost = false,
bool sendErrorToHost = true,
bool addToHistory = false)
bool addToHistory = false,
CancellationToken cancellationToken = default)
{
return
this.ExecuteCommandAsync<TResult>(
Expand All @@ -560,7 +563,8 @@ public Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
WriteOutputToHost = sendOutputToHost,
WriteErrorsToHost = sendErrorToHost,
AddToHistory = addToHistory
});
},
cancellationToken);
}


Expand All @@ -581,7 +585,8 @@ public Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
public async Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
PSCommand psCommand,
StringBuilder errorMessages,
ExecutionOptions executionOptions)
ExecutionOptions executionOptions,
CancellationToken cancellationToken = default)
{
Validate.IsNotNull(nameof(psCommand), psCommand);
Validate.IsNotNull(nameof(executionOptions), executionOptions);
Expand Down Expand Up @@ -759,8 +764,11 @@ public async Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
return shell.Invoke<TResult>(null, invocationSettings);
}

// May need a cancellation token here
return await Task.Run<IEnumerable<TResult>>(() => shell.Invoke<TResult>(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<IEnumerable<TResult>>(
() => shell.Invoke<TResult>(input: null, invocationSettings), cancellationToken)
.ConfigureAwait(false);
}
finally
{
Expand Down Expand Up @@ -872,7 +880,7 @@ public async Task<IEnumerable<TResult>> ExecuteCommandAsync<TResult>(
// 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,8 @@ private async Task WritePromptStringToHostAsync(CancellationToken cancellationTo

cancellationToken.ThrowIfCancellationRequested();
string promptString =
(await this.powerShellContext.ExecuteCommandAsync<PSObject>(promptCommand, false, false).ConfigureAwait(false))
(await this.powerShellContext.ExecuteCommandAsync<PSObject>(
promptCommand, false, false, cancellationToken).ConfigureAwait(false))
.Select(pso => pso.BaseObject)
.OfType<string>()
.FirstOrDefault() ?? "PS> ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ public async Task<string> InvokeReadLineAsync(bool isCommandLine, CancellationTo
IEnumerable<string> readLineResults = await _powerShellContext.ExecuteCommandAsync<string>(
readLineCommand,
errorMessages: null,
s_psrlExecutionOptions).ConfigureAwait(false);
s_psrlExecutionOptions,
cancellationToken).ConfigureAwait(false);

string line = readLineResults.FirstOrDefault();

Expand Down