Bug 59830 - Post V2.3.3.193: crash when subscribing/unsubscribing from IVisualElementRenderer.ElementChanged during element change
Summary: Post V2.3.3.193: crash when subscribing/unsubscribing from IVisualElementRend...
Status: CONFIRMED
Alias: None
Product: Forms
Classification: Xamarin
Component: Android ()
Version: 2.4.0
Hardware: PC Mac OS
: Normal major
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2017-09-28 18:45 UTC by Ben Askren
Modified: 2017-09-29 14:31 UTC (History)
3 users (show)

Tags: elementchanged, ac
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 59830 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
CONFIRMED

Description Ben Askren 2017-09-28 18:45:32 UTC
== Description:

After Xamarin.Forms v2.3.3.193, Android applications will crash when subscribing/unsubscribing from IVisualElementRenderer.ElementChanged event in response to an IVisualElementRenderer.ElementChanged event.

== Steps to reproduce:

1. Get sample project here:  https://github.com/baskren/AndroidRendererElementChangedCrash
2. Build and run 
3. Scroll 'til your heart's content
4. Update Xamarin.Forms from v2.3.3.193 to a more recent version
5. Build and run
6. Watch the app crash with a "System.InvalidOperation (Collection was modified; enumeration operation may not execute)" exception.


== Arm chair analysis

v2.3.3.193 and earlier used the following code for Xamarin.Forms.Platform.Android.VisualElementRenderer.OnElementChanged

                protected virtual void OnElementChanged(ElementChangedEventArgs<TElement> e)
		{
			var args = new VisualElementChangedEventArgs(e.OldElement, e.NewElement);
			for (var i = 0; i < _elementChangedHandlers.Count; i++)
				_elementChangedHandlers[i](this, args);

			EventHandler<ElementChangedEventArgs<TElement>> changed = ElementChanged;
			if (changed != null)
				changed(this, e);
		}

Later versions refactored the "for" loop for a "foreach" iterator:

		protected virtual void OnElementChanged(ElementChangedEventArgs<TElement> e)
		{
			var args = new VisualElementChangedEventArgs(e.OldElement, e.NewElement);
			foreach (EventHandler<VisualElementChangedEventArgs> handler in _elementChangedHandlers)
				handler(this, args);

			ElementChanged?.Invoke(this, e);

			ElevationHelper.SetElevation(this, e.NewElement);
		}


Upon each iteration, the current EventHandlers (handler) in _elementChangedHandlers is called.  Thus, if using "foreach", a System.InvalidOperation (Collection was modified; enumeration operation may not execute) exception will be thrown if the _elementChangedHandlers collection is changed (as it is in the provided sample app).

== Earnest plea:

This has been a painful issue for me since 2.3.4 was made available.  The only workaround has been for me to use 2.3.3.193 in my Android builds (it took me 3 days to figure out that much).  However, with fast renderers (v2.4.0+) available for Android, I really would like to move beyond 2.3.3.193.  Your help with this is greatly appreciated.


== System Information

=== Visual Studio Community 2017 for Mac ===

Version 7.1.5 (build 2)
Installation UUID: c3c61da2-4d87-4c39-b8d3-9fbf7e6af430
Runtime:
	Mono 5.2.0.224 (d15-3/14f2c81) (64-bit)
	GTK+ 2.24.23 (Raleigh theme)

	Package version: 502000224

=== NuGet ===

Version: 4.3.0.2418

=== .NET Core ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
	1.1.1
	1.0.4
SDK: /usr/local/share/dotnet/sdk/1.0.3/Sdks
SDK Version: 1.0.3
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/5.2.0/lib/mono/msbuild/15.0/bin/Sdks

=== Xamarin.Profiler ===

Version: 1.5.5
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Apple Developer Tools ===

Xcode 9.0 (13247)
Build 9A235

=== Xamarin.Mac ===

Version: 3.6.3.3 (Visual Studio Community)

=== Xamarin.Android ===

Version: 7.4.5.1 (Visual Studio Community)
Android SDK: /Users/ben/Library/Developer/Xamarin/android-sdk-macosx
	Supported Android versions:
		2.3    (API level 10)
		4.0.3  (API level 15)
		4.1    (API level 16)
		4.2    (API level 17)
		4.3    (API level 18)
		4.4    (API level 19)
		4.4.87 (API level 20)
		5.0    (API level 21)
		5.1    (API level 22)
		6.0    (API level 23)
		7.0    (API level 24)
		7.1    (API level 25)

SDK Tools Version: 26.0.2
SDK Platform Tools Version: 25.0.6
SDK Build Tools Version: 25.0.3

Java SDK: /usr
java version "1.8.0_121"
Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Xamarin.iOS ===

Version: 11.0.0.0 (Visual Studio Community)
Hash: 152b654a
Branch: xcode9
Build date: 2017-09-15 02:25:56-0400

=== Xamarin Inspector ===

Version: 1.3.1
Hash: cbc48dd
Branch: 1.3-release
Build date: Thu, 21 Sep 2017 19:52:53 GMT
Client compatibility: 1

=== Build Information ===

Release ID: 701050002
Git revision: 7afedcaef8e7542e70e3cf8f9bdb26938b8c0876
Build date: 2017-09-15 08:39:58-04
Xamarin addins: 3262aadf811a18c12eac6742532d052b0139a808
Build lane: monodevelop-lion-d15-3-xcode9

=== Operating System ===

Mac OS X 10.12.6
Darwin 16.7.0 Darwin Kernel Version 16.7.0
    Thu Jun 15 17:36:27 PDT 2017
    root:xnu-3789.70.16~2/RELEASE_X86_64 x86_64
Comment 1 Ben Askren 2017-09-28 20:01:30 UTC
I forgot one thing:

== Arm chair recommendation:

It would be great if the code for Xamarin.Forms.Platform.Android.VisualElementRenderer.OnElementChanged could be changed to:


		protected virtual void OnElementChanged(ElementChangedEventArgs<TElement> e)
		{
			var args = new VisualElementChangedEventArgs(e.OldElement, e.NewElement);
                        var elementChangeHandlers = _elementChangedHandlers.ToList();
			foreach (EventHandler<VisualElementChangedEventArgs> handler in elementChangedHandlers)
				handler(this, args);

			ElementChanged?.Invoke(this, e);

			ElevationHelper.SetElevation(this, e.NewElement);
		}

This would assure that all the elements of _elementChangedHandlers are invoked as well as prevent a change to _elementChangedHandlers from causing an the System.InvalidOperandException.
Comment 2 Paul DiPietro [MSFT] 2017-09-29 14:31:00 UTC
The crash does occur on 2.4.0 stable, so for now I'll set this to confirmed until it can be investigated further to determine a course of action.