Skip to content

Commit 87b3c9c

Browse files
committed
Fix extra prompting and manual debugger commands
This requires that we set the REPL to exit correctly. Note that we don't have a notion of "disconnect" so we removed that logic. We are assuming the debugger is "active" a little earlier now, that is, also when we've received an LSP notification to start it (via a launch or attach handler). Because we receive a notification on "disconnect" even for a script just finishing, we need to know that the server is not active and so not cancel the current prompt.
1 parent 9926944 commit 87b3c9c

File tree

3 files changed

+54
-34
lines changed

3 files changed

+54
-34
lines changed

Diff for: src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs

+13-6
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,16 @@
1111
using Microsoft.Extensions.Logging;
1212
using Microsoft.PowerShell.EditorServices.Logging;
1313
using Microsoft.PowerShell.EditorServices.Services;
14-
using OmniSharp.Extensions.DebugAdapter.Protocol.Events;
15-
using OmniSharp.Extensions.JsonRpc;
16-
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
17-
using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
18-
using OmniSharp.Extensions.DebugAdapter.Protocol.Server;
1914
using Microsoft.PowerShell.EditorServices.Services.PowerShell;
2015
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Context;
21-
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
2216
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution;
17+
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
18+
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace;
19+
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
20+
using OmniSharp.Extensions.DebugAdapter.Protocol.Events;
21+
using OmniSharp.Extensions.DebugAdapter.Protocol.Requests;
22+
using OmniSharp.Extensions.DebugAdapter.Protocol.Server;
23+
using OmniSharp.Extensions.JsonRpc;
2324

2425
namespace Microsoft.PowerShell.EditorServices.Handlers
2526
{
@@ -122,6 +123,9 @@ public LaunchAndAttachHandler(
122123

123124
public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, CancellationToken cancellationToken)
124125
{
126+
// The debugger has officially started. We use this to later check if we should stop it.
127+
((PsesInternalHost)_executionService).DebugContext.IsActive = true;
128+
125129
_debugEventHandlerService.RegisterEventHandlers();
126130

127131
// Determine whether or not the working directory should be set in the PowerShellContext.
@@ -213,6 +217,9 @@ public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, Can
213217

214218
public async Task<AttachResponse> Handle(PsesAttachRequestArguments request, CancellationToken cancellationToken)
215219
{
220+
// The debugger has officially started. We use this to later check if we should stop it.
221+
((PsesInternalHost)_executionService).DebugContext.IsActive = true;
222+
216223
_debugStateService.IsAttachSession = true;
217224

218225
_debugEventHandlerService.RegisterEventHandlers();

Diff for: src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs

+28-19
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public void EnableDebugMode()
9494
_psesHost.Runspace.Debugger.SetDebugMode(DebugModes.LocalScript | DebugModes.RemoteScript);
9595
}
9696

97-
public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop, isDisconnect: true);
97+
public void Abort() => SetDebugResuming(DebuggerResumeAction.Stop);
9898

9999
public void BreakExecution() => _psesHost.Runspace.Debugger.SetDebuggerStepMode(enabled: true);
100100

@@ -106,10 +106,12 @@ public void EnableDebugMode()
106106

107107
public void StepOver() => SetDebugResuming(DebuggerResumeAction.StepOver);
108108

109-
public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction, bool isDisconnect = false)
109+
public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction)
110110
{
111-
// NOTE: We exit because the paused/stopped debugger is currently in a prompt REPL, and
112-
// to resume the debugger we must exit that REPL.
111+
// We exit because the paused/stopped debugger is currently in a prompt REPL, and to
112+
// resume the debugger we must exit that REPL. If we're continued from 'c' or 's', this
113+
// is already set and so is a no-op; but if the user clicks the continue or step button,
114+
// then this came over LSP and we need to set it.
113115
_psesHost.SetExit();
114116

115117
if (LastStopEventArgs is not null)
@@ -127,23 +129,31 @@ public void SetDebugResuming(DebuggerResumeAction debuggerResumeAction, bool isD
127129
return;
128130
}
129131

130-
if (debuggerResumeAction is DebuggerResumeAction.Stop)
132+
// If we're stopping (or disconnecting, which is the same thing in LSP-land), then we
133+
// want to cancel any debug prompts, remote prompts, debugged scripts, etc. However, if
134+
// the debugged script has exited normally (or was quit with 'q'), we still get an LSP
135+
// notification that eventually lands here with a stop event. In this case, the debug
136+
// context is NOT active and we do not want to cancel the regular REPL.
137+
if (!_psesHost.DebugContext.IsActive)
131138
{
132-
// If we're disconnecting we want to unwind all the way back to the default, local
133-
// state. So we use UnwindCallStack here to ensure every context frame is cancelled.
134-
if (isDisconnect)
135-
{
136-
_psesHost.UnwindCallStack();
137-
return;
138-
}
139+
return;
140+
}
139141

140-
_psesHost.CancelIdleParentTask();
142+
// If the debugger is active and we're stopping, we need to unwind everything.
143+
if (debuggerResumeAction is DebuggerResumeAction.Stop)
144+
{
145+
// TODO: We need to assign cancellation tokens to each frame, because the current
146+
// logic results in a deadlock here when we try to cancel the scopes...which
147+
// includes ourself (hence running it in a separate thread).
148+
Task.Run(() => _psesHost.UnwindCallStack());
141149
return;
142150
}
143151

152+
// Otherwise we're continuing or stepping (i.e. resuming) so we need to cancel the
153+
// debugger REPL.
144154
if (_psesHost.CurrentFrame.IsRepl)
145155
{
146-
_psesHost.CancelCurrentTask();
156+
_psesHost.CancelIdleParentTask();
147157
}
148158
}
149159

@@ -166,15 +176,14 @@ public void ProcessDebuggerResult(DebuggerCommandResults debuggerResult)
166176
{
167177
if (debuggerResult?.ResumeAction is not null)
168178
{
169-
SetDebugResuming(debuggerResult.ResumeAction.Value);
170-
171-
// If a debugging command like `c` is specified in a nested remote
172-
// debugging prompt we need to unwind the nested execution loop.
173-
if (_psesHost.CurrentFrame.IsRemote)
179+
// Since we're processing a command like 'c' or 's' remotely, we need to tell the
180+
// host to stop the remote REPL loop.
181+
if (debuggerResult.ResumeAction is not DebuggerResumeAction.Stop || _psesHost.CurrentFrame.IsRemote)
174182
{
175183
_psesHost.ForceSetExit();
176184
}
177185

186+
SetDebugResuming(debuggerResult.ResumeAction.Value);
178187
RaiseDebuggerResumingEvent(new DebuggerResumingEventArgs(debuggerResult.ResumeAction.Value));
179188

180189
// The Terminate exception is used by the engine for flow control

Diff for: src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs

+13-9
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,19 @@ private void DoOneRepl(CancellationToken cancellationToken)
724724
return;
725725
}
726726

727+
// TODO: We must remove this awful logic, it causes so much pain. The StopDebugContext()
728+
// requires that we're not in a prompt that we're skipping, otherwise the debugger is
729+
// "active" but we haven't yet hit a breakpoint.
730+
//
731+
// When a task must run in the foreground, we cancel out of the idle loop and return to
732+
// the top level. At that point, we would normally run a REPL, but we need to
733+
// immediately execute the task. So we set _skipNextPrompt to do that.
734+
if (_skipNextPrompt)
735+
{
736+
_skipNextPrompt = false;
737+
return;
738+
}
739+
727740
// We use the REPL as a poll to check if the debug context is active but PowerShell
728741
// indicates we're no longer debugging. This happens when PowerShell was used to start
729742
// the debugger (instead of using a Code launch configuration) via Wait-Debugger or
@@ -736,15 +749,6 @@ private void DoOneRepl(CancellationToken cancellationToken)
736749
StopDebugContext();
737750
}
738751

739-
// When a task must run in the foreground, we cancel out of the idle loop and return to the top level.
740-
// At that point, we would normally run a REPL, but we need to immediately execute the task.
741-
// So we set _skipNextPrompt to do that.
742-
if (_skipNextPrompt)
743-
{
744-
_skipNextPrompt = false;
745-
return;
746-
}
747-
748752
try
749753
{
750754
string prompt = GetPrompt(cancellationToken);

0 commit comments

Comments
 (0)