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][debugger] Proxy but don't process events from worker sessions #57974

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

lewing
Copy link
Member

@lewing lewing commented Aug 23, 2021

Should Fix #57949

@thaystg can you see if you can construct a test for this?

@ghost
Copy link

ghost commented Aug 23, 2021

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

Issue Details

Should Fix #57949

@thaystg can you see if you can construct a test for this?

Author: lewing
Assignees: -
Labels:

area-Debugger-mono

Milestone: -

thaystg
thaystg previously approved these changes Aug 23, 2021
@thaystg thaystg dismissed their stale review August 23, 2021 21:45

It's not compiling

@radical
Copy link
Member

radical commented Aug 23, 2021

Isn't the same check needed for Runtime.consoleAPICalled, Target.attachedToTarget, and Target.targetDestroyed?

@lewing
Copy link
Member Author

lewing commented Aug 23, 2021

Isn't the same check needed for Runtime.consoleAPICalled, Target.attachedToTarget, and Target.targetDestroyed?

Not exactly, those don't expect the context to exist in the same way:
Runtime.consoleAPICalled only does work when a mono runtime is executing.
Target.AttachedToTarget really does work on those sessions
Target.targetDestroyed will fail to evaluate on the js side but it should be harmless.

@radical
Copy link
Member

radical commented Aug 23, 2021

Isn't the same check needed for Runtime.consoleAPICalled, Target.attachedToTarget, and Target.targetDestroyed?

Not exactly, those don't expect the context to exist in the same way:
Runtime.consoleAPICalled only does work when a mono runtime is executing.

What are the "worker sessions" mentioned in the title? Are these sessions executing while the mono runtime is also executing? If so, then couldn't we receive this message for a non-mono session?

Looking at the code, it's handling two mono [1][2] specific cases, so it won't be a problem with that, IIUC. But if we can receive messages not meant for us, then it might still be a good idea to check in that case, and proxy the message?

@lewing
Copy link
Member Author

lewing commented Aug 23, 2021

Isn't the same check needed for Runtime.consoleAPICalled, Target.attachedToTarget, and Target.targetDestroyed?

Not exactly, those don't expect the context to exist in the same way:
Runtime.consoleAPICalled only does work when a mono runtime is executing.

What are the "worker sessions" mentioned in the title? Are these sessions executing while the mono runtime is also executing? If so, then couldn't we receive this message for a non-mono session?

https://developer.mozilla.org/en-US/docs/Web/API/Worker

Looking at the code, it's handling two mono [1][2] specific cases, so it won't be a problem with that, IIUC. But if we can receive messages not meant for us, then it might still be a good idea to check in that case, and proxy the message?

I'm not sure I follow? In the cases where it isn't from mono it does get proxied.

@lewing lewing merged commit 07b85a7 into dotnet:main Aug 24, 2021
@lewing
Copy link
Member Author

lewing commented Aug 24, 2021

/backport to release/6.0-rc1

@lewing lewing deleted the ignore-worker-events branch August 24, 2021 01:08
@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc1: https://github.com/dotnet/runtime/actions/runs/1160816690

steveisok pushed a commit that referenced this pull request Aug 24, 2021
…57990)

Backport of #57974

Should Fix #57949

Co-authored-by: Larry Ewing <lewing@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2021
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.

VS2022 debuger crashes when reading HTTP Response Object response.Content.ReadAsStringAsync();
3 participants