Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CollectionView crashes/behaves weird when ObservableCollection updated quickly #7237

Closed
michaelstonis opened this issue May 16, 2022 · 21 comments
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView high It doesn't work at all, crashes or has a big impact. p/1 Work that is critical for the release, but we could probably ship without platform/iOS 🍎 s/triaged Issue has been reviewed s/try-latest-version Please try to reproduce the potential issue on the latest public version t/bug Something isn't working

Comments

@michaelstonis
Copy link

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 michaelstonis added s/needs-verification Indicates that this issue needs initial verification before further triage will happen t/bug Something isn't working labels May 16, 2022
@VincentBu VincentBu added the s/triaged Issue has been reviewed label May 17, 2022
@VincentBu
Copy link

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

@VincentBu VincentBu added s/verified Verified / Reproducible Issue ready for Engineering Triage and removed s/triaged Issue has been reviewed s/needs-verification Indicates that this issue needs initial verification before further triage will happen labels May 17, 2022
@jsuarezruiz jsuarezruiz added fatal area-controls-collectionview CollectionView, CarouselView, IndicatorView labels May 17, 2022
@emorell96
Copy link

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.

@Redth Redth added platform/iOS 🍎 p/1 Work that is critical for the release, but we could probably ship without labels May 17, 2022
@Redth Redth modified the milestones: 6.0.300-servicing, 6.0.3xx-sr1 May 17, 2022
@vniehues
Copy link

I just ran into the issue that @emorell96 mentioned.

@ghost
Copy link

ghost commented Aug 30, 2022

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.

@samhouts samhouts removed the p/1 Work that is critical for the release, but we could probably ship without label Oct 14, 2022
@KieranMaclagan
Copy link

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

@Kremed
Copy link

Kremed commented Dec 27, 2022

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 !?

@singhwong
Copy link

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

@Kremed
Copy link

Kremed commented Dec 28, 2022

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

@VWilcox2000
Copy link

VWilcox2000 commented Feb 24, 2023

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.

@deckelmouck

This comment was marked as off-topic.

@ITPDevAruba
Copy link

ITPDevAruba commented Apr 5, 2023

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);
}
}

@samhouts samhouts removed the s/verified Verified / Reproducible Issue ready for Engineering Triage label Apr 5, 2023
@deckelmouck
Copy link

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 ;)

@DDHSchmidt
Copy link

DDHSchmidt commented Jul 11, 2023

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

@UkeHa
Copy link

UkeHa commented Sep 1, 2023

@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
Copy link

UkeHa commented Sep 1, 2023

@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)

@samhouts
Copy link
Member

samhouts commented Sep 1, 2023

Yep, I'm going to add this one to NET 8 GA because I'm concerned that workarounds for this problem will exacerbate #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! xamarin/Xamarin.Forms#15439

@samhouts samhouts modified the milestones: Backlog, .NET 8 GA Sep 1, 2023
@samhouts samhouts added the high It doesn't work at all, crashes or has a big impact. label Sep 1, 2023
@jfversluis
Copy link
Member

jfversluis commented Sep 5, 2023

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?

@Zhanglirong-Winnie Zhanglirong-Winnie added s/triaged Issue has been reviewed s/try-latest-version Please try to reproduce the potential issue on the latest public version labels Sep 14, 2023
@ghost
Copy link

ghost commented Sep 14, 2023

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.

@Zhanglirong-Winnie
Copy link
Collaborator

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

@samhouts samhouts added the p/1 Work that is critical for the release, but we could probably ship without label Sep 18, 2023
@ghost ghost closed this as completed Sep 26, 2023
@borrmann
Copy link
Contributor

Will this be fixed in .NET 7 as well?

@ghost ghost locked as resolved and limited conversation to collaborators Oct 29, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView high It doesn't work at all, crashes or has a big impact. p/1 Work that is critical for the release, but we could probably ship without platform/iOS 🍎 s/triaged Issue has been reviewed s/try-latest-version Please try to reproduce the potential issue on the latest public version t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests