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

[mono][wasm] Initialize debugger_tls_id on WASM as well. #66560

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Mar 13, 2022

Some code paths are using mono_native_tls_get_value () instead of GET_DEBUGGER_TLS ().

Some code paths are using mono_native_tls_get_value () instead of GET_DEBUGGER_TLS ().
@ghost
Copy link

ghost commented Mar 13, 2022

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Some code paths are using mono_native_tls_get_value () instead of GET_DEBUGGER_TLS ().

Author: vargaz
Assignees: vargaz
Labels:

area-Debugger-mono

Milestone: -

@vargaz vargaz requested review from thaystg and lewing and removed request for marek-safar March 13, 2022 22:34
@vargaz
Copy link
Contributor Author

vargaz commented Mar 13, 2022

Some comments:

  • the two init functions should be merged to reduce the chance of similar errors
  • maybe use mono_native_tls_get_value () on wasm as well, its not really perf critical and will be needed later for threading support.
  • this code gets executed in Release mode as well, is that ok ?

@vargaz
Copy link
Contributor Author

vargaz commented Mar 13, 2022

This was causing (some) of the 3.1.x crashes.

@vargaz
Copy link
Contributor Author

vargaz commented Mar 13, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@thaystg
Copy link
Member

thaystg commented Mar 13, 2022

I think it's okay to initialize it, I would like to know which code paths were causing the crashes on 3.1.x, because as far as I remember when I implemented it, the code paths that are being used when debugging on wasm are correctly using the GET_DEBUGGER_TLS.

@vargaz
Copy link
Contributor Author

vargaz commented Mar 13, 2022

debugger_agent_begin_exception_filter () for example. Having two ways to access this seems error prone.

@thaystg
Copy link
Member

thaystg commented Mar 13, 2022

Yeah, I agree to use "mono_native_tls_get_value () on wasm as well, its not really perf critical and will be needed later for threading support", as you suggested above.
I will add it in my next tasks.

@vargaz vargaz merged commit accf6fb into dotnet:main Mar 14, 2022
@vargaz vargaz deleted the fix-wasm-dbg branch March 14, 2022 00:52
@lewing
Copy link
Member

lewing commented Mar 14, 2022

cc @kg

@kg
Copy link
Member

kg commented Mar 14, 2022

I think this works already, but I'll keep it in mind if things break once I get back to eventpipe

@ghost ghost locked as resolved and limited conversation to collaborators Apr 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants