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

duplicated top stackframe with old debug adapters #28808

Closed
weinand opened this issue Jun 15, 2017 · 7 comments
Closed

duplicated top stackframe with old debug adapters #28808

weinand opened this issue Jun 15, 2017 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Jun 15, 2017

VSCode Version: Insider 1.14.0

  • debug simple program with mono-debug

Observe: the stack frame is duplicated:

2017-06-15_13-06-08

First VS Code asks for stackframes with startFrame = 0 and levels = 1.
Then mono-debug returns all frames (which happens to be exactly one).
Then VS Code asks for stackframes with startFrame = 0 and levels = 20 (probably because the first result did not return the "totalFrames" attribute and VS Code falls back to the old behavior).
Then mono-debug returns all frames again and VS Code adds them to the existing one instead of replacing the existing one.

Then I tried a program where the stacktrace has one additional level but this doesn't make a difference:

2017-06-15_13-18-13

@vscodebot vscodebot bot added the insiders label Jun 15, 2017
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues bug Issue identified by VS Code Team member as probable bug and removed insiders labels Jun 15, 2017
@weinand weinand added this to the June 2017 milestone Jun 15, 2017
@weinand weinand changed the title duplicated stackframes with old debug adapters duplicated top stackframe with old debug adapters Jun 15, 2017
@weinand
Copy link
Contributor Author

weinand commented Jun 18, 2017

VS Code 1.13 uses the following heuristic to decide whether a DA supports loading stack frames ranges: it request a single frame (startFrame: 0, levels: 1). If the DA returns more frames, VS Code assumes that the DA does not support ranges.

This heuristic fails if there is only one frame available or if the DA honours 'levels' but not 'startFrame'. This latter case is the reason for the mono-debug problem.

Since we have not found a way to fix the heuristic in a robust way, we've decided to require that DAs opt-into the delayed loading of stack frames by introducing a new capability supportsDelayedStackTraceLoading.

@roblourens please enable the supportsDelayedStackTraceLoading for the inspector protocol.

roblourens added a commit to microsoft/vscode-chrome-debug-core that referenced this issue Jun 19, 2017
@weinand weinand added the verified Verification succeeded label Jun 20, 2017
@isidorn
Copy link
Contributor

isidorn commented Jun 21, 2017

@delmyers @gregg-miskelly @MSLaguana @DanTup @devoncarew @WebFreak001 @roblourens @andysterland @hbenl @lukaszunity @svaarala @ramya-rao-a @vadimcn @felixfbecker @daviwil @rkeithhill @DonJayamanne @MSLaguana @rebornix in order to benefit from delayed stack trace loading your adapters now need to explicitly specify this capability supportsDelayedStackTraceLoading.

This is the original feature request, however we have now introduced a capability as @weinand mentions in the comment above. The capability is available in the preview version of the adapter npm module.

@DanTup
Copy link
Contributor

DanTup commented Jun 23, 2017

@ishanarora @weinand Do we need to target a specific Code version to use supportsDelayedStackTraceLoading and if so, is it one that's already shipped or is this currently only usable in Insiders?

@weinand
Copy link
Contributor Author

weinand commented Jun 23, 2017

VS Code will make use of supportsDelayedStackTraceLoading in the June release (that's the reason for the June 2017 "milestone" on this issue) or in current Insiders.
But as always you can use it in your debugAdapter already and it will just be ignored by older versions of VS Code.

@DanTup
Copy link
Contributor

DanTup commented Jun 23, 2017

@weinand Do we need an updated vscode-debugprotocol to get supportsDelayedStackTraceLoading? The latest one on npm doesn't have these changes (apologies if this is a silly question; I haven't had to use anything that wasn't released yet). Thanks!

@weinand
Copy link
Contributor Author

weinand commented Jun 23, 2017

As Isi said: "The capability is available in the preview version of the adapter npm module." So either load the "pre" version (like I do for node-debug: https://github.com/Microsoft/vscode-node-debug/blob/master/package.json#L31) or wait until I've released the final version of the npm module Wednesday next week.

@DanTup
Copy link
Contributor

DanTup commented Jun 23, 2017

@weinand Apologies; I missed that but also didn't realise that the npm page doesn't show that there is a pre-release (it shows "1.20.0 is the latest of 97 releases" with seemingly no way to view a full list or pre-releases - I'm used to NuGet!). Everything's clear now, thanks! 👍

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants