snoopwpf/snoopwpf

StackOverflowException on Ctrl+Shift

miloush opened this issue · 9 comments

The code in ApplyReduceDepthFilterIfNeeded does not seem to be effective when selecting elements using Ctrl+Shift.

I am getting stack overflow during arrange (i.e. ProperTreeViewItem.ArrangeOverride) and/or measure.

This condition:

var shouldReduce = this.scrollViewer is { ExtentWidth: >= MaxExtentWidth } || currentDepthFromCurrentRoot > MaxDepth;

is never true (and I confirm that if I make it true in the debugger, it all works). The scrollViewer.ExtentWidth remains constant (at about 373.3) for me and item.Depth is always 56 throughout the arrange (because the selectedItem is set as item). curNode.Indent keeps consistently increasing though, and the indent just before SO is 540. The depth at which SO occurs is 45.

The ExtentWidth check does not seem to be useful when selecting an item using Ctrl+Shift, since it will not be updated yet. Should we just add another check for indent say >500?

The injected Snoop.Core.dll is from net452 directory (1.0.0+f96a86e5d5a332bb497b0e385e3c878db93629f2).

batzen commented

The problem with that code is:
Whatever limit we choose, it's wrong...
I tried quite a lot variants to get it right...
If it runs into an SO depends on quite a few factors like the combined width of the nodes, the depth, if a debugger is attached or not...

So even lowering it to 45, to cover your case, won't work for everyone.
Adding a check for indent makes no sense as indent is just depth * 12. ;-)

So i don't know how to solve this issue properly...

Good point, i got distracted by the depth being fixed to the selected node.

Yeah, not great. Perhaps the depth could be configurable, and we might even be able to capture the failing value during SO.

How do you feel about increasing the fixed stack size?

batzen commented

Good point, i got distracted by the depth being fixed to the selected node.

The depth there is relative to the current root node and as the root node gets swapped when reducing it's ok to use a fixed limit there.

we might even be able to capture the failing value during SO.

Most of the time you can't recover from an SO. I think most processes will just crash.

How do you feel about increasing the fixed stack size?

Is that possible?

You can't recover from SO but you might be able to note down the depth at which it happened. This would have to be done without any stack allocations obviously. - also it might be possible in .NET Framework only.

Yeah you can have a post-build step of calling editbin from MSVC tools... https://learn.microsoft.com/en-us/cpp/build/reference/stack?view=msvc-170

batzen commented

You can't recover from SO but you might be able to note down the depth at which it happened. This would have to be done without any stack allocations obviously. - also it might be possible in .NET Framework only.

The problem is that there isn't a fixed size/depth at which the issues happens.
Previously the limit was 100 and worked for years until it started to happen at around 75 even though I have seen it happening at 45, but only when I was debugging with Rider (didn't happen when debugging with Visual Studio).
If 45 would be the magic number I would happily change the limit to that, but it's, sadly, dynamic/random...

Yeah you can have a post-build step of calling editbin from MSVC tools

That won't have any effect for Snoop then, as Snoop injects itself into foreign processes and does not start or own those.
Or would it work for single libraries?

Snoop injects itself into foreign processes

🤦 Of course! Nevermind then. From top of my head I don't have a good answer. I can see if you can estimate the available stack size, but maybe the ultimate solution would be to not rely on nested UI elements.

batzen commented

but maybe the ultimate solution would be to not rely on nested UI elements.

I don't know how to achieve that with a tree view.
Actually that's possible, but would require porting code from one of my private projects.
I have code there that can display hierarchical data in a DataGrid, just by changing the type of CollectionView being used (and a custom cell template of course). But it's quite slow when expanding a lot of items at once, as it doesn't do so differential.

Either I port that or find a way to just get rid of element nesting, as you suggested.

I won't have time to try that before my vacation, but will try afterwards (mid July).

OK so we can PInvoke GetCurrentThreadStackLimits to get the memory range of the stack, and then we can get an address of stackalloc'ed memory in the filter method and check if there is enough memory left in the stack. The threshold will have to be an educated estimate, but it should not be as random as the current limits.
The downside of this is that getting stackalloc to return a pointer requires enabling unsafe code.

Is that a solution you would be interested in exploring?