Skip to content

[wasm][debugger] Move the breakpoint line after applying changes - #64359

Merged
thaystg merged 4 commits into
dotnet:mainfrom
thaystg:add_test_to_apply_updates
Jan 28, 2022
Merged

[wasm][debugger] Move the breakpoint line after applying changes#64359
thaystg merged 4 commits into
dotnet:mainfrom
thaystg:add_test_to_apply_updates

Conversation

@thaystg

@thaystgthaystg commented Jan 26, 2022

Copy link
Copy Markdown
Member

When hot-reloading code with new lines added, VS automatically updates the breakpoints by sending Remove/SetBreakpoint commands.

Add a test case for the reload, with simulated VS part.

… to the next line as it's done by VS will work.
@ghost

Copy link
Copy Markdown

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

Issue Details

Add test case to prove that changing a line and moving the breakpoint to the next line as it's done by VS will work.

Author:thaystg
Assignees:thaystg
Labels:

area-Debugger-mono

Milestone:-

@radicalradical added the arch-wasm WebAssembly architecture label Jan 27, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details

Add test case to prove that changing a line and moving the breakpoint to the next line as it's done by VS will work.

Author:thaystg
Assignees:thaystg
Labels:

arch-wasm, area-Debugger-mono

Milestone:-

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@thaystg
thaystg merged commit 1092617 into dotnet:mainJan 28, 2022

@lewinglewing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine but I think we should review how we're handling the queued breakpoints and how they get applied. We should also walk through what events are generated when a delta is applied there are some potential complexities with the ordering there.

@thaystg

Copy link
Copy Markdown
MemberAuthor

We could decide to: have a list of all breakpoints that are not bind because the line is not valid, and try to bind them again after receiving an apply changes. I'm not sure if this makes sense. I will do more tests in a console app running on coreclr. Because each time that we apply the updates we would need to loop in this list of invalid breakpoints and try to set them again.

The events generated when delta is applied are okay I think:

  • debugger-agent receives the update and after it's applied it sends a message to debugger-libs/browserDebugProxy to apply the changes on the debugger side, like updating offsets and adding new variables, and then we look at all the breakpoints for that method and get the new offset and set them again.

@ghostghost locked as resolved and limited conversation to collaborators Feb 27, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-Debugger-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@thaystg@radical@lewing