maui icon indicating copy to clipboard operation
maui copied to clipboard

CollectionView crashes/behaves weird when ObservableCollection updated quickly

Open michaelstonis opened this issue 3 years ago • 6 comments
trafficstars

Description

If I have a view model with an ObservableCollection<T> bound to a CollectionView. From the view model, I perform a Clear and then Add, it will crash out on iOS and macOS. On macOS, the application will sometimes not crash, but the items for the CollectionView will not appear.

Steps to Reproduce

  1. Clone this code: https://github.com/TheEightBot/eshop-maui-client/tree/feature/issue/collectionview-updates
  2. Run the application on iOS or macOS
  3. Login using any credentials (username: test, password: test)
  4. The application will crash when trying to load the CatalogView

The crash is being triggered from code in eShopOnContainers/ViewModels/ObservableCollectionEx.cs. There are two methods named ReloadData which clear and reload the items of the ObservableCollection

Version with bug

Release Candidate 3 (current)

Last version that worked well

Unknown/Other

Affected platforms

iOS, macOS

Affected platform versions

macOS Monterey, iOS 15.4

Did you find any workaround?

Yes, if I handle the list updates and delay notification of changes, it will respond appropriately. Modified versions of the ReloadData methods are provided in eShopOnContainers/ViewModels/ObservableCollectionEx.cs that will avoid this crash.

public void ReloadData(Action<IList<T>> innerListAction)
{
    Items.Clear();

    innerListAction(Items);

    this.OnPropertyChanged(new PropertyChangedEventArgs(nameof(Count)));
    this.OnPropertyChanged(new PropertyChangedEventArgs("Items[]"));
    this.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
}

public async Task ReloadDataAsync(Func<IList<T>, Task> innerListAction)
{
    Items.Clear();

    await innerListAction(Items);

    this.OnPropertyChanged(new PropertyChangedEventArgs(nameof(Count)));
    this.OnPropertyChanged(new PropertyChangedEventArgs("Items[]"));
    this.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
}

Relevant log output

Objective-C exception thrown.  Name: System.ArgumentOutOfRangeException Reason: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index') (System.ArgumentOutOfRangeException)
   at System.Collections.Generic.List`1[[eShopOnContainers.Models.Catalog.CatalogItem, eShopOnContainers, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].get_Item(Int32 index)
   at System.Collections.ObjectModel.Collection`1[[eShopOnContainers.Models.Catalog.CatalogItem, eShopOnContainers, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].System.Collections.IList.get_Item(Int32 index)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.ElementAt(Int32 index)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.get_Item(Int32 index)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.get_Item(NSIndexPath indexPath)
   at Microsoft.Maui.Controls.Handlers.Items.ItemsViewController`1[[Microsoft.Maui.Controls.ReorderableItemsView, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].GetSizeForItem(NSIndexPath indexPath)
   at Microsoft.Maui.Controls.Handlers.Items.ItemsViewDelegator`2[[Microsoft.Maui.Controls.ReorderableItemsView, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[Microsoft.Maui.Controls.Handlers.Items.ReorderableItemsViewController`1[[Microsoft.Maui.Controls.ReorderableItemsView, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout, NSIndexPath indexPath)
   at UIKit.UICollectionView.InsertItems(NSIndexPath[] indexPaths)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.<>c__DisplayClass41_0.<Add>b__0()
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.Update(Action update, NotifyCollectionChangedEventArgs args)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.Add(NotifyCollectionChangedEventArgs args)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.CollectionChanged(NotifyCollectionChangedEventArgs args)
   at Microsoft.Maui.Controls.Handlers.Items.ObservableItemsSource.<>c__DisplayClass37_0.<CollectionChanged>b__0()
   at Foundation.NSAsyncActionDispatcher.Apply()

Native stack trace:
	0   CoreFoundation                      0x00000001101bed44 __exceptionPreprocess + 242
	1   libobjc.A.dylib                     0x000000011d071a65 objc_exception_throw + 48
	2   libxamarin-dotnet-debug.dylib       0x000000010bc3af6f xamarin_process_managed_exception + 943
	3   eShopOnContainers                   0x0000000102c14d29 _ZL31native_to_managed_trampoline_10P11objc_objectP13objc_selectorPP11_MonoMethodj + 361
	4   eShopOnContainers                   0x0000000102c154d9 -[__MonoMac_NSAsyncActionDispatcher xamarinApplySelector] + 41
	5   Foundation                          0x00000001131a73bf __NSThreadPerformPerform + 179
	6   CoreFoundation                      0x000000011012b833 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
	7   CoreFoundation                      0x000000011012b72b __CFRunLoopDoSource0 + 180
	8   CoreFoundation                      0x000000011012abf8 __CFRunLoopDoSources0 + 242
	9   CoreFoundation                      0x00000001101252f4 __CFRunLoopRun + 871
	10  CoreFoundation                      0x0000000110124a90 CFRunLoopRunSpecific + 562
	11  GraphicsServices                    0x000000011fdbfc8e GSEventRunModal + 139
	12  UIKitCore                           0x000000012873890e -[UIApplication _run] + 928
	13  UIKitCore                           0x000000012873d569 UIApplicationMain + 101
	14  libmonosgen-2.0.dylib               0x000000010c428b12 do_icall + 194
	15  libmonosgen-2.0.dylib               0x000000010c427b2d do_icall_wrapper + 253
	16  libmonosgen-2.0.dylib               0x000000010c419d4a interp_exec_method + 2970
	17  libmonosgen-2.0.dylib               0x000000010c417fcf interp_runtime_invoke + 239
	18  libmonosgen-2.0.dylib               0x000000010c30fc1b mono_jit_runtime_invoke + 1227
	19  libmonosgen-2.0.dylib               0x000000010c232318 mono_runtime_invoke_checked + 136
	20  libmonosgen-2.0.dylib               0x000000010c2390bb mono_runtime_exec_main_checked + 107
	21  libmonosgen-2.0.dylib               0x000000010c3720b2 mono_jit_exec + 354
	22  libxamarin-dotnet-debug.dylib       0x000000010bc4ef0d xamarin_main + 1949
	23  eShopOnContainers                   0x0000000102ca6954 main + 52
	24  dyld                                0x000000010b822f21 start_sim + 10
	25  ???                                 0x000000020312e51e 0x0 + 8641504542
	26  ???                                 0x0000000000000003 0x0 + 3

michaelstonis avatar May 16 '22 21:05 michaelstonis

repro on IOS with vs main build[32516.290.main]

VincentBu avatar May 17 '22 02:05 VincentBu

There's a very similar issue with CollectionView when adding and clearing items quickly that even existed on XF: https://github.com/xamarin/Xamarin.Forms/issues/13268

In my own app, I have to use a bunch of Task.Delay(50) or else the collection view throws a fatal exception identical to the one linked.

emorell96 avatar May 17 '22 17:05 emorell96

I just ran into the issue that @emorell96 mentioned.

vniehues avatar May 19 '22 09:05 vniehues

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost avatar Aug 30 '22 14:08 ghost

There's a very similar issue with CollectionView when adding and clearing items quickly that even existed on XF: xamarin/Xamarin.Forms#13268

In my own app, I have to use a bunch of Task.Delay(50) or else the collection view throws a fatal exception identical to the one linked.

Use Task Delay (50) can solve problems sometimes, thanks

itldg avatar Oct 16 '22 09:10 itldg

Workaround for me was to instantiate a new ObservableCollection. Not ideal for memory usage but didn't need hardcoded delays.

KieranMaclagan avatar Nov 24 '22 18:11 KieranMaclagan

Workaround for me was to instantiate a new ObservableCollection. Not ideal for memory usage but didn't need hardcoded delays.

@KieranMaclagan Can You Please Share The Simple Code !! I instantiate a new Observable Like This :

if  (transactions.Count > 0)
     {
            transactions.Clear();
            transactions = new ObservableCollection<Datum>();
     }
                 
foreach (var item in responseData.data)
             transactions.Add(item);

The error has been fixed, but the new results items are not Binding in the CollectionView !?

Kremed avatar Dec 27 '22 15:12 Kremed

Using ObservableRangeCollection may help, you can use colleciton.AddRange(items) to add items, which is provided by a third-party nuget package (Refractored.MvvmHelpers).

singhwong avatar Dec 27 '22 18:12 singhwong

Using ObservableRangeCollection may help, you can use colleciton.AddRange(items) to add items, which is provided by a third-party nuget package (Refractored.MvvmHelpers).

That works great for me, but I still need some of the tasks.Delay(50) Before Add The Range. Thanks, Sir

Kremed avatar Dec 28 '22 10:12 Kremed

I appreciate the Delay(50) advice, but with latest .Net MAUI and collection view, that is not reliably working for me, not even at 100ms. Running inserts / removes in main thread seemed to help, but makes app less responsive. I think the iOS handler fix would be simple, but I don't know how to override / replace it, sadly. I finally resorted to using SfListView from Syncfusion -- it took my same templates, so I only had to adjust a few parameters and it worked right away. Not only does it avoid crashing, but also helps work around the default CollectionView's tendency to layout landscape mode on potrait phone screen on iOS. Telerik also has a similar control. I also wrote my own view, but could not get the tap / hold I need on items working reliably with my list contained in a scrollview control. Hopefully this is fixed sometime soon or a free one is out there, but for those as desperate as I was, at least there is a paid option that works.

VWilcox2000 avatar Feb 24 '23 07:02 VWilcox2000

hey there, had a similar issue (on Android): I use a ObservableCollection<T> in a CarouselView. when the collection get its items, all fine. but when you scroll to an other item and want to reload the collection, it seems like that the collection want to load the last index. is this index not available in the reloaded collection, it occurs an error: System.ArgumentOutOfRangeException: 'Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')'

//Task to load collection ... if(mycollection.Count != 0) { mycollection.Clear(); }

foreach (var newitem in loadit) { mycollection.Add(newitem); } ...

Any thoughts, how I could fix this issue?

deckelmouck avatar Mar 31 '23 10:03 deckelmouck

This was how I managed to solve the issue on my end.

I had a similar issue today when I was testing a solution on an iOS emulator. The ObservableCollection I had when i was populating the CollectionView, would cause a crash. Since I am using the MVVM pattern and the MVVM Community Toolkit, I decided to replace the Observable Collection with a List that uses the ObservableProperty attribute. Just had to make sure I instantiated the list properly and it fixed the error for me.

This is done in the MVVM pattern. Viewmodel:

[ObservableProperty]
public List<Model> collection;

public async Task LoadCollection()
{
...code goes here...

Collection= new List<Model>();
foreach (var item in results)
{
Collection.Add(Item);
}
}

ITPDevAruba avatar Apr 05 '23 15:04 ITPDevAruba

thanks for your reply @ITPDevAruba

I had a workaround for our case too: It seems like the carouselView with it Position-property causes our problems, so if the collectionHeader (an order number) changes, I reset Position to 0, clear the collection and add new items (from another order number) to it.

Not so pretty, but it works ;)

deckelmouck avatar Apr 07 '23 12:04 deckelmouck

Any news on this? I don't see any progress or linked PRs/discussions for this? Not even for .Net 8 Our code is littered with

#if IOS
                ObservableCollection<X> tempCollection = new(sourceCollection);
                BoundCollection = tempCollection;
#else
                BoundCollection.Clear(); //Or just Remove single item/multiple items
                BoundCollection.AddRange(sourceCollection);
#endif

DDHSchmidt avatar Jul 11 '23 15:07 DDHSchmidt

@samhouts, could you triage this bug? For GA .net8 or even better with a backport to 7 would be swell!

I was able to reproduce it in both .net8 (latest preview) and .net7 but it seems a lot more stable on .net8. Interestingly the selected item isn't removed once you go back with a swipe gesture but it is if you go back with the toolbar button it will.

UkeHa avatar Sep 01 '23 07:09 UkeHa

@DDHSchmidt, does this workaround really work for you? if i do a bound DevExpress.Data.XtraReports.ObservableRangeCollection and set it's content to a temporary ObservableRangeCollection it renders an empty collection. Same is true for a normal ObserverableCollection (tested .net7 ios16)

UkeHa avatar Sep 01 '23 08:09 UkeHa

Yep, I'm going to add this one to NET 8 GA because I'm concerned that workarounds for this problem will exacerbate https://github.com/dotnet/maui/issues/16125. It looks like @jfversluis has a PR for Forms that either fixes or reduces the crashes, so maybe that's a place to start! https://github.com/xamarin/Xamarin.Forms/pull/15439

samhouts avatar Sep 01 '23 16:09 samhouts

Hm yeah I think I didn't merge that PR because it didn't seem to fix the issue, but it also introduced a massive performance hit. So there is still work to do on this one!

Also, the Forms PR talks about grouped CollectionViews specifically and looking at this one it isn't grouped, right? Just to make sure we're looking at the right things here.

Trying on the latest .NET 8 bits, it seems to work for me on the branch that is posted @michaelstonis. Any chance you might be able to test that?

Also on the latest .NET 7 bits I can't reproduce it with the given reproduction in the first post. Is there another reproduction I could try from someone?

jfversluis avatar Sep 05 '23 13:09 jfversluis

Hi @michaelstonis. We have added the "s/try-latest-version" label to this issue, which indicates that we'd like you to try and reproduce this issue on the latest available public version. This can happen because we think that this issue was fixed in a version that has just been released, or the information provided by you indicates that you might be working with an older version.

You can install the latest version by installing the latest Visual Studio (Preview) with the .NET MAUI workload installed. If the issue still persists, please let us know with any additional details and ideally a reproduction project provided through a GitHub repository.

This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

ghost avatar Sep 14 '23 09:09 ghost

Verified this issue with Visual Studio Enterprise 17.8.0 Preview 1.0. Not repro on iOS platforms with sample project. eshop-maui-client-feature-issue-collectionview-updates.zip Screenshot 2023-09-14 162242

Zhanglirong-Winnie avatar Sep 14 '23 09:09 Zhanglirong-Winnie

Will this be fixed in .NET 7 as well?

borrmann avatar Sep 29 '23 12:09 borrmann