[Bug][iOS] Concurrent issue leading to crash in SemaphoreSlim.Release in ObservableItemsSource

[softlion]

Description

Calls to ObservableItemsSource.BatchUpdate should be synchronized to prevent a crash in _batchUpdating.Release(), because this last one can be called 2 times in a row and will fail the second time.

https://github.com/xamarin/Xamarin.Forms/blob/main/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs

Steps to Reproduce

I am able to reproduce this issue easily in my customer's app.
I've already checked that all calls are done on the main thread.

It happens nearly everytime when updating the bound ObservableCollection 3 times in a row:
1st deleting some items, 2nd inserting some new items, 3rd moving some existing items (it's an algorithm that compute the changes between an ObservableCollection and a "new" list, and apply the 3 above actions so after the ObservableCollection becomes equal to the list).

I've already checked that all 3 calls are done on the main thread.

Expected Behavior

don't crash

Actual Behavior

crash when releasing the SemaphoreSlim because its CurrentCount is already 1.

Stack trace:

SemaphoreFullException: Adding the specified count to the semaphore would cause it to exceed its maximum count.

  at System.Threading.SemaphoreSlim.Release (System.Int32 releaseCount) [0x0004c] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/threading/SemaphoreSlim.cs:795 
  at System.Threading.SemaphoreSlim.Release () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/threading/SemaphoreSlim.cs:760 
  at Xamarin.Forms.Platform.iOS.ObservableItemsSource.Reload () [0x00098] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:146 
  at Xamarin.Forms.Platform.iOS.ObservableItemsSource.Add (System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x00037] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:165 
  at Xamarin.Forms.Platform.iOS.ObservableItemsSource.CollectionChanged (System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x00061] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:117 
  at Xamarin.Forms.Platform.iOS.ObservableItemsSource.CollectionChanged (System.Object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x000b7] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:108 
  at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1021 
  at Foundation.NSAsyncSynchronizationContextDispatcher.Apply () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/Foundation/NSAction.cs:178 
--- End of stack trace from previous location where exception was thrown ---

Basic Information

• Version with issue: 4.8
• Last known good version: 4.5
• IDE: VS Windows
• Platform Target Frameworks:
  • iOS: latest stable
• Affected Devices: iOS simulator

Screenshots

Reproduction Link

Repro app: https://github.com/softlion/ReproFormsCollectionViewBug
Run the app on an iOS simulator or device.

Tested on iOS 12.4 / iPhone 6 simulator.
But happens on all iOS OS and devices.

Does not happen on Android / UWP.

This app will trigger one of the 3 above crashes, which comes from different synchronization bugs.
The key is the change of the IsVisible property just before or after items have been added to the collectionviewsource.

Workaround

None

Other infos

I've checked the xamarin forms source code.

I'm pretty sure you can fix it easily by making BatchUpdate return a Task, then start BatchUpdate() with await _batchUpdating.WaitAsync(); instead of putting _batchUpdating.Wait() inside the PerformBatchUpdates action parameter.

[jsuarezruiz]

@softlion Thanks for the feedback. Do you have a small example where reproduce the issue?.

[jsuarezruiz]

This are the possible changes: https://gist.github.com/jsuarezruiz/341a2eea56dd9205729c317912b0bee5#file-gistfile1-txt-L362

[softlion]

I'm unable to reproduce the issue in a demo project.
So i'll double check that all calls are done on the main thread.

[acuntex]

I can also confirm that I have days where it does not happen at all. But suddenly when my system is under load I sometimes get these exceptions in the iOS simulator.

I was using the ObservableRangeCollection by @jamesmontemagno and did add about 25 elements when the error occurs regularly.

It happens during the ItemThresholdCommand of a CollectionView, so my guess was that it's on the main thread anyway.

[softlion]

Ok I have a repro project ! I'll post it here.

Btw there are 3 different issues triggered by the same bug.
1

{System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.Parameter name: index at System.Collections.Generic.List1[T].get_Item (System.Int32 index) [0x00009] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/external/corefx/src/Common/src/CoreLib/System/Collections/Generic/List.cs:161 at System.Collections.ObjectModel.Collection1[T].System.Collections.IList.get_Item (System.Int32 index) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/external/corefx/src/Common/src/CoreLib/System/Collections/ObjectModel/Collection.cs:266 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.ElementAt (System.Int32 index) [0x0000f] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:254 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.get_Item (System.Int32 index) [0x00000] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:40 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.get_Item (Foundation.NSIndexPath indexPath) [0x00019] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:96 at Xamarin.Forms.Platform.iOS.ItemsViewController1[TItemsView].UpdateTemplatedCell (Xamarin.Forms.Platform.iOS.TemplatedCell cell, Foundation.NSIndexPath indexPath) [0x00012] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ItemsViewController.cs:199 at Xamarin.Forms.Platform.iOS.ItemsViewController1[TItemsView].GetCell (UIKit.UICollectionView collectionView, Foundation.NSIndexPath indexPath) [0x00033] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ItemsViewController.cs:84 at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr) at UIKit.UIApplication.Main (System.String[] args, System.IntPtr principal, System.IntPtr delegate) [0x00005] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/UIKit/UIApplication.cs:86 at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x0000e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/UIKit/UIApplication.cs:65 at ReproApp.iOS.Application.Main (System.String[] args) [0x00001] in D:\repos\ReproApp\ReproApp.iOS\Main.cs:17 }

2

[CollectionView] An attempt to prepare a layout while a prepareLayout call was already in progress (i.e. reentrant call) has been ignored. Please file a bug. UICollectionView instance is (<UICollectionView: 0x7f9e79f4f400; frame = (0 0; 375 320); clipsToBounds = YES; autoresize = W+H; gestureRecognizers = <NSArray: 0x600002561410>; layer = <CALayer: 0x600002c2dc20>; contentOffset: {0, 0}; contentSize: {375, 0}; adjustedContentInset: {0, 0, 0, 0}> collection view layout: <Xamarin_Forms_Platform_iOS_ListViewLayout: 0x7f9e7863af60>)

3

{System.Threading.SemaphoreFullException: Adding the specified count to the semaphore would cause it to exceed its maximum count. at System.Threading.SemaphoreSlim.Release (System.Int32 releaseCount) [0x0004c] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/threading/SemaphoreSlim.cs:795 at System.Threading.SemaphoreSlim.Release () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/threading/SemaphoreSlim.cs:760 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.Reload () [0x00098] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:146 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.Add (System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x00037] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:165 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.CollectionChanged (System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x00061] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:117 at Xamarin.Forms.Platform.iOS.ObservableItemsSource.CollectionChanged (System.Object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs args) [0x000b7] in D:\a\1\s\Xamarin.Forms.Platform.iOS\CollectionView\ObservableItemsSource.cs:108 at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.b__7_0 (System.Object state) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1021 at Foundation.NSAsyncSynchronizationContextDispatcher.Apply () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/Foundation/NSAction.cs:178 --- End of stack trace from previous location where exception was thrown --- at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr) at UIKit.UIApplication.Main (System.String[] args, System.IntPtr principal, System.IntPtr delegate) [0x00005] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/UIKit/UIApplication.cs:86 at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x0000e] in /Library/Frameworks/Xamarin.iOS.framework/Versions/13.20.2.2/src/Xamarin.iOS/UIKit/UIApplication.cs:65 at ReproApp.iOS.Application.Main (System.String[] args) [0x00001] in D:\repos\ReproApp\ReproApp.iOS\Main.cs:17 } | System.Threading.SemaphoreFullException

[softlion]

Repro app: https://github.com/softlion/ReproFormsCollectionViewBug
Run the app on an iOS simulator or device.

Tested on iOS 12.4 / iPhone 6 simulator.
But happens on all iOS OS and devices.

Does not happen on Android / UWP.

This app will trigger one of the 3 above crashes, which comes from different synchronization bugs.
The key is the change of the IsVisible property just before or after items have been added to the collectionviewsource.