From 0c33964ae0a91c23d0c3d121dddbffbcbbd1920f Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 22 Feb 2022 21:35:50 +0000 Subject: [PATCH 01/11] [wasm][debugger] Fail test if an assertion is detected --- .../debugger/DebuggerTestSuite/DevToolsClient.cs | 15 ++++++++++++++- .../wasm/debugger/DebuggerTestSuite/Inspector.cs | 12 +++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DevToolsClient.cs b/src/mono/wasm/debugger/DebuggerTestSuite/DevToolsClient.cs index 7c5874df22717..6eed6e177f9a9 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/DevToolsClient.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/DevToolsClient.cs @@ -18,6 +18,7 @@ internal class DevToolsClient : IDisposable ClientWebSocket socket; TaskCompletionSource _clientInitiatedClose = new TaskCompletionSource(); TaskCompletionSource _shutdownRequested = new TaskCompletionSource(); + readonly TaskCompletionSource _failRequested = new(); TaskCompletionSource _newSendTaskAvailable = new (); protected readonly ILogger logger; @@ -65,6 +66,14 @@ public async Task Shutdown(CancellationToken cancellationToken) } } + public void Fail(Exception exception) + { + if (_failRequested.Task.IsCompleted) + logger.LogError($"Fail requested again with {exception}"); + else + _failRequested.TrySetResult(exception); + } + async Task ReadOne(CancellationToken token) { byte[] buff = new byte[4000]; @@ -172,7 +181,8 @@ protected async Task ConnectWithMainLoops( ReadOne(linkedCts.Token), _newSendTaskAvailable.Task, _clientInitiatedClose.Task, - _shutdownRequested.Task + _shutdownRequested.Task, + _failRequested.Task }; // In case we had a Send called already @@ -192,6 +202,9 @@ protected async Task ConnectWithMainLoops( if (_clientInitiatedClose.Task.IsCompleted) return (RunLoopStopReason.ClientInitiatedClose, new TaskCanceledException("Proxy or the browser closed the connection")); + if (_failRequested.Task.IsCompleted) + return (RunLoopStopReason.Exception, _failRequested.Task.Result); + if (_newSendTaskAvailable.Task.IsCompleted) { // Just needed to wake up. the new task has already diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs index 517c46dd56334..41684ca057475 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs @@ -30,6 +30,7 @@ class Inspector public const string READY = "ready"; public CancellationToken Token { get; } public InspectorClient Client { get; } + public bool DetectAndFailOnAssertions { get; set; } = true; private CancellationTokenSource _cancellationTokenSource; @@ -179,8 +180,17 @@ async Task OnMessage(string method, JObject args, CancellationToken token) NotifyOf(READY, args); break; case "Runtime.consoleAPICalled": - _logger.LogInformation(FormatConsoleAPICalled(args)); + { + string line = FormatConsoleAPICalled(args); + _logger.LogInformation(line); + if (DetectAndFailOnAssertions && line.Contains("console.error: * Assertion at")) + { + args["__forMethod"] = method; + Client.Fail(new ArgumentException($"Assertion detected in the messages: {line}{Environment.NewLine}{args}")); + return; + } break; + } case "Inspector.detached": case "Inspector.targetCrashed": case "Inspector.targetReloadedAfterCrash": From fdc0d16020874da55372211388303f51e4b852eb Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 22 Feb 2022 22:23:49 +0000 Subject: [PATCH 02/11] [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` to add the ops, and then read those in the main loop, and add to the *local* `pending_ops` list. --- .../BrowserDebugProxy/DevToolsProxy.cs | 76 ++++++++++++++----- .../debugger/BrowserDebugProxy/MonoProxy.cs | 12 +-- 2 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs index 28033d58dad95..785466a3c7788 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs @@ -8,6 +8,7 @@ using System.Net.WebSockets; using System.Text; using System.Threading; +using System.Threading.Channels; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Newtonsoft.Json; @@ -24,7 +25,8 @@ internal class DevToolsProxy private ClientWebSocket browser; private WebSocket ide; private int next_cmd_id; - private List pending_ops = new List(); + private readonly ChannelWriter _channelWriter; + private readonly ChannelReader _channelReader; private List queues = new List(); protected readonly ILogger logger; @@ -32,6 +34,10 @@ internal class DevToolsProxy public DevToolsProxy(ILoggerFactory loggerFactory) { logger = loggerFactory.CreateLogger(); + + var channel = Channel.CreateUnbounded(new UnboundedChannelOptions { SingleReader = true }); + _channelWriter = channel.Writer; + _channelReader = channel.Reader; } protected virtual Task AcceptEvent(SessionId sessionId, string method, JObject args, CancellationToken token) @@ -93,7 +99,7 @@ private DevToolsQueue GetQueueForTask(Task task) return queues.FirstOrDefault(q => q.CurrentSend == task); } - private void Send(WebSocket to, JObject o, CancellationToken token) + private async Task Send(WebSocket to, JObject o, CancellationToken token) { string sender = browser == to ? "Send-browser" : "Send-ide"; @@ -105,7 +111,7 @@ private void Send(WebSocket to, JObject o, CancellationToken token) Task task = queue.Send(bytes, token); if (task != null) - pending_ops.Add(task); + await _channelWriter.WriteAsync(task, token); } private async Task OnEvent(SessionId sessionId, string method, JObject args, CancellationToken token) @@ -115,7 +121,7 @@ private async Task OnEvent(SessionId sessionId, string method, JObject args, Can if (!await AcceptEvent(sessionId, method, args, token)) { //logger.LogDebug ("proxy browser: {0}::{1}",method, args); - SendEventInternal(sessionId, method, args, token); + await SendEventInternal(sessionId, method, args, token); } } catch (Exception e) @@ -131,7 +137,7 @@ private async Task OnCommand(MessageId id, string method, JObject args, Cancella if (!await AcceptCommand(id, method, args, token)) { Result res = await SendCommandInternal(id, method, args, token); - SendResponseInternal(id, res, token); + await SendResponseInternal(id, res, token); } } catch (Exception e) @@ -152,7 +158,7 @@ private void OnResponse(MessageId id, Result result) logger.LogError("Cannot respond to command: {id} with result: {result} - command is not pending", id, result); } - private void ProcessBrowserMessage(string msg, CancellationToken token) + private Task ProcessBrowserMessage(string msg, CancellationToken token) { var res = JObject.Parse(msg); @@ -160,23 +166,30 @@ private void ProcessBrowserMessage(string msg, CancellationToken token) Log("protocol", $"browser: {msg}"); if (res["id"] == null) - pending_ops.Add(OnEvent(res.ToObject(), res["method"].Value(), res["params"] as JObject, token)); + { + return OnEvent(res.ToObject(), res["method"].Value(), res["params"] as JObject, token); + } else + { OnResponse(res.ToObject(), Result.FromJson(res)); + return null; + } } - private void ProcessIdeMessage(string msg, CancellationToken token) + private Task ProcessIdeMessage(string msg, CancellationToken token) { Log("protocol", $"ide: {msg}"); if (!string.IsNullOrEmpty(msg)) { var res = JObject.Parse(msg); var id = res.ToObject(); - pending_ops.Add(OnCommand( + return OnCommand( id, res["method"].Value(), - res["params"] as JObject, token)); + res["params"] as JObject, token); } + + return null; } internal async Task SendCommand(SessionId id, string method, JObject args, CancellationToken token) @@ -185,7 +198,7 @@ internal async Task SendCommand(SessionId id, string method, JObject arg return await SendCommandInternal(id, method, args, token); } - private Task SendCommandInternal(SessionId sessionId, string method, JObject args, CancellationToken token) + private async Task SendCommandInternal(SessionId sessionId, string method, JObject args, CancellationToken token) { int id = Interlocked.Increment(ref next_cmd_id); @@ -203,17 +216,17 @@ private Task SendCommandInternal(SessionId sessionId, string method, JOb //Log ("verbose", $"add cmd id {sessionId}-{id}"); pending_cmds[msgId] = tcs; - Send(this.browser, o, token); - return tcs.Task; + await Send(browser, o, token); + return await tcs.Task; } - public void SendEvent(SessionId sessionId, string method, JObject args, CancellationToken token) + public Task SendEvent(SessionId sessionId, string method, JObject args, CancellationToken token) { //Log ("verbose", $"sending event {method}: {args}"); - SendEventInternal(sessionId, method, args, token); + return SendEventInternal(sessionId, method, args, token); } - private void SendEventInternal(SessionId sessionId, string method, JObject args, CancellationToken token) + private Task SendEventInternal(SessionId sessionId, string method, JObject args, CancellationToken token) { var o = JObject.FromObject(new { @@ -223,7 +236,7 @@ private void SendEventInternal(SessionId sessionId, string method, JObject args, if (sessionId.sessionId != null) o["sessionId"] = sessionId.sessionId; - Send(this.ide, o, token); + return Send(ide, o, token); } internal void SendResponse(MessageId id, Result result, CancellationToken token) @@ -231,13 +244,13 @@ internal void SendResponse(MessageId id, Result result, CancellationToken token) SendResponseInternal(id, result, token); } - private void SendResponseInternal(MessageId id, Result result, CancellationToken token) + private Task SendResponseInternal(MessageId id, Result result, CancellationToken token) { JObject o = result.ToJObject(id); if (!result.IsOk) logger.LogError($"sending error response for id: {id} -> {result}"); - Send(this.ide, o, token); + return Send(this.ide, o, token); } // , HttpContext context) @@ -257,10 +270,14 @@ public async Task Run(Uri browserUri, WebSocket ideSocket) Log("verbose", $"DevToolsProxy: Client connected on {browserUri}"); var x = new CancellationTokenSource(); + List pending_ops = new(); + pending_ops.Add(ReadOne(browser, x.Token)); pending_ops.Add(ReadOne(ide, x.Token)); pending_ops.Add(side_exception.Task); pending_ops.Add(client_initiated_close.Task); + Task readerTask = _channelReader.WaitToReadAsync(x.Token).AsTask(); + pending_ops.Add(readerTask); try { @@ -277,6 +294,16 @@ public async Task Run(Uri browserUri, WebSocket ideSocket) break; } + if (readerTask.IsCompleted) + { + while (_channelReader.TryRead(out Task newTask)) + { + pending_ops.Add(newTask); + } + + pending_ops[4] = _channelReader.WaitToReadAsync(x.Token).AsTask(); + } + //logger.LogTrace ("pump {0} {1}", task, pending_ops.IndexOf (task)); if (completedTask == pending_ops[0]) { @@ -284,7 +311,9 @@ public async Task Run(Uri browserUri, WebSocket ideSocket) if (msg != null) { pending_ops[0] = ReadOne(browser, x.Token); //queue next read - ProcessBrowserMessage(msg, x.Token); + Task newTask = ProcessBrowserMessage(msg, x.Token); + if (newTask != null) + pending_ops.Add(newTask); } } else if (completedTask == pending_ops[1]) @@ -293,7 +322,9 @@ public async Task Run(Uri browserUri, WebSocket ideSocket) if (msg != null) { pending_ops[1] = ReadOne(ide, x.Token); //queue next read - ProcessIdeMessage(msg, x.Token); + Task newTask = ProcessIdeMessage(msg, x.Token); + if (newTask != null) + pending_ops.Add(newTask); } } else if (completedTask == pending_ops[2]) @@ -313,10 +344,13 @@ public async Task Run(Uri browserUri, WebSocket ideSocket) } } } + + _channelWriter.Complete(); } catch (Exception e) { Log("error", $"DevToolsProxy::Run: Exception {e}"); + _channelWriter.Complete(e); //throw; } finally diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 8a9d714428cea..eecccca1f90e4 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -139,7 +139,7 @@ protected override async Task AcceptEvent(SessionId sessionId, string meth case "Runtime.executionContextCreated": { - SendEvent(sessionId, method, args, token); + await SendEvent(sessionId, method, args, token); JToken ctx = args?["context"]; var aux_data = ctx?["auxData"] as JObject; int id = ctx["id"].Value(); @@ -986,7 +986,7 @@ private async Task SendCallStack(SessionId sessionId, ExecutionContext con await SendCommand(sessionId, "Debugger.resume", new JObject(), token); return true; } - SendEvent(sessionId, "Debugger.paused", o, token); + await SendEvent(sessionId, "Debugger.paused", o, token); return true; @@ -1103,7 +1103,7 @@ internal async Task LoadSymbolsOnDemand(AssemblyInfo asm, int method foreach (SourceFile source in asm.Sources) { var scriptSource = JObject.FromObject(source.ToScriptSource(context.Id, context.AuxData)); - SendEvent(sessionId, "Debugger.scriptParsed", scriptSource, token); + await SendEvent(sessionId, "Debugger.scriptParsed", scriptSource, token); } return asm.GetMethodByToken(method_token); } @@ -1333,7 +1333,7 @@ private async Task OnSourceFileAdded(SessionId sessionId, SourceFile source, Exe { JObject scriptSource = JObject.FromObject(source.ToScriptSource(context.Id, context.AuxData)); Log("debug", $"sending {source.Url} {context.Id} {sessionId.sessionId}"); - SendEvent(sessionId, "Debugger.scriptParsed", scriptSource, token); + await SendEvent(sessionId, "Debugger.scriptParsed", scriptSource, token); foreach (var req in context.BreakpointRequests.Values) { @@ -1412,7 +1412,7 @@ private async Task RuntimeReady(SessionId sessionId, CancellationTok DebugStore store = await LoadStore(sessionId, token); context.ready.SetResult(store); - SendEvent(sessionId, "Mono.runtimeReady", new JObject(), token); + await SendEvent(sessionId, "Mono.runtimeReady", new JObject(), token); context.SdbAgent.ResetStore(store); return store; } @@ -1502,7 +1502,7 @@ private async Task SetBreakpoint(SessionId sessionId, DebugStore store, Breakpoi }; if (sendResolvedEvent) - SendEvent(sessionId, "Debugger.breakpointResolved", JObject.FromObject(resolvedLocation), token); + await SendEvent(sessionId, "Debugger.breakpointResolved", JObject.FromObject(resolvedLocation), token); } req.Locations.AddRange(breakpoints); From 12084e49275a0286399bb8600caf065ec4948134 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 22 Feb 2022 23:45:23 +0000 Subject: [PATCH 03/11] [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 --- src/libraries/sendtohelix-wasm.targets | 30 +++----------- src/mono/wasm/BrowsersForTesting.props | 28 +++++++++++++ src/mono/wasm/README.md | 2 + .../DebuggerTestSuite/DebuggerTestBase.cs | 14 ++++++- .../DebuggerTestSuite.csproj | 39 +++++++++++++++++++ 5 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 src/mono/wasm/BrowsersForTesting.props diff --git a/src/libraries/sendtohelix-wasm.targets b/src/libraries/sendtohelix-wasm.targets index ee6267244d3f7..0c80ec4ec1f79 100644 --- a/src/libraries/sendtohelix-wasm.targets +++ b/src/libraries/sendtohelix-wasm.targets @@ -38,8 +38,8 @@ - - + + @@ -60,8 +60,8 @@ - - + + @@ -96,6 +96,8 @@ + + $([MSBuild]::NormalizeDirectory('$(RepoRoot)', 'src', 'mono', 'wasm', 'build')) @@ -106,26 +108,6 @@ $(Scenario)- - - - 929513 - https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chrome-linux.zip - https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chromedriver_linux64.zip - - - 929513 - https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chrome-win.zip - https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chromedriver_win32.zip - - diff --git a/src/mono/wasm/BrowsersForTesting.props b/src/mono/wasm/BrowsersForTesting.props new file mode 100644 index 0000000000000..eb0c1476f0032 --- /dev/null +++ b/src/mono/wasm/BrowsersForTesting.props @@ -0,0 +1,28 @@ + + + + 929513 + https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chrome-linux.zip + https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/$(ChromiumRevision)/chromedriver_linux64.zip + chrome-linux + chromedriver_linux64 + chrome + + + + 929513 + https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chrome-win.zip + https://storage.googleapis.com/chromium-browser-snapshots/Win_x64/$(ChromiumRevision)/chromedriver_win32.zip + chrome-win + chromedriver_win32 + chrome.exe + + diff --git a/src/mono/wasm/README.md b/src/mono/wasm/README.md index 86e7b56912718..2fd07e682ad72 100644 --- a/src/mono/wasm/README.md +++ b/src/mono/wasm/README.md @@ -143,6 +143,8 @@ To run a test with `FooBar` in the name: 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. +- Chrome can be installed for testing by setting `InstallChromeForDebuggerTests=true` when building the tests. + ## Run samples The samples in `src/mono/sample/wasm` can be build and run like this: diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs index a4104328b982b..bad24f74a8906 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs @@ -82,14 +82,17 @@ static string GetChromePath() { chrome_path = FindChromePath(); if (!string.IsNullOrEmpty(chrome_path)) + { + chrome_path = Path.GetFullPath(chrome_path); Console.WriteLine ($"** Using chrome from {chrome_path}"); + } else throw new Exception("Could not find an installed Chrome to use"); } return chrome_path; - string FindChromePath() + static string FindChromePath() { string chrome_path_env_var = Environment.GetEnvironmentVariable("CHROME_PATH_FOR_DEBUGGER_TESTS"); if (!string.IsNullOrEmpty(chrome_path_env_var)) @@ -100,6 +103,15 @@ string FindChromePath() Console.WriteLine ($"warning: Could not find CHROME_PATH_FOR_DEBUGGER_TESTS={chrome_path_env_var}"); } + // Look for a chrome installed in artifacts, for local runs + string baseDir = Path.Combine(Path.GetDirectoryName(typeof(DebuggerTestBase).Assembly.Location), "..", ".."); + string path = Path.Combine(baseDir, "chrome", "chrome-linux", "chrome"); + if (File.Exists(path)) + return path; + path = Path.Combine(baseDir, "chrome", "chrome-win", "chrome.exe"); + if (File.Exists(path)) + return path; + return PROBE_LIST.FirstOrDefault(p => File.Exists(p)); } } diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj index 7e6cc501b2449..771a10beb8a65 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj +++ b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestSuite.csproj @@ -5,6 +5,17 @@ true false true + $(MSBuildThisFileDirectory)..\..\BrowsersForTesting.props + windows + true + + + + + + $(ArtifactsBinDir)DebuggerTestSuite\chrome\ + $(ArtifactsBinDir)DebuggerTestSuite\ + $(ChromeStampDir).install-chrome-$(ChromiumRevision).stamp @@ -33,4 +44,32 @@ + + + + + <_StampFile Include="$(ChromeStampDir).install-chrome*.stamp" /> + + + + + + + + + + + + <_ChromeBinaryPath>$([MSBuild]::NormalizePath($(ChromeDir), $(ChromiumDirName), $(ChromiumBinaryName))) + + + + + + + + From d9bf56718032a2756838a494a64db16e860d130b Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Wed, 23 Feb 2022 00:16:13 +0000 Subject: [PATCH 04/11] [wasm][debugger] Disable tests failing with runtime assertions `DebuggerTests.EvaluateOnCallFrameTests.EvaluateSimpleMethodCallsError`: https://github.com/dotnet/runtime/issues/65744 `DebuggerTests.ArrayTests.InvalidArrayId`: https://github.com/dotnet/runtime/issues/65742 --- src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs | 2 ++ .../wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs index 438eceea378d3..a9e87746bd89a 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs @@ -552,6 +552,8 @@ await CompareObjectPropertiesFor(frame_locals, "this", } [Fact] + [Trait("Category", "windows-failing")] // https://github.com/dotnet/runtime/issues/65742 + [Trait("Category", "linux-failing")] // https://github.com/dotnet/runtime/issues/65742 public async Task InvalidArrayId() => await CheckInspectLocalsAtBreakpointSite( "DebuggerTests.Container", "PlaceholderMethod", 1, "PlaceholderMethod", "window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.ArrayTestsClass:ObjectArrayMembers'); }, 1);", diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index c38596af1e5c6..946dcbd394b65 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -494,6 +494,8 @@ async Task EvaluateOnCallFrameFail(string call_frame_id, params (string expressi [Fact] + [Trait("Category", "windows-failing")] // https://github.com/dotnet/runtime/issues/65744 + [Trait("Category", "linux-failing")] // https://github.com/dotnet/runtime/issues/65744 public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAtBreakpointSite( "DebuggerTests.EvaluateMethodTestsClass.TestEvaluate", "run", 9, "run", "window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.EvaluateMethodTestsClass:EvaluateMethods'); })", From 9422921fbbe4ded421531bf9aeb42e3d1d2077cb Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Wed, 23 Feb 2022 02:29:10 +0000 Subject: [PATCH 05/11] [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/debugger/BrowserDebugProxy/EvaluateExpression.cs | 4 ++-- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs index 213a50ae1ce7e..de3c0716cfa50 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs @@ -396,7 +396,7 @@ internal static async Task CompileAndRunTheExpression(string expression string.Join("\n", findVarNMethodCall.variableDefinitions) + "\nreturn " + syntaxTree.ToString()); var state = await newScript.RunAsync(cancellationToken: token); - return JObject.FromObject(ConvertCSharpToJSType(state.ReturnValue, state.ReturnValue.GetType())); + return JObject.FromObject(ConvertCSharpToJSType(state.ReturnValue, state.ReturnValue?.GetType())); } catch (CompilationErrorException cee) { @@ -419,7 +419,7 @@ internal static async Task CompileAndRunTheExpression(string expression private static object ConvertCSharpToJSType(object v, Type type) { if (v == null) - return new { type = "object", subtype = "null", className = type.ToString(), description = type.ToString() }; + return new { type = "object", subtype = "null", className = type?.ToString(), description = type?.ToString() }; if (v is string s) return new { type = "string", value = s, description = s }; if (NumericTypes.Contains(v.GetType())) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index eecccca1f90e4..71fa7a3f10cb6 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -805,12 +805,15 @@ private async Task EvaluateCondition(SessionId sessionId, ExecutionContext if (retValue?["value"]?.Value() == true) return true; } - else if (retValue?["value"]?.Type != JTokenType.Null) + else if (retValue?["value"] != null && // null object, missing value + retValue?["value"]?.Type != JTokenType.Null) + { return true; + } } catch (Exception e) { - Log("info", $"Unable evaluate conditional breakpoint: {e} condition:{condition}"); + Log("info", $"Unable to evaluate breakpoint condition {condition}: {e}"); bp.ConditionAlreadyEvaluatedWithError = true; return false; } From fd29e30375c10040fbf4a70936c8350cb1ded5d3 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Wed, 23 Feb 2022 02:51:41 +0000 Subject: [PATCH 06/11] [wasm] Improve default message for ReturnAsErrorException, .. since we seem to be not catching them as intended in many places. --- .../wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs index de3c0716cfa50..dc9296b5f6b08 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs @@ -443,10 +443,11 @@ public Result Error } set { } } - public ReturnAsErrorException(JObject error) + public ReturnAsErrorException(JObject error) : base(error.ToString()) => Error = Result.Err(error); public ReturnAsErrorException(string message, string className) + : base($"[{className}] {message}") { var result = new { @@ -466,5 +467,7 @@ public ReturnAsErrorException(string message, string className) } })); } + + public override string ToString() => $"Error object: {Error}. {base.ToString()}"; } } From 391c1f044889f709cffd982c7f06446277e9e26c Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Wed, 23 Feb 2022 02:52:20 +0000 Subject: [PATCH 07/11] [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/BrowserDebugProxy/MonoProxy.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 71fa7a3f10cb6..e8aaac3443663 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -51,7 +51,7 @@ private bool UpdateContext(SessionId sessionId, ExecutionContext executionContex internal Task SendMonoCommand(SessionId id, MonoCommands cmd, CancellationToken token) => SendCommand(id, "Runtime.evaluate", JObject.FromObject(cmd), token); - internal void SendLog(SessionId sessionId, string message, CancellationToken token) + internal void SendLog(SessionId sessionId, string message, CancellationToken token, string type = "warning") { if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) return; @@ -68,7 +68,7 @@ internal void SendLog(SessionId sessionId, string message, CancellationToken tok SendEvent(id, "Log.entryAdded", o, token);*/ var o = JObject.FromObject(new { - type = "warning", + type, args = new JArray(JObject.FromObject(new { type = "string", @@ -811,11 +811,16 @@ private async Task EvaluateCondition(SessionId sessionId, ExecutionContext return true; } } + catch (ReturnAsErrorException raee) + { + logger.LogDebug($"Unable to evaluate breakpoint condition '{condition}': {raee}"); + SendLog(sessionId, raee.Message, token, type: "error"); + bp.ConditionAlreadyEvaluatedWithError = true; + } catch (Exception e) { - Log("info", $"Unable to evaluate breakpoint condition {condition}: {e}"); + Log("info", $"Unable to evaluate breakpoint condition '{condition}': {e}"); bp.ConditionAlreadyEvaluatedWithError = true; - return false; } return false; } From d58e0762b7625ba412ac17745bcb816a0480749b Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 22 Feb 2022 19:53:04 +0000 Subject: [PATCH 08/11] [wasm][debugger] Allow setting proxy port for BrowserDebugHost .. with `--proxy-port=`. Fixes https://github.com/dotnet/runtime/issues/65209 . --- src/mono/wasm/debugger/BrowserDebugHost/Program.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugHost/Program.cs b/src/mono/wasm/debugger/BrowserDebugHost/Program.cs index 6c4a79dfeb238..52380a079997a 100644 --- a/src/mono/wasm/debugger/BrowserDebugHost/Program.cs +++ b/src/mono/wasm/debugger/BrowserDebugHost/Program.cs @@ -3,13 +3,11 @@ using System; using System.IO; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; + +#nullable enable namespace Microsoft.WebAssembly.Diagnostics { @@ -24,6 +22,11 @@ public class Program { public static void Main(string[] args) { + IConfigurationRoot config = new ConfigurationBuilder().AddCommandLine(args).Build(); + int proxyPort = 0; + if (config["proxy-port"] is not null && int.TryParse(config["proxy-port"], out int port)) + proxyPort = port; + IWebHost host = new WebHostBuilder() .UseSetting("UseIISIntegration", false.ToString()) .UseKestrel() @@ -33,7 +36,7 @@ public static void Main(string[] args) { config.AddCommandLine(args); }) - .UseUrls("http://127.0.0.1:0") + .UseUrls($"http://127.0.0.1:{proxyPort}") .Build(); host.Run(); From 8105e4286658b96d3f9eb351dd2b5d1cef0fd2f6 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 22 Feb 2022 23:41:39 -0500 Subject: [PATCH 09/11] [wasm][debugger] Handle errors in getting value for locals Essentially, catch, and skip. --- .../wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 793d3c980b9f1..f30d42e69de14 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -2121,8 +2121,16 @@ public async Task StackFrameGetValues(MethodInfoWithDebugInformation met using var localsDebuggerCmdReader = await SendDebuggerAgentCommand(CmdFrame.GetValues, commandParamsWriter, token); foreach (var var in varIds) { - var var_json = await CreateJObjectForVariableValue(localsDebuggerCmdReader, var.Name, false, -1, false, token); - locals.Add(var_json); + try + { + var var_json = await CreateJObjectForVariableValue(localsDebuggerCmdReader, var.Name, false, -1, false, token); + locals.Add(var_json); + } + catch (Exception ex) + { + logger.LogDebug($"Failed to create value for local var {var}: {ex}"); + continue; + } } if (!method.Info.IsStatic()) { From 30be2429a3192ccccb66f236a0e9f8c6536ebaa0 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Wed, 23 Feb 2022 00:21:26 -0500 Subject: [PATCH 10/11] message cleanup --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index e8aaac3443663..02110593215d2 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -814,7 +814,7 @@ private async Task EvaluateCondition(SessionId sessionId, ExecutionContext catch (ReturnAsErrorException raee) { logger.LogDebug($"Unable to evaluate breakpoint condition '{condition}': {raee}"); - SendLog(sessionId, raee.Message, token, type: "error"); + SendLog(sessionId, $"Unable to evaluate breakpoint condition '{condition}': {raee.Message}", token, type: "error"); bp.ConditionAlreadyEvaluatedWithError = true; } catch (Exception e) From b8da0606108d368f650936bcf7f06b2e75f01081 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Wed, 23 Feb 2022 16:08:58 -0500 Subject: [PATCH 11/11] remove trailing space --- src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs index 750de6808bd08..567b1d7837e66 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsProxy.cs @@ -35,7 +35,7 @@ public DevToolsProxy(ILoggerFactory loggerFactory, string loggerId) { string loggerSuffix = string.IsNullOrEmpty(loggerId) ? string.Empty : $"-{loggerId}"; logger = loggerFactory.CreateLogger($"{nameof(DevToolsProxy)}{loggerSuffix}"); - + var channel = Channel.CreateUnbounded(new UnboundedChannelOptions { SingleReader = true }); _channelWriter = channel.Writer; _channelReader = channel.Reader;