Variables window collapses all expanded nodes after a Step/Step-Over/Continue

[haneefdm]

Issue Type: Bug

This is a continuation of Issue #25652. Since it is closed for comments, creating a new one here.

I have a way to demonstrate the problem and a fix for it as well. It is not specific to cpptools/MIEngine. I can show the problem with Typescript debugging as well. The problem happens when you go from a stopped->running->stopped transition of a program. Not specific to step/next/continue actions and not specific to a particular debug-adapter/language.

As mentioned in issue #25652, it is true that variableReferences must be preserved by the debug adapter for the state to be preserved. I verified that MIEngine does indeed conform, all references are the same as before. This problem only happens if the elapsed time from a running->stopped transition is more than about a half a second.

When going from a stopped->running transition, the variable window gets a onDidFocusStackFrame event -- but call-stack is empty. In the event handler, an actual tree update is scheduled with a delay of 400ms. If a new stackFrame is available by the time the update function is called, then it has a stack frame and will update the tree nicely. But by the time the update function is called we still don't have a valid stack frame (a running->stopped transition has not occurred yet), then the children are destroyed and the tree becomes empty. Eventually, when a running->stopped transition does occur, it all looks new and the tree gets its default state (the first scope expanded). Lost state.

I have two examples that you can get from https://github.com/haneefdm/cortex-debug-samples
There are two subdirectories of interest:

• var-win-bug-cpp
  see main.cpp for instructions on where to set breakpoints and what to do
• var-win-bug-ts
  see src/main.ts for instructions on where to set breakpoints and what to do

I have a fix in my fork, and I hope someone can review my changes (just one file)

https://github.com/haneefdm/vscode/blob/master/src/vs/workbench/contrib/debug/browser/variablesView.ts

My changes are at the beginning of the VariablesView.constructor() and beginning of VariablesView.renderBody(). The Variable window also behaves differently on different OSes and speed of the machine.

I welcome any other fix, but this is a big problem and it becomes very obvious/painful when debugging remotely or anything complicated with complex data structures.

VS Code version: Code 1.35.1 (c7d83e5, 2019-06-12T14:29:22.216Z)
OS version: Darwin x64 18.6.0

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz (12 x 2600)
GPU Status	2d_canvas: enabled checker_imaging: disabled_off flash_3d: enabled flash_stage3d: enabled flash_stage3d_baseline: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on native_gpu_memory_buffers: enabled rasterization: enabled surface_synchronization: enabled_on video_decode: enabled webgl: enabled webgl2: enabled
Load (avg)	2, 2, 2
Memory (System)	16.00GB (0.03GB free)
Process Argv
Screen Reader	no
VM	0%

Extensions (22)

Extension	Author (truncated)	Version
easy-cpp-projects	ACh	1.7.7
npm-intellisense	chr	1.3.0
arm	dan	0.4.0
vscode-markdownlint	Dav	0.28.0
vscode-eslint	dba	1.9.0
EditorConfig	Edi	0.13.0
vscode-npm-script	eg2	0.3.7
vscode-pull-request-github	Git	0.8.0
csharpextensions	jch	1.3.0
vscode-csharp-snippets	jor	0.3.1
csharpfixformat	Leo	0.0.81
cortex-debug	mar	0.3.1
extension-manifest-editor	ms-	0.1.5
python	ms-	2019.6.22090
cpptools	ms-	0.24.0-insiders3
csharp	ms-	1.20.0
mono-debug	ms-	0.15.8
vscode-typescript-tslint-plugin	ms-	1.2.1
debugger-for-chrome	msj	4.11.6
tcl	sle	0.2.0
python	tht	0.2.3
vscode-icons	vsc	8.8.0

[haneefdm]

CC: @pieandcakes @gregg-miskelly

[isidorn]

@haneefdm Thanks for the detailed analysis and for the potential fix.
Let's first look into the issue: yes, if we do not recieve a stopped event after some time (400ms) we simply clear the tree. You are correct. If a stopped event comes after some time the view state (expand and selection) will be lost.
Looking at your fix: this feels like a hack to me, that we simply hide the tree from the dom. Even though a stopped event might not ever arrive.

How we could potentially fix this:

• Every time we update the tree we would store the last viewState of the tree when we were stopped. By calling this.tree.getViewState()
• When the next stop happens instead of saying tree.upadteChildren we would this.tree.setInput(this.debugService.getViewModel(), previousStoppedViewState)
• When a running state happens we simply tree.updateChildren() to clear the tree

Feel free to open a PR with this appraoch and we can try it out and see if it fixes your issue.
Thank you very much

[haneefdm]

@isidorn, Thanks for the quick reply. Yes, I considered using getViewState() like what the ExplorerView does. but I thought since there is no other client for the tree, we are not trying to remember across sessions, hiding it was fine. From a users perspective, makes no difference. Yes, the stopped event may never arrive but we are not waiting for that event either.

I will implement the getViewState() method today.

But, may I ask for one change from your proposal. The last bullet says, 'clear the tree when running state happens'. May I suggest we not do that instantly. The current method of waiting for 400ms has a nice visual effect of not clearing the window and then repainting it. Causes flashing. 400ms is a nice enough time because the user won't have time to interact with the tree while it is invalid and lots of times, it debugger stops.

I will implement without losing the current nice feature of not flashing ... unless you insist.

[isidorn]

Yes, your correction sounds good to me. You can sketch up a PR and we can take it from there if it looks good. Thanks!

[haneefdm]

@isidorn, I created a PR #76476 using the getViewState() method you suggested. From a UX point of view, it looks/feels the same as my first implementation.

FYI, I was working with the MIEngine folks to add a feature to MIEngine (registers scope) and ran into this during my testing. I made sure all variable IDs were the same and I was still losing tree state. If accepted, it will help a lot.

[isidorn]

Thank you for the PR I will review it later today.

[isidorn]

Fixed via #76476

[isidorn]

@haneefdm once the new insiders is out tomorrow please let me know if you can still reproduce the original issue.

[haneefdm]

Will do and report back.

[haneefdm]

Done. It works as expected in today's insiders build and the original issue is no more.

[isidorn]

Great, adding verified label.