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

[wasm] Misc Debugger improvements (including tests) #65752

Merged
merged 13 commits into from
Feb 24, 2022

Conversation

radical
Copy link
Member

@radical radical commented Feb 23, 2022

Fixes #65209

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.
- 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
`DebuggerTests.EvaluateOnCallFrameTests.EvaluateSimpleMethodCallsError`: dotnet#65744
`DebuggerTests.ArrayTests.InvalidArrayId`: dotnet#65742
@radical radical added arch-wasm WebAssembly architecture area-Debugger-mono labels Feb 23, 2022
@ghost
Copy link

ghost commented Feb 23, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
Author: radical
Assignees: -
Labels:

arch-wasm, area-Debugger-mono

Milestone: -

@ghost ghost assigned radical Feb 23, 2022
`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
```
.. since we seem to be not catching them as intended in many places.
.. breakpoints.
Catch the proper exception `ReturnAsErrorException`, so we can get the
actual error message. And log that as an error for the user too.
@radical radical changed the title [wasm] Debugger tests improvements [wasm] Misc Debugger improvements (including tests) Feb 23, 2022
@radical
Copy link
Member Author

radical commented Feb 23, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Feb 23, 2022

The failing library test (unrelated) is #65796 .

@radical
Copy link
Member Author

radical commented Feb 24, 2022

Filed two issues for the unrelated failures - #65817, and #65818 .

Other unrelated issues seem to have inaccessible helix logs.

@radical radical merged commit 67d8b96 into dotnet:main Feb 24, 2022
@radical radical deleted the debugger-improvements branch February 24, 2022 01:08
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Debugger-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wasm] allow the debug proxy to startup on a fixed port
2 participants