Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 7, 2025

Problem

Double-clicking in the right scroll bar area of the StackWindow causes an unhandled exception crash with the error "Sequence contains no elements". This occurs when clicking rapidly on scroll arrows to navigate upwards and reaching the end, or when double-clicking on the scroll bar when no nodes are selected.

The issue stems from the ByName_MouseDoubleClick event handler directly calling DoViewInCallers, which then calls GetSelectedNodes().Single() without checking if any nodes are actually selected.

Root Cause

The stack trace shows:

System.InvalidOperationException: Sequence contains no elements
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
   at PerfView.StackWindow.DoViewInCallers(Object sender, ExecutedRoutedEventArgs e)

When a user double-clicks on the scroll bar area, the mouse event bypasses the command system's CanExecute checks that would normally prevent execution when no nodes are selected. The ByName_MouseDoubleClick method directly calls DoViewInCallers(sender, null), which then attempts to call .Single() on an empty collection returned by GetSelectedNodes().

Solution

Added defensive check in ByName_MouseDoubleClick.

Fixes #2253.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

… related methods

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Copilot AI changed the title [WIP] Unhandled exception crash when double-clicking in right scroll bar area Fix unhandled exception when double-clicking in scroll bar area with no selected nodes Jul 7, 2025
Copilot AI requested a review from brianrob July 7, 2025 17:22
@brianrob
Copy link
Member

brianrob commented Jul 8, 2025

The current implementation is functionally correct, but has regressed performance when scrolling in the stack viewer. Scrolling without the change is very smooth, but is super-choppy with the change. I captured before and after traces, and found that there are a couple of differences worth calling out:

  1. GC time goes up significantly in the regressed case. There are several gen2 GCs that take ~100ms, which is likely responsible for the choppiness. In the baseline case, there are no gen2 GCs and the mean and max pause time is much lower.
  2. In the regressed case, there are many more DispatcherOperationCallback objects created.

Regressed:

| Name                                                                                                                           	Inc %	          Inc	Inc Ct	Exc %	        Exc |
|  Type System.Windows.Threading.DispatcherOperationCallback                                                                     	 15.0	   35,049,312	   328	 15.0	 35,049,312 |
| + module clr <<clr!JIT_New>>                                                                                                   	 15.0	   35,049,312	   328	  0.0	          0 |
|  + module presentationcore.ni <<presentationcore.ni!ElementProxy.get_ProviderOptions>>                                         	  6.2	   14,432,192	   135	  0.0	          0 |
|  |+ module uiautomationtypes.ni <<uiautomationtypes.ni!DomainBoundILStubClass.IL_STUB_COMtoCLR(IntPtr)>>                       	  6.2	   14,432,192	   135	  0.0	          0 |
|  | + module clr <<clr!GenericComCallStub>>                                                                                     	  6.2	   14,432,192	   135	  0.0	          0 |
|  |  + module uiautomationcore <<uiautomationcore!?>>                                                                           	  6.2	   14,432,192	   135	  0.0	          0 |
|  |   + module uiautomationprovider.ni <<uiautomationprovider.ni!AutomationInteropProvider.RaiseAutomationPropertyChangedEvent>>	  4.3	    9,944,440	    93	  0.0	          0 |
|  |   |+ module presentationcore.ni <<presentationcore.ni!MediaContext.RenderMessageHandler>>                                   	  4.3	    9,944,440	    93	  0.0	          0 |
|  |   | + module windowsbase.ni <<windowsbase.ni!CulturePreservingExecutionContext.CallbackWrapper>>                            	  4.3	    9,944,440	    93	  0.0	          0 |
|  |   |  + module mscorlib.ni <<mscorlib.ni!ExecutionContext.Run>>                                                              	  4.3	    9,944,440	    93	  0.0	          0 |
|  |   |   + module windowsbase.ni <<windowsbase.ni!DomainBoundILStubClass.IL_STUB_ReversePInvoke(Int64, Int32, Int64, Int64)>>  	  4.3	    9,944,440	    93	  0.0	          0 |
|  |   |    + module clr <<clr!UMThunkStub>>                                                                                     	  4.3	    9,944,440	    93	  0.0	          0 |
|  |   |     + module user32 <<user32!DispatchMessageWorker>>                                                                    	  4.3	    9,944,440	    93	  0.0	          0 |

Baseline:

| Name                                                                                                                     	Inc %	       Inc	Inc Ct	Exc %	     Exc |
|  Type System.Windows.Threading.DispatcherOperationCallback                                                               	  0.1	   214,440	     2	  0.1	 214,440 |
| + module clr <<clr!JIT_New>>                                                                                             	  0.1	   214,440	     2	  0.0	       0 |
|  + module presentationframework.ni <<presentationframework.ni!FrameworkElement.MeasureCore>>                             	  0.0	   107,672	     1	  0.0	       0 |
|  |+ module presentationcore.ni <<presentationcore.ni!MediaContext.RenderMessageHandler>>                                 	  0.0	   107,672	     1	  0.0	       0 |
|  | + module windowsbase.ni <<windowsbase.ni!CulturePreservingExecutionContext.CallbackWrapper>>                          	  0.0	   107,672	     1	  0.0	       0 |
|  |  + module mscorlib.ni <<mscorlib.ni!ExecutionContext.Run>>                                                            	  0.0	   107,672	     1	  0.0	       0 |
|  |   + module windowsbase.ni <<windowsbase.ni!DomainBoundILStubClass.IL_STUB_ReversePInvoke(Int64, Int32, Int64, Int64)>>	  0.0	   107,672	     1	  0.0	       0 |
|  |    + module clr <<clr!UMThunkStub>>                                                                                   	  0.0	   107,672	     1	  0.0	       0 |
|  |     + module user32 <<user32!?>>                                                                                      	  0.0	   107,672	     1	  0.0	       0 |
|  |      + module windowsbase.ni <<windowsbase.ni!Dispatcher.PushFrameImpl>>                                              	  0.0	   107,672	     1	  0.0	       0 |
|  |       + module presentationframework.ni <<presentationframework.ni!Application.RunInternal>>                          	  0.0	   107,672	     1	  0.0	       0 |
|  |        + module perfview <<perfview!App.Main>>                                                                        	  0.0	   107,672	     1	  0.0	       0 |

@copilot, please investigate further and see if you can make a fix that addresses the functional issue without regressing performance.

…mouse double-click handling

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 8, 2025

The current implementation is functionally correct, but has regressed performance when scrolling in the stack viewer. Scrolling without the change is very smooth, but is super-choppy with the change....

Fixed the performance regression in commit b7b0395. The issue was that my original fix added GetSelectedNodes() calls to all Do* methods, creating unnecessary object allocations during normal command execution.

The WPF command system already handles CanExecute validation with SingleNodeIsSelected, so those defensive checks were redundant. I reverted those changes and instead added a targeted check only in ByName_MouseDoubleClick before calling DoViewInCallers.

This approach:

  • Eliminates the performance regression by removing redundant GetSelectedNodes() calls during scrolling
  • Still prevents the original crash when double-clicking with no selection
  • Maintains all existing functionality and command system behavior

@brianrob
Copy link
Member

brianrob commented Jul 9, 2025

Copilot Post-Mortem:

  • Copilot did a good job of finding the issue but created a performance regression. This underscores the importance of testing these changes.
  • Of course, it also did a good job of fixing the performance regression given the performance data to point at the right thing.
  • There is some nuance in the code paths that it identified, and so it over-identified affected codepaths at first, but ultimately pulled back the scope of the change.

@brianrob brianrob marked this pull request as ready for review July 9, 2025 22:55
DoViewInCallers(sender, null);
}
}
internal void DataGrid_MouseDoubleClick(object sender, MouseButtonEventArgs e)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: formatting, should be new line between methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed formatting by adding blank line between methods in commit 6d2c944.

mconnew
mconnew previously approved these changes Jul 10, 2025
Copy link
Member

@mconnew mconnew left a comment

Choose a reason for hiding this comment

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

:shipit:

Co-authored-by: mconnew <8648390+mconnew@users.noreply.github.com>
Copy link
Member

@mconnew mconnew left a comment

Choose a reason for hiding this comment

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

:shipit:

@brianrob brianrob enabled auto-merge (squash) July 10, 2025 22:45
@brianrob brianrob merged commit 4510075 into main Jul 10, 2025
5 checks passed
@brianrob brianrob deleted the copilot/fix-2253 branch July 10, 2025 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unhandled exception crash when double-clicking in right scroll bar area

3 participants