Skip to content
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

Fix extra prompting and manual debugger commands #1778

Merged
merged 2 commits into from
Apr 27, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -122,6 +123,9 @@ public LaunchAndAttachHandler(

public async Task<LaunchResponse> 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.
Expand Down Expand Up @@ -213,6 +217,9 @@ public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, Can

public async Task<AttachResponse> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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)
Expand All @@ -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();
}
}

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public override IReadOnlyList<TResult> Run(CancellationToken cancellationToken)

if (PowerShellExecutionOptions.WriteInputToHost)
{
_psesHost.WriteWithPrompt(_psCommand, cancellationToken);
_psesHost.UI.WriteLine(_psCommand.GetInvocationText());
}

return _pwsh.Runspace.Debugger.InBreakpoint
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -753,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);
Expand All @@ -778,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}");
Expand Down Expand Up @@ -825,25 +831,14 @@ private string GetPrompt(CancellationToken cancellationToken)
return prompt;
}

/// <summary>
/// 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.
/// </summary>
/// <param name="command">The PSCommand we'll print after the prompt.</param>
/// <param name="cancellationToken"></param>
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
Expand Down