Skip to content

Commit

Permalink
11853 - iOS - fix concurency problem with batch updates in Observable…
Browse files Browse the repository at this point in the history
…ItemsSource- refactored ObservableItemsSource and ObservableGroupedSource to not use SemaphoreSlim- ObservableItemsSource now uses a copy of the original items source to keep it in sync with the state of the UICollectionView- added warnings for using BatchUpdate on the UICollectionView- fixes xamarin#11853
  • Loading branch information
bruzkovsky committed Nov 30, 2020
1 parent 7460140 commit c81ebc6
Show file tree
Hide file tree
Showing 6 changed files with 294 additions and 321 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.Linq;
using Xamarin.Forms.CustomAttributes;

#if UITEST
using Xamarin.UITest;
using NUnit.Framework;
using Xamarin.Forms.Core.UITests;
#endif

namespace Xamarin.Forms.Controls.Issues
{
[Issue(IssueTracker.Github, 11853,
"[Bug][iOS] Concurrent issue leading to crash in SemaphoreSlim.Release in ObservableItemsSource",
platformsAffected: PlatformAffected.iOS)]
public class Issue11853 : TestContentPage
{
const string Success = "Success";
const string Run = "Run";

ObservableCollection<string> _items;

protected override void Init()
{
Title = "Issue 11853";

var descriptionLabel = new Label { Text = "Press \"Start test\", if the App doesn't crash, it passed." };
var successLabel = new Label { Text = Success, FontSize = Device.GetNamedSize(NamedSize.Large, typeof(Label)), IsVisible = false };

var collectionView = new CollectionView();
ResetItemsSource(collectionView);

var startButton = new Button { Text = Run };
startButton.Clicked += (sender, args) =>
{
ResetTest(successLabel, collectionView);

var random = new Random();
_items.RemoveAt(random.Next(0, 99));
_items.RemoveAt(random.Next(0, 98));
_items.RemoveAt(random.Next(0, 97));
_items.Clear();
successLabel.IsVisible = true;
};
var resetButton = new Button { Text = "Reset test" };
resetButton.Clicked += (sender, args) =>
{
ResetTest(successLabel, collectionView);
};

Content = new StackLayout { Children = { descriptionLabel, successLabel, collectionView, startButton, resetButton } };
}

void ResetTest(VisualElement successLabel, ItemsView collectionView)
{
successLabel.IsVisible = false;
if (_items.Count != 100)
ResetItemsSource(collectionView);
}

void ResetItemsSource(ItemsView collectionView)
{
_items = new ObservableCollection<string>(Create100StringItems());

void OnItemsOnCollectionChanged(object sender, NotifyCollectionChangedEventArgs args) =>
System.Diagnostics.Debug.WriteLine("items.CollectionChanged: {0} (new: {1}, old: {2})", args.Action,
args.NewItems?.Count, args.OldItems?.Count);

_items.CollectionChanged += OnItemsOnCollectionChanged;

collectionView.ItemsSource = _items;
}

static IEnumerable<string> Create100StringItems()
{
// create 100 items from Item 1 - Item 100
return Enumerable.Range(1, 100).Select(i => $"Item {i}");
}

#if UITEST
[Category(UITestCategories.CollectionView)]
[Test]
public void GivenAnObservableCollection_WhenItemsAreRemovedAndCollectionIsCleared_ThenApplicationDoesNotCrash()
{
RunningApp.WaitForElement(Run);
RunningApp.Tap(Run);
RunningApp.WaitForElement(Success);
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,21 @@ ContentPage FirstPage()
var button2 = new Button() { Text = Add2Label, AutomationId = Add2 };
button2.Clicked += Button2Clicked;

var layout = new StackLayout { Children = { instructions, button1, button2 } };
var button3 = new Button { Text = "Go back" };
button3.Clicked += Button3OnClicked;

var layout = new StackLayout { Children = { instructions, button1, button2, button3 } };

page.Content = layout;

return page;
}

void Button3OnClicked(object sender, EventArgs e)
{
Navigation.PopModalAsync();
}

void Button1Clicked(object sender, EventArgs e)
{
_source.Insert(0, Success);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildThisFileDirectory)CollectionViewGroupTypeIssue.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue11853.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue12153.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue10324.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Github9536.xaml.cs">
Expand Down
14 changes: 9 additions & 5 deletions Xamarin.Forms.Platform.iOS/CollectionView/ListSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,25 @@ namespace Xamarin.Forms.Platform.iOS
{
sealed class ListSource : List<object>, IItemsViewSource
{
public ListSource()
readonly int _section;

public ListSource(int section = 0)
{
_section = section;
}

public ListSource(IEnumerable<object> enumerable) : base(enumerable)
public ListSource(IEnumerable<object> enumerable, int section = 0) : base(enumerable)
{

_section = section;
}

public ListSource(IEnumerable enumerable)
public ListSource(IEnumerable enumerable, int section = 0)
{
foreach (object item in enumerable)
{
Add(item);
}
_section = section;
}

public void Dispose()
Expand All @@ -33,7 +37,7 @@ public object this[NSIndexPath indexPath]
{
get
{
if (indexPath.Section > 0)
if (indexPath.Section != _section)
{
throw new ArgumentOutOfRangeException(nameof(indexPath));
}
Expand Down
Loading

0 comments on commit c81ebc6

Please sign in to comment.