Skip to content

Commit e2688a8

Browse files
committed
This is a *proposed* change to address the issue that the launch request could come after the configurationDone request (microsoft/vscode#3548).
This commit "should" allow the messages to come in either order and still work. However, it needs a good review. BTW it appears all the messages are processed on the same thread (or maybe that was coincidence). If that is the case then perhaps the lock is not needed. Still it is a fairly lightweight lock - one bool check and an assignment.
1 parent 3605e55 commit e2688a8

File tree

1 file changed

+68
-30
lines changed

1 file changed

+68
-30
lines changed

src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs

Lines changed: 68 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ public class DebugAdapter : DebugAdapterBase
2020
{
2121
private EditorSession editorSession;
2222
private OutputDebouncer outputDebouncer;
23+
private object syncLock = new object();
24+
private bool isConfigurationDoneRequestComplete;
25+
private bool isLaunchRequestComplete;
2326
private string scriptPathToLaunch;
2427
private string arguments;
2528

@@ -65,6 +68,23 @@ protected override void Initialize()
6568
this.SetRequestHandler(EvaluateRequest.Type, this.HandleEvaluateRequest);
6669
}
6770

71+
protected Task LaunchScript(RequestContext<object> requestContext)
72+
{
73+
return editorSession.PowerShellContext
74+
.ExecuteScriptAtPath(this.scriptPathToLaunch, this.arguments)
75+
.ContinueWith(
76+
async (t) => {
77+
Logger.Write(LogLevel.Verbose, "Execution completed, terminating...");
78+
79+
await requestContext.SendEvent(
80+
TerminatedEvent.Type,
81+
null);
82+
83+
// Stop the server
84+
this.Stop();
85+
});
86+
}
87+
6888
protected override void Shutdown()
6989
{
7090
// Make sure remaining output is flushed before exiting
@@ -81,6 +101,31 @@ protected override void Shutdown()
81101

82102
#region Built-in Message Handlers
83103

104+
protected async Task HandleConfigurationDoneRequest(
105+
object args,
106+
RequestContext<object> requestContext)
107+
{
108+
// Ensure that only the second message between launch and
109+
// configurationDone - actually launches the script.
110+
lock (syncLock)
111+
{
112+
if (!this.isLaunchRequestComplete)
113+
{
114+
this.isConfigurationDoneRequestComplete = true;
115+
}
116+
}
117+
118+
// The order of debug protocol messages apparently isn't as guaranteed as we might like.
119+
// Need to be able to handle the case where we get configurationDone after launch request
120+
// and vice-versa.
121+
if (this.isLaunchRequestComplete)
122+
{
123+
this.LaunchScript(requestContext);
124+
}
125+
126+
await requestContext.SendResult(null);
127+
}
128+
84129
protected async Task HandleLaunchRequest(
85130
LaunchRequestArguments launchParams,
86131
RequestContext<object> requestContext)
@@ -114,14 +159,32 @@ protected async Task HandleLaunchRequest(
114159
Logger.Write(LogLevel.Verbose, "Script arguments are: " + arguments);
115160
}
116161

117-
// NOTE: We don't actually launch the script in response to this
118-
// request. We wait until we receive the configurationDone request
119-
// to actually launch the script under the debugger. This gives
120-
// us and VSCode a chance to finish configuring all the types of
121-
// breakpoints.
162+
// We may not actually launch the script in response to this
163+
// request unless it comes after the configurationDone request.
164+
// If the launch request comes first, then stash the launch
165+
// params so that the subsequent configurationDone request can
166+
// launch the script.
122167
this.scriptPathToLaunch = launchParams.Program;
123168
this.arguments = arguments;
124169

170+
// Ensure that only the second message between launch and
171+
// configurationDone - actually launches the script.
172+
lock (syncLock)
173+
{
174+
if (!this.isConfigurationDoneRequestComplete)
175+
{
176+
this.isLaunchRequestComplete = true;
177+
}
178+
}
179+
180+
// The order of debug protocol messages apparently isn't as guaranteed as we might like.
181+
// Need to be able to handle the case where we get configurationDone after launch request
182+
// and vice-versa.
183+
if (this.isConfigurationDoneRequestComplete)
184+
{
185+
this.LaunchScript(requestContext);
186+
}
187+
125188
await requestContext.SendResult(null);
126189
}
127190

@@ -133,31 +196,6 @@ protected Task HandleAttachRequest(
133196
throw new NotImplementedException();
134197
}
135198

136-
protected async Task HandleConfigurationDoneRequest(
137-
object args,
138-
RequestContext<object> requestContext)
139-
{
140-
// Execute the given PowerShell script and send the response.
141-
// Note that we aren't waiting for execution to complete here
142-
// because the debugger could stop while the script executes.
143-
Task executeTask =
144-
editorSession.PowerShellContext
145-
.ExecuteScriptAtPath(this.scriptPathToLaunch, this.arguments)
146-
.ContinueWith(
147-
async (t) => {
148-
Logger.Write(LogLevel.Verbose, "Execution completed, terminating...");
149-
150-
await requestContext.SendEvent(
151-
TerminatedEvent.Type,
152-
null);
153-
154-
// Stop the server
155-
this.Stop();
156-
});
157-
158-
await requestContext.SendResult(null);
159-
}
160-
161199
protected Task HandleDisconnectRequest(
162200
object disconnectParams,
163201
RequestContext<object> requestContext)

0 commit comments

Comments
 (0)