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.

Sign up for GitHub

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

Jump to bottom

[wasm-ep] Configure the Diagnostic Server using DOTNET_DiagnosticPorts #73011

Closed
lambdageek opened this issue Jul 28, 2022 · 3 comments · Fixed by #73370
Closed

[wasm-ep] Configure the Diagnostic Server using DOTNET_DiagnosticPorts #73011

lambdageek opened this issue Jul 28, 2022 · 3 comments · Fixed by #73370
Assignees
@lambdageek
Labels
arch-wasm WebAssembly architecture area-Diagnostics-mono
Milestone
7.0.0

Comments

@lambdageek
Copy link
Member

In order to make diagnostics work with Blazor, we need to provide a mechanism that works with their WebAssebly startup sequence (near here: MonoPlatform.ts). Using a MonoConfig diagnostic_options section doesn't work because they don't have this object. Additionally the current call to mono_wasm_init_diagnostics is in mono_wasm_before_user_runtime_initialized which is not called for Blazor.

The other constraint is that mono_wasm_init_diagnostics is async (because it waits for the DS thread to start), so we need to call it during an async part of runtime startup.

So the task is:

  1. Use the DOTNET_DiagnosticPorts environment variable to configure the diagnostic server - similar to desktop. Setting it to a value like ws://127.0.0.1:8088/diagnostics,connect,suspend will cause the runtime to connect to the given WebSocket URL and suspend until a diagnostic tool sends a resume command. We will only support connect, not listen (which is not meaningful for a websocket). We will support suspend (the default) and nosuspend
  2. Blazor can setenv DOTENT_DiagnosticPorts from their onRuntimeInitialized callback
  3. Move the call to await mono_wasm_init_diagnostics() to mono_wasm_after_user_runtime_initialized which is async and runs after onRuntimeInitialized but before we call Blazor's postRun (which calls MONO.mono_wasm_load_runtime which is a sync function that actually begins running our C code - ie we must have the diagnostic server running by this point)
@lambdageek lambdageek added arch-wasm WebAssembly architecture area-Diagnostics-mono labels Jul 28, 2022
@lambdageek lambdageek added this to the 7.0.0 milestone Jul 28, 2022
@lambdageek lambdageek self-assigned this Jul 28, 2022
@ghost
Copy link

ghost commented Jul 28, 2022

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

Issue Details

In order to make diagnostics work with Blazor, we need to provide a mechanism that works with their WebAssebly startup sequence (near here: MonoPlatform.ts). Using a MonoConfig diagnostic_options section doesn't work because they don't have this object. Additionally the current call to mono_wasm_init_diagnostics is in mono_wasm_before_user_runtime_initialized which is not called for Blazor.

The other constraint is that mono_wasm_init_diagnostics is async (because it waits for the DS thread to start), so we need to call it during an async part of runtime startup.

So the task is:

  1. Use the DOTNET_DiagnosticPorts environment variable to configure the diagnostic server - similar to desktop. Setting it to a value like ws://127.0.0.1:8088/diagnostics,connect,suspend will cause the runtime to connect to the given WebSocket URL and suspend until a diagnostic tool sends a resume command. We will only support connect, not listen (which is not meaningful for a websocket). We will support suspend (the default) and nosuspend
  2. Blazor can setenv DOTENT_DiagnosticPorts from their onRuntimeInitialized callback
  3. Move the call to await mono_wasm_init_diagnostics() to mono_wasm_after_user_runtime_initialized which is async and runs after onRuntimeInitialized but before we call Blazor's postRun (which calls MONO.mono_wasm_load_runtime which is a sync function that actually begins running our C code - ie we must have the diagnostic server running by this point)
Author: lambdageek
Assignees: lambdageek
Labels:

arch-wasm, area-Diagnostics-mono
Milestone: 7.0.0

@lewing
Copy link
Member

lewing commented Jul 28, 2022

cc @radical @javiercn

@lambdageek lambdageek mentioned this issue Jul 28, 2022
Closed
@lambdageek lambdageek mentioned this issue Aug 4, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 4, 2022
@lambdageek
Copy link
Member Author

Turns out we have to init diagnostics in two differnet places because blazor and non-blazor startup will set environment variables in two different codepaths

  • for Blazor, init diagnostics after their onRuntimeInitalized sets env variables, but before their postRun callback (which calls mono_wasm_load_runtime)
  • for non-Blazor, init diagnostics in _apply_configuration_from_args - which sets environment variables - and which is called from mono_wasm_before_user_runtime_initialized just before mono_wasm_load_runtime

lambdageek added a commit to lambdageek/runtime that referenced this issue Aug 7, 2022
@lambdageek
[wasm-ep] Use DOTNET_DiagnosticPorts to configure Diagnostic Server
895861d 
Parse it the same way that the C code does:

```
   <uri>[,<connect|listen>][,<suspend|nosuspend>]
```

- uri should be a websocket uri
- listen is not supported as it doesn't make sense with a WebSocket
- connect is the default if omitted
- suspend is the default if omitted

---

Additionally, move `mono_wasm_diagnostics_init` to later in the
startup flow.  This gives Blazor a chance to set
DOTNET_DiagnosticPorts from their `onRuntimeInitialized` callback.

Fixes dotnet#73011
lambdageek added a commit that referenced this issue Aug 8, 2022
@lambdageek
[wasm-ep] Use DOTNET_DiagnosticPorts to configure Diagnostic Server (#…
89db9c0 
…73370)

* [wasm-ep] Use DOTNET_DiagnosticPorts to configure Diagnostic Server

Parse it the same way that the C code does:

```
   <uri>[,<connect|listen>][,<suspend|nosuspend>]
```

- uri should be a websocket uri
- listen is not supported as it doesn't make sense with a WebSocket
- connect is the default if omitted
- suspend is the default if omitted

---

Additionally, move `mono_wasm_diagnostics_init` to later in the
startup flow.  This gives Blazor a chance to set
DOTNET_DiagnosticPorts from their `onRuntimeInitialized` callback.

Fixes #73011

* Initialize diagnostic server in different places for Blazor and non-Blazor

It has to be after environment variables are set, but before
mono_wasm_load_runtime is called.

There is no good place that's common to both startup paths. Try it on
both.  Use a flag to make diagnostics initialization run at most
once

* update browser-eventpipe sample to use DOTNET_DiagnosticPorts

* remove unused imports
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@lambdageek lambdageek

Labels
arch-wasm WebAssembly architecture area-Diagnostics-mono
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

[wasm-ep] Use DOTNET_DiagnosticPorts to configure Diagnostic Server lambdageek/runtime
2 participants
@lewing @lambdageek