CommunityToolkit / WindowsCommunityToolkit

The Windows Community Toolkit is a collection of helpers, extensions, and custom controls. It simplifies and demonstrates common developer tasks building .NET apps with UWP and the Windows App SDK / WinUI 3 for Windows 10 and Windows 11. The toolkit is part of the .NET Foundation.
https://docs.microsoft.com/windows/communitytoolkit/
Other
5.87k stars 1.38k forks source link

DataGrid goes into Infinity Loop #3073

Open filipwa84 opened 4 years ago

filipwa84 commented 4 years ago

Describe the bug

The DataGrid sometimes goes into an infinity loop. What triggers it is somewhat random and seems to depend on the screen resolution, the DPI settings and the width of the given DataGrid rows.

In this example - and on my computer - the application just hangs as soon as it starts if the DataGrid is in a StackPanel. However, putting it inside a Grid works. Testing the same code on a second computer with higher resolution and a DPI setting of 150, it crashes in both senarios.

Opening the Task Manager while it is haning one can see that there is a core is consistently running at 100%.

Importing the DataGrid source code into the project and hitting pause while it is haning, the code keeps coming back to DataGridRow.cs line 1967: element.Measure(new Size(double.PositiveInfinity, double.PositiveInfinity)).

Steps to Reproduce

Steps to reproduce the behavior:

  1. Clone https://github.com/filipwa84/DataGridInfinity.git
  2. Add the DataGrid project to the solution
  3. From MainPage.xaml uncomment the code inside the StackPanel marked as not working
  4. Debug, hit pause and set a break point in DataGridRow.cs on line 1967
  5. If it doesn't hang, try hit refresh, change the resolution, DPI settings or try on another computer...

Expected behavior

Should not go into an infinity loop

NuGet Package(s): 
Microsoft.Toolkit.Uwp.UI.Controls.DataGrid
Package Version(s): 
6.0

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [x] May 2019 Update (18362)
- [ ] Insider Build (build number: )

App min and target version:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [x] May 2019 Update (18362)
- [ ] Insider Build (xxxxx)

Device form factor:
- [x] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [ ] 2017 (version: )
- [x] 2019 (version: ) 
- [ ] 2019 Preview (version: )
michael-hawker commented 4 years ago

Thanks for reporting @filipwa84.

Adding @anawishnoff and @RBrid for insights too.

anawishnoff commented 4 years ago

This is definitely an odd bug, and a big one too if its affecting multiple users/apps. I haven't encountered this issue before, and I'm not too sure what's causing it. The line that it stops at is likely our clue...

@RBrid, do you have any insights as to what line 1967 in DataGridRow.cs accomplishes and why it may break at certain scenarios? Figured you're most familiar with the source code of DataGrid :)

filipwa84 commented 4 years ago

@anawishnoff @RBrid Weird indeed. I first encountered it in the application we are working on as it would hang/crash when moving the window from my laptop screen to an external monitor. The two screens had different resolutions and DPI settings. On the laptop the application runs fine but when opening the app on the external monitor it crashes/hangs.

A temporary fix on my computer proved to be changing the margins of the DataGrid but it doesn't ensure that it won't happen on another computer...

Unfortunately I am not able to share the code of the production application but I am available for a Skype call if you need me to show you more in detail how it is triggered.

filipwa84 commented 4 years ago

@anawishnoff Any update on this?

Kyaa-dost commented 4 years ago

@RBrid any update on this? CC: @anawishnoff

RBrid commented 4 years ago

Hello @filipwa84, I tried to repro the infinite loop and could not, at various screen resolutions.

What I saw were two things:

Number 1, I had to add the line "if (PropertyChanged != null)" in your app code below (MainPage.xaml.cs)

        [NotifyPropertyChangedInvocator]
        public void OnPropertyChanged([CallerMemberName]string name = null)
        {
            if (PropertyChanged != null)
                PropertyChanged.Invoke(this, new PropertyChangedEventArgs(name));
        }

because sometimes PropertyChanged was null. That fixed a NullRefException.

Number 2, I did not witness an infinite loop but a long loop. The data source has 1344 items coming from data.csv and so 1344 rows are created and rendered - that causes long loops. There is no vertical scrollbar, or vertical scrolling, because the DataGrid container is a vertical StackPanel that lets its content grow as tall as it wishes. The DataGrid's available height is infinity so it creates rows for all the databound items. If the container were a Grid, the DataGrid's available height would be limited to the Grid's height.

When you shorten the loops by only putting 100 items in the data.cvs file, the DataGrid's performance gets much better.

The DataGrid's height really should be constrained when it holds many rows.

Are you still seeing an infinite loop or crash that I missed? Thanks.

filipwa84 commented 4 years ago

@RBrid Thanks for your reply. I included the StackPanel in the example as it makes it reproducible. However, I am getting the infinity/"long loop" with the Grid as well - at least it stops at the same line in DataGridRow.cs on every iteration.

When the DataGrid is in a Grid the loop occurs sporadically. One trigger is when the window is moved from one screen to another which makes me suspect that it is related to resolution/DPI. I'll see if I can find some way to perhaps do a screen capture of it happening.

RBrid commented 4 years ago

Thanks @filipwa84, so the infinite loop occasionally occurs with the Grid too, and we should ignore the <!--Works--> ?

    <!--Doesn't work-->
    <StackPanel>
        <CommandBar>
            <AppBarButton Icon="Refresh" Click="{x:Bind Refresh}"/>
        </CommandBar>
        <data:DataGrid ItemsSource="{x:Bind DataModels, Mode=OneWay}"/>
    </StackPanel>

    <!--Works-->
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="Auto"/>
            <RowDefinition Height="*"/>
        </Grid.RowDefinitions>
        <CommandBar Grid.Row="0">
            <AppBarButton Icon="Refresh" Click="{x:Bind Refresh}"/>
        </CommandBar>
        <data:DataGrid Grid.Row="1" ItemsSource="{x:Bind DataModels, Mode=OneWay}"/>
    </Grid>

You may want to check what the DataGrid's MeasureOverride is given as availableSize. The available height should not be infinity. For example the Grid/DataGrid should not be placed in a vertically scrollable ScrollViewer. Thanks.

filipwa84 commented 4 years ago

@RBrid Yes, the loop occasionally occurs in the Grid too. My comments might be a bit confusing there - sorry about that.

I just checked the views where we encounter this issue in our production code and the DataGrid sits directly under the Grid so there is no StackPanel or ScrollViewer causing issues. As it only happens sporadically it's quite hard to come up with some reliable steps to reproduce it.

I'll try a few things tomorrow and see if I can trigger and document it somehow.

filipwa84 commented 3 years ago

@RBrid I just had this happen again after not seeing it for months. I managed to reproduce it while doing a screen capture. https://www.youtube.com/watch?v=hC7KdJ2rQdA&feature=youtu.be

Kyaa-dost commented 3 years ago

@RBrid was there any recent change that is causing this issue?

filipwa84 commented 3 years ago

@Kyaa-dost This has happened sporadically for the last 2 years

Kyaa-dost commented 3 years ago

Thanks, @filipwa84 for additional information. Let's see what the team have to say @RBrid ⬆️

ghost commented 3 years ago

Thanks filipwa84 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

ghost commented 3 years ago

Thanks filipwa84 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

ghost commented 3 years ago

Thanks filipwa84 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

ghost commented 3 years ago

Thanks filipwa84 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks