Xamarin.Forms icon indicating copy to clipboard operation
Xamarin.Forms copied to clipboard

[Bug] CarouselView System.ArgumentOutOfRangeException when adding and removing at same time

Open ichijikuibo opened this issue 4 years ago • 9 comments

Description

I opened issue #13458 a couple of months ago and it was marked as closed but it is still happening. The issue is that when both adding a removing items without giving the CaroselView a chance to execute code on the main thread causes a crash.

Steps to Reproduce

  1. Add 2 items to a CarouselView's bound ObservableCollection
  2. Move the CarouselView to the 2nd item (this updates the CurrentPosition to 1)
  3. Add a third item and remove the first 2 within the same method (It attempts to set CurrentPosition back to 1 when there is only 1 item)

Expected Behavior

CarouselView to update

Actual Behavior

Crash with exception System.ArgumentOutOfRangeException

Basic Information

  • Version with issue: 5.0.0.2012
  • Last known good version: 4.8.1821
  • Platform Target Frameworks:
    • iOS: 11.4
    • Android: 10

Screenshots

https://user-images.githubusercontent.com/61333434/107873875-9ac60b80-6ead-11eb-8913-48bbd7412758.mp4

Reproduction Link

CarouselView Crash.zip

Workaround

It is better when loop is enabled and removing items first befo re adding but it real usage the crash still happens.

In my case i was able to use a CollectionView instead of a CarouselView. Screenshot_1615279310

ichijikuibo avatar Mar 09 '21 08:03 ichijikuibo

I got an email with someone posting a fix but for some reason I can't see the post here now but I tried the suggestion of adding:

        Xamarin.Forms.BindingBase.EnableCollectionSynchronization(testItems,` null, ObservableCollectionCallback);
        void ObservableCollectionCallback(IEnumerable collection, object context, Action accessMethod, bool writeAccess)
        {
            // lock ensures that only one thread access the collection at a time
            lock (collection)
            {
                accessMethod?.Invoke();
            }
        }

but it didn't fix it, the collection is only being altered from 1 thread already but I'm glad to know of that method now I already know somewhere else that I need it.

The problem is that its trying to set CurrentPosition to a value higher than the number in the collection when the collection is updated too quickly.

https://user-images.githubusercontent.com/61333434/111065679-cb9c5f00-84b2-11eb-9054-e0d64b8d4eae.mp4

Changing part of CollectionItemsSourceChanged in CarouselViewRenderer.cs from

		Device.BeginInvokeOnMainThread(() =>
			{
				SetCurrentItem(carouselPosition);
				UpdatePosition(carouselPosition);

				//If we are adding or removing the last item we need to update
				//the inset that we give to items so they are centered
				if (e.NewStartingIndex == count - 1 || removingLastElement)
				{
					UpdateItemDecoration();
				}

				UpdateVisualStates();
			});

To


		        Device.BeginInvokeOnMainThread(() =>
			{
				if (count > carouselPosition)
				{
					SetCurrentItem(carouselPosition);
					UpdatePosition(carouselPosition);
				}

				//If we are adding or removing the last item we need to update
				//the inset that we give to items so they are centered
				if (e.NewStartingIndex == count - 1 || removingLastElement)
				{
					UpdateItemDecoration();
				}

				UpdateVisualStates();
			});

seems to fix it but thats probably not the best solution.

ichijikuibo avatar Mar 14 '21 10:03 ichijikuibo

[mono] System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
[mono] Parameter name: index
[mono]   at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.61(intptr,intptr,intptr)
[mono]   at (wrapper native-to-managed) Android.Runtime.DynamicMethodNameCounter.61(intptr,intptr,intptr)
[mono-rt] [ERROR] FATAL UNHANDLED EXCEPTION: System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
[mono-rt] Parameter name: index

jsuarezruiz avatar Mar 23 '21 12:03 jsuarezruiz

I noticed this problem and did my testing of it on android but it is also on iOS as well. I haven't checked myself on the current version but someone else posted in the old closed topic confirming that it is still in iOS.

ichijikuibo avatar Mar 23 '21 12:03 ichijikuibo

I've seen this on my Recipes app too, just when removing something I suspect this might also be related to CollectionView issues people are experiencing when removing the last item/group

rachelkang avatar Apr 28 '21 18:04 rachelkang

i was testing and wasn't able to make it crash, am i doing it right?

  • Add 2 items
  • Go to position 1
  • Remove item 0
  • Remove item 1
  • Add item

i can't get it to crash like that, what i see on video is :

  • Go to position 1
  • Remove item 0
  • Remove item 1

That will always crash because there's 2 items in the collection, if you remove item at position 0, you can't then remove at position 1 because there's only 1 item on the collection

rmarinho avatar May 04 '21 17:05 rmarinho

On the sample I uploaded 2 are already added so if you move to the 2nd value and hit the "Add 1 Remove 2" button it will crash. The button removes the 2nd one first then the first one then adds a third adding a third then removing the 2nd then 1st also will crash it.

I just discovered it doesn't crash if the same one is added 2 times I never put anything in the prevent that from happening.

It is important that it happens in the same event as the Device.BeginInvokeOnMainThread within CollectionItemsSourceChanged in CarouselViewRenderer.cs isn't given the opportunity to run until the event reaches the end of the method.

ichijikuibo avatar May 04 '21 17:05 ichijikuibo

Don't know if this helps or not but here's where I discovered the crash.

With a Collection view this is what happens:

https://user-images.githubusercontent.com/61333434/117054343-e7cdb700-ad11-11eb-9019-6a0dbf32a1f2.mp4

but with a CarouselView this is what happens:

https://user-images.githubusercontent.com/61333434/117054528-177cbf00-ad12-11eb-891d-634dae415dbb.mp4

This isn't the only situation it crashes it seems that if I touch it at all it will eventually crash it was just the one that I was able to replicate first in a blank project and I think its all the same reason.

ichijikuibo avatar May 04 '21 18:05 ichijikuibo

Any updates on this?

alexmartinezm avatar Sep 14 '22 08:09 alexmartinezm

I have the same problem, whats the status here. Want be fixed because of MAUI? ...

i have just found 2 work arounds for 2 scenarios:

  1. If you only have to add or remove items, for example you have to add or remove more than one item in the same loop, use the Xamarin Community Toolkit ObservableRangeCollection (Xamarin.CommunityToolkit.ObjectModel.ObservableRangeCollection)
  2. If you have to remove and add an item in a very short of period, you have to use a dirty Task.Delay ........ I know its rly dirty but its working. Even a 2ms delay is working ...

stoff99 avatar Sep 21 '22 09:09 stoff99

This is still a problem. I get the exeception when adding and removing images in the same session. I am binding to a ObservableRangeCollection from the Community Toolkit.

MartinMilan avatar Sep 07 '23 07:09 MartinMilan