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][debugger] Protecting send message to debugger if it's disabled #61927

Merged
merged 1 commit into from
Nov 30, 2021
Merged

[wasm][debugger] Protecting send message to debugger if it's disabled #61927

merged 1 commit into from
Nov 30, 2021

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Nov 22, 2021 •
edited
Loading

This PR fixes this issue: #61917

Avoid sending messages to debugger if the debugger is not enabled because it's a release app.

@ghost
Copy link

ghost commented Nov 22, 2021

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

Issue Details

This PR fixes this issue: #61917
But the correct fix should be to link the stub version of the debugger and don't execute anything on mini-wasm-debugger.c at all.
@lewing @radekdoulik

Author: thaystg
Assignees: -
Labels:

area-Debugger-mono
Milestone: -

@thaystg thaystg requested a review from lewing November 22, 2021 18:07
@radical radical added the arch-wasm WebAssembly architecture label Nov 22, 2021
@ghost
Copy link

ghost commented Nov 22, 2021

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

Issue Details

This PR fixes this issue: #61917
But the correct fix should be to link the stub version of the debugger and don't execute anything on mini-wasm-debugger.c at all.
@lewing @radekdoulik

Author: thaystg
Assignees: -
Labels:

arch-wasm, area-Debugger-mono
Milestone: -

@thaystg
Copy link
Member Author

thaystg commented Nov 22, 2021

@lewing should we backport this?

@lewing
Copy link
Member

lewing commented Nov 23, 2021

We need the debugger to work in the release build that we ship in the runtime pack because without the workload that is the only runtime a user has access to.

@thaystg thaystg merged commit 35f01d7 into dotnet:main Nov 30, 2021
@thaystg
Copy link
Member Author

thaystg commented Nov 30, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1522689284

@github-actions github-actions bot mentioned this pull request Nov 30, 2021
Merged
This was referenced Nov 30, 2021
Closed
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pavelsavara pavelsavara pavelsavara left review comments

@radical radical radical approved these changes

@radekdoulik radekdoulik radekdoulik approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

@lewing lewing Awaiting requested review from lewing

Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture area-Debugger-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thaystg @lewing @radical @pavelsavara @radekdoulik