Skip to content

Commit 67d8b96

Browse files
radicalradical
authored
[wasm] Misc Debugger improvements (including tests) (#65752)
* [wasm][debugger] Fail test if an assertion is detected * [wasm][debugger] Make DevToolsProxy's `pending_ops` thread-safe Currently, `pending_ops` can get written by different threads at the same time, and also read in parallel. This causes tests to randomly fail with: ``` DevToolsProxy::Run: Exception System.ArgumentException: The tasks argument included a null value. (Parameter 'tasks') at System.Threading.Tasks.Task.WhenAny(Task[] tasks) at Microsoft.WebAssembly.Diagnostics.DevToolsProxy.Run(Uri browserUri, WebSocket ideSocket) in /_/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs:line 269 ``` Instead, we use `Channel<T>` to add the ops, and then read those in the main loop, and add to the *local* `pending_ops` list. * [wasm] Install chrome for debugger tests - controlled by `$(InstallChromeForDebuggerTests)` which defaults to `true` for non-CI docker containers - Useful for using the correct chrome version when testing locally, or on codespaces - Also, add support for detecting, and defaulting to such an installation * [wasm][debugger] Disable tests failing with runtime assertions `DebuggerTests.EvaluateOnCallFrameTests.EvaluateSimpleMethodCallsError`: #65744 `DebuggerTests.ArrayTests.InvalidArrayId`: #65742 * [wasm][debugger] Fix NRE with `"null"` condition for a breakpoint `ConditionalBreakpoint` test fails. The test with condition=`"null"` failed with a NRE, instead of correctly handling it as a condition that returns null. ``` Unable evaluate conditional breakpoint: System.Exception: Internal Error: Unable to run (null ), error: Object reference not set to an instance of an object.. ---> System.NullReferenceException: Object reference not set to an instance of an object. at Microsoft.WebAssembly.Diagnostics.EvaluateExpression.CompileAndRunTheExpression(String expression, MemberReferenceResolver resolver, CancellationToken token) in /workspaces/runtime/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs:line 399 --- End of inner exception stack trace --- at Microsoft.WebAssembly.Diagnostics.EvaluateExpression.CompileAndRunTheExpression(String expression, MemberReferenceResolver resolver, CancellationToken token) in /workspaces/runtime/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs:line 407 at Microsoft.WebAssembly.Diagnostics.MonoProxy.EvaluateCondition(SessionId sessionId, ExecutionContext context, Frame mono_frame, Breakpoint bp, CancellationToken token) in /workspaces/runtime/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs:line 801 condition:null ``` * [wasm] Improve default message for ReturnAsErrorException, .. since we seem to be not catching them as intended in many places. * [wasm] Better log, and surface errors in evaluation conditional .. breakpoints. Catch the proper exception `ReturnAsErrorException`, so we can get the actual error message. And log that as an error for the user too. * [wasm][debugger] Allow setting proxy port for BrowserDebugHost .. with `--proxy-port=<port>`. Fixes #65209 . * [wasm][debugger] Handle errors in getting value for locals Essentially, catch, and skip. * message cleanup * remove trailing space
1 parent ef231a1 commit 67d8b96

13 files changed

Lines changed: 207 additions & 64 deletions

‎src/libraries/sendtohelix-wasm.targets‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838
<HelixPreCommand Include="export XHARNESS_DISABLE_COLORED_OUTPUT=true" />
3939
<HelixPreCommand Include="export XHARNESS_LOG_WITH_TIMESTAMPS=true" />
4040

41-
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="export PATH=$HELIX_CORRELATION_PAYLOAD/chromedriver_linux64:$PATH" />
42-
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="export PATH=$HELIX_CORRELATION_PAYLOAD/chrome-linux:$PATH" />
41+
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="export PATH=$HELIX_CORRELATION_PAYLOAD/$(ChromeDriverDirName):$PATH" />
42+
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="export PATH=$HELIX_CORRELATION_PAYLOAD/$(ChromiumDirName):$PATH" />
4343

4444
<!-- Fix symbolic links that are broken already on build machine and also in the correlation payload -->
4545
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="export HELIX_NODEJS_VERSION=%24(ls $HELIX_CORRELATION_PAYLOAD/build/emsdk/node)" />
@@ -60,8 +60,8 @@
6060
<HelixPreCommand Include="set XHARNESS_DISABLE_COLORED_OUTPUT=true" />
6161
<HelixPreCommand Include="set XHARNESS_LOG_WITH_TIMESTAMPS=true" />
6262

63-
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\chromedriver_win32%3B%PATH%" />
64-
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\chrome-win%3B%PATH%" />
63+
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\$(ChromeDriverDirName)%3B%PATH%" />
64+
<HelixPreCommand Condition="'$(NeedsToRunOnBrowser)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\$(ChromiumDirName)%3B%PATH%" />
6565

6666
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="for /f %%i in ('dir %HELIX_CORRELATION_PAYLOAD%\build\emsdk\node /b') do set HELIX_NODEJS_VERSION=%%i" />
6767
<HelixPreCommand Condition="'$(NeedsEMSDKNode)' == 'true'" Include="set PATH=%HELIX_CORRELATION_PAYLOAD%\build\emsdk\node\%HELIX_NODEJS_VERSION%\bin%3B%PATH%" />
@@ -96,6 +96,8 @@
9696
</When>
9797
</Choose>
9898

99+
<Import Project="$(RepoRoot)src\mono\wasm\BrowsersForTesting.props" />
100+
99101
<Target Name="PrepareForBuildHelixWorkItems_Wasm">
100102
<PropertyGroup>
101103
<WasmBuildTargetsDir>$([MSBuild]::NormalizeDirectory('$(RepoRoot)', 'src', 'mono', 'wasm', 'build'))</WasmBuildTargetsDir>
@@ -106,26 +108,6 @@
106108
<WorkItemPrefix Condition="'$(WorkItemPrefix)' == '' and '$(Scenario)' != ''">$(Scenario)-</WorkItemPrefix>
107109
</PropertyGroup>
108110

109-
<!-- Version number to revision number mapping from http://omahaproxy.appspot.com/ and find the closest one in
110-
https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Linux_x64/
111-
and https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Win_x64/
112-
113-
Eg. latest stable version is 96.0.4664.45 with revision 929512.
114-
but the closest one available in the snapshosts is 929513.
115-
Please make sure to check both platforms as sometime
116-
the same snapshot might not be available in one of them.
117-
-->
118-
<PropertyGroup Condition="'$(BrowserHost)' != 'windows'">
119-
<ChromiumRevision>929513</ChromiumRevision>
120-
<ChromiumUrl>https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chrome-linux.zip</ChromiumUrl>
121-
<ChromeDriverUrl>https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chromedriver_linux64.zip</ChromeDriverUrl>
122-
</PropertyGroup>
123-
<PropertyGroup Condition="'$(BrowserHost)' == 'windows'">
124-
<ChromiumRevision>929513</ChromiumRevision>
125-
<ChromiumUrl>https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chrome-win.zip</ChromiumUrl>
126-
<ChromeDriverUrl>https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chromedriver_win32.zip</ChromeDriverUrl>
127-
</PropertyGroup>
128-
129111
<ItemGroup Condition="'$(NeedsToRunOnBrowser)' == 'true'">
130112
<HelixCorrelationPayload Include="chromium" Uri="$(ChromiumUrl)" />
131113
<HelixCorrelationPayload Include="chromedriver" Uri="$(ChromeDriverUrl)" />

‎src/mono/wasm/BrowsersForTesting.props‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<Project>
2+
<!-- Version number to revision number mapping from http://omahaproxy.appspot.com/ and find the closest one in
3+
https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Linux_x64/
4+
and https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Win_x64/
5+
6+
Eg. latest stable version is 96.0.4664.45 with revision 929512.
7+
but the closest one available in the snapshosts is 929513.
8+
Please make sure to check both platforms as sometime
9+
the same snapshot might not be available in one of them.
10+
-->
11+
<PropertyGroup Condition="'$(BrowserHost)' != 'windows'">
12+
<ChromiumRevision>929513</ChromiumRevision>
13+
<ChromiumUrl>https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chrome-linux.zip</ChromiumUrl>
14+
<ChromeDriverUrl>https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chromedriver_linux64.zip</ChromeDriverUrl>
15+
<ChromiumDirName>chrome-linux</ChromiumDirName>
16+
<ChromeDriverDirName>chromedriver_linux64</ChromeDriverDirName>
17+
<ChromiumBinaryName>chrome</ChromiumBinaryName>
18+
</PropertyGroup>
19+
20+
<PropertyGroup Condition="'$(BrowserHost)' == 'windows'">
21+
<ChromiumRevision>929513</ChromiumRevision>
22+
<ChromiumUrl>https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chrome-win.zip</ChromiumUrl>
23+
<ChromeDriverUrl>https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chromedriver_win32.zip</ChromeDriverUrl>
24+
<ChromiumDirName>chrome-win</ChromiumDirName>
25+
<ChromeDriverDirName>chromedriver_win32</ChromeDriverDirName>
26+
<ChromiumBinaryName>chrome.exe</ChromiumBinaryName>
27+
</PropertyGroup>
28+
</Project>

‎src/mono/wasm/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ To run a test with `FooBar` in the name:
143143

144144
Additional arguments for `dotnet test` can be passed via `MSBUILD_ARGS` or `TEST_ARGS`. For example `MSBUILD_ARGS="/p:WasmDebugLevel=5"`. Though only one of `TEST_ARGS`, or `TEST_FILTER` can be used at a time.
145145

146+
- Chrome can be installed for testing by setting `InstallChromeForDebuggerTests=true` when building the tests.
147+
146148
## Run samples
147149

148150
The samples in `src/mono/sample/wasm` can be build and run like this:

‎src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Net.WebSockets;
99
using System.Text;
1010
using System.Threading;
11+
using System.Threading.Channels;
1112
using System.Threading.Tasks;
1213
using Microsoft.Extensions.Logging;
1314
using Newtonsoft.Json;
@@ -24,7 +25,8 @@ internal class DevToolsProxy
2425
private ClientWebSocket browser;
2526
private WebSocket ide;
2627
private int next_cmd_id;
27-
private List<Task> pending_ops = new List<Task>();
28+
private readonly ChannelWriter<Task> _channelWriter;
29+
private readonly ChannelReader<Task> _channelReader;
2830
private List<DevToolsQueue> queues = new List<DevToolsQueue>();
2931

3032
protected readonly ILogger logger;
@@ -33,6 +35,10 @@ public DevToolsProxy(ILoggerFactory loggerFactory, string loggerId)
3335
{
3436
string loggerSuffix = string.IsNullOrEmpty(loggerId) ? string.Empty : $"-{loggerId}";
3537
logger = loggerFactory.CreateLogger($"{nameof(DevToolsProxy)}{loggerSuffix}");
38+
39+
var channel = Channel.CreateUnbounded<Task>(new UnboundedChannelOptions { SingleReader = true });
40+
_channelWriter = channel.Writer;
41+
_channelReader = channel.Reader;
3642
}
3743

3844
protected virtual Task<bool> AcceptEvent(SessionId sessionId, string method, JObject args, CancellationToken token)
@@ -94,7 +100,7 @@ private DevToolsQueue GetQueueForTask(Task task)
94100
return queues.FirstOrDefault(q => q.CurrentSend == task);
95101
}
96102

97-
private void Send(WebSocket to, JObject o, CancellationToken token)
103+
private async Task Send(WebSocket to, JObject o, CancellationToken token)
98104
{
99105
string sender = browser == to ? "Send-browser" : "Send-ide";
100106

@@ -106,7 +112,7 @@ private void Send(WebSocket to, JObject o, CancellationToken token)
106112

107113
Task task = queue.Send(bytes, token);
108114
if (task != null)
109-
pending_ops.Add(task);
115+
await _channelWriter.WriteAsync(task, token);
110116
}
111117

112118
private async Task OnEvent(SessionId sessionId, string method, JObject args, CancellationToken token)
@@ -116,7 +122,7 @@ private async Task OnEvent(SessionId sessionId, string method, JObject args, Can
116122
if (!await AcceptEvent(sessionId, method, args, token))
117123
{
118124
//logger.LogDebug ("proxy browser: {0}::{1}",method, args);
119-
SendEventInternal(sessionId, method, args, token);
125+
await SendEventInternal(sessionId, method, args, token);
120126
}
121127
}
122128
catch (Exception e)
@@ -132,7 +138,7 @@ private async Task OnCommand(MessageId id, string method, JObject args, Cancella
132138
if (!await AcceptCommand(id, method, args, token))
133139
{
134140
Result res = await SendCommandInternal(id, method, args, token);
135-
SendResponseInternal(id, res, token);
141+
await SendResponseInternal(id, res, token);
136142
}
137143
}
138144
catch (Exception e)
@@ -153,31 +159,38 @@ private void OnResponse(MessageId id, Result result)
153159
logger.LogError("Cannot respond to command: {id} with result: {result} - command is not pending", id, result);
154160
}
155161

156-
private void ProcessBrowserMessage(string msg, CancellationToken token)
162+
private Task ProcessBrowserMessage(string msg, CancellationToken token)
157163
{
158164
var res = JObject.Parse(msg);
159165

160166
//if (method != "Debugger.scriptParsed" && method != "Runtime.consoleAPICalled")
161167
Log("protocol", $"browser: {msg}");
162168

163169
if (res["id"] == null)
164-
pending_ops.Add(OnEvent(res.ToObject<SessionId>(), res["method"].Value<string>(), res["params"] as JObject, token));
170+
{
171+
return OnEvent(res.ToObject<SessionId>(), res["method"].Value<string>(), res["params"] as JObject, token);
172+
}
165173
else
174+
{
166175
OnResponse(res.ToObject<MessageId>(), Result.FromJson(res));
176+
return null;
177+
}
167178
}
168179

169-
private void ProcessIdeMessage(string msg, CancellationToken token)
180+
private Task ProcessIdeMessage(string msg, CancellationToken token)
170181
{
171182
Log("protocol", $"ide: {msg}");
172183
if (!string.IsNullOrEmpty(msg))
173184
{
174185
var res = JObject.Parse(msg);
175186
var id = res.ToObject<MessageId>();
176-
pending_ops.Add(OnCommand(
187+
return OnCommand(
177188
id,
178189
res["method"].Value<string>(),
179-
res["params"] as JObject, token));
190+
res["params"] as JObject, token);
180191
}
192+
193+
return null;
181194
}
182195

183196
internal async Task<Result> SendCommand(SessionId id, string method, JObject args, CancellationToken token)
@@ -186,7 +199,7 @@ internal async Task<Result> SendCommand(SessionId id, string method, JObject arg
186199
return await SendCommandInternal(id, method, args, token);
187200
}
188201

189-
private Task<Result> SendCommandInternal(SessionId sessionId, string method, JObject args, CancellationToken token)
202+
private async Task<Result> SendCommandInternal(SessionId sessionId, string method, JObject args, CancellationToken token)
190203
{
191204
int id = Interlocked.Increment(ref next_cmd_id);
192205

@@ -204,17 +217,17 @@ private Task<Result> SendCommandInternal(SessionId sessionId, string method, JOb
204217
//Log ("verbose", $"add cmd id {sessionId}-{id}");
205218
pending_cmds[msgId] = tcs;
206219

207-
Send(this.browser, o, token);
208-
return tcs.Task;
220+
await Send(browser, o, token);
221+
return await tcs.Task;
209222
}
210223

211-
public void SendEvent(SessionId sessionId, string method, JObject args, CancellationToken token)
224+
public Task SendEvent(SessionId sessionId, string method, JObject args, CancellationToken token)
212225
{
213226
//Log ("verbose", $"sending event {method}: {args}");
214-
SendEventInternal(sessionId, method, args, token);
227+
return SendEventInternal(sessionId, method, args, token);
215228
}
216229

217-
private void SendEventInternal(SessionId sessionId, string method, JObject args, CancellationToken token)
230+
private Task SendEventInternal(SessionId sessionId, string method, JObject args, CancellationToken token)
218231
{
219232
var o = JObject.FromObject(new
220233
{
@@ -224,21 +237,21 @@ private void SendEventInternal(SessionId sessionId, string method, JObject args,
224237
if (sessionId.sessionId != null)
225238
o["sessionId"] = sessionId.sessionId;
226239

227-
Send(this.ide, o, token);
240+
return Send(ide, o, token);
228241
}
229242

230243
internal void SendResponse(MessageId id, Result result, CancellationToken token)
231244
{
232245
SendResponseInternal(id, result, token);
233246
}
234247

235-
private void SendResponseInternal(MessageId id, Result result, CancellationToken token)
248+
private Task SendResponseInternal(MessageId id, Result result, CancellationToken token)
236249
{
237250
JObject o = result.ToJObject(id);
238251
if (!result.IsOk)
239252
logger.LogError($"sending error response for id: {id} -> {result}");
240253

241-
Send(this.ide, o, token);
254+
return Send(this.ide, o, token);
242255
}
243256

244257
// , HttpContext context)
@@ -258,10 +271,14 @@ public async Task Run(Uri browserUri, WebSocket ideSocket)
258271
Log("verbose", $"DevToolsProxy: Client connected on {browserUri}");
259272
var x = new CancellationTokenSource();
260273

274+
List<Task> pending_ops = new();
275+
261276
pending_ops.Add(ReadOne(browser, x.Token));
262277
pending_ops.Add(ReadOne(ide, x.Token));
263278
pending_ops.Add(side_exception.Task);
264279
pending_ops.Add(client_initiated_close.Task);
280+
Task<bool> readerTask = _channelReader.WaitToReadAsync(x.Token).AsTask();
281+
pending_ops.Add(readerTask);
265282

266283
try
267284
{
@@ -278,14 +295,26 @@ public async Task Run(Uri browserUri, WebSocket ideSocket)
278295
break;
279296
}
280297

298+
if (readerTask.IsCompleted)
299+
{
300+
while (_channelReader.TryRead(out Task newTask))
301+
{
302+
pending_ops.Add(newTask);
303+
}
304+
305+
pending_ops[4] = _channelReader.WaitToReadAsync(x.Token).AsTask();
306+
}
307+
281308
//logger.LogTrace ("pump {0} {1}", task, pending_ops.IndexOf (task));
282309
if (completedTask == pending_ops[0])
283310
{
284311
string msg = ((Task<string>)completedTask).Result;
285312
if (msg != null)
286313
{
287314
pending_ops[0] = ReadOne(browser, x.Token); //queue next read
288-
ProcessBrowserMessage(msg, x.Token);
315+
Task newTask = ProcessBrowserMessage(msg, x.Token);
316+
if (newTask != null)
317+
pending_ops.Add(newTask);
289318
}
290319
}
291320
else if (completedTask == pending_ops[1])
@@ -294,7 +323,9 @@ public async Task Run(Uri browserUri, WebSocket ideSocket)
294323
if (msg != null)
295324
{
296325
pending_ops[1] = ReadOne(ide, x.Token); //queue next read
297-
ProcessIdeMessage(msg, x.Token);
326+
Task newTask = ProcessIdeMessage(msg, x.Token);
327+
if (newTask != null)
328+
pending_ops.Add(newTask);
298329
}
299330
}
300331
else if (completedTask == pending_ops[2])
@@ -314,10 +345,13 @@ public async Task Run(Uri browserUri, WebSocket ideSocket)
314345
}
315346
}
316347
}
348+
349+
_channelWriter.Complete();
317350
}
318351
catch (Exception e)
319352
{
320353
Log("error", $"DevToolsProxy::Run: Exception {e}");
354+
_channelWriter.Complete(e);
321355
//throw;
322356
}
323357
finally

‎src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ internal static async Task<JObject> CompileAndRunTheExpression(string expression
396396
string.Join("\n", findVarNMethodCall.variableDefinitions) + "\nreturn " + syntaxTree.ToString());
397397

398398
var state = await newScript.RunAsync(cancellationToken: token);
399-
return JObject.FromObject(ConvertCSharpToJSType(state.ReturnValue, state.ReturnValue.GetType()));
399+
return JObject.FromObject(ConvertCSharpToJSType(state.ReturnValue, state.ReturnValue?.GetType()));
400400
}
401401
catch (CompilationErrorException cee)
402402
{
@@ -419,7 +419,7 @@ internal static async Task<JObject> CompileAndRunTheExpression(string expression
419419
private static object ConvertCSharpToJSType(object v, Type type)
420420
{
421421
if (v == null)
422-
return new { type = "object", subtype = "null", className = type.ToString(), description = type.ToString() };
422+
return new { type = "object", subtype = "null", className = type?.ToString(), description = type?.ToString() };
423423
if (v is string s)
424424
return new { type = "string", value = s, description = s };
425425
if (NumericTypes.Contains(v.GetType()))
@@ -443,10 +443,11 @@ public Result Error
443443
}
444444
set { }
445445
}
446-
public ReturnAsErrorException(JObject error)
446+
public ReturnAsErrorException(JObject error) : base(error.ToString())
447447
=> Error = Result.Err(error);
448448

449449
public ReturnAsErrorException(string message, string className)
450+
: base($"[{className}] {message}")
450451
{
451452
var result = new
452453
{
@@ -466,5 +467,7 @@ public ReturnAsErrorException(string message, string className)
466467
}
467468
}));
468469
}
470+
471+
public override string ToString() => $"Error object: {Error}. {base.ToString()}";
469472
}
470473
}

0 commit comments

Comments
 (0)