Bug 60140 - ArgumentNullException on MapRenderer.RemoveAnnotations
Summary: ArgumentNullException on MapRenderer.RemoveAnnotations
Status: RESOLVED FIXED
Alias: None
Product: Forms
Classification: Xamarin
Component: iOS ()
Version: 2.4.0
Hardware: PC Mac OS
: High major
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2017-10-11 19:51 UTC by clay.zuvich
Modified: 2018-01-04 17:38 UTC (History)
5 users (show)

Tags: ios, maps, ac, fr
Is this bug a regression?: Yes
Last known good build: 2.3.4.270


Attachments
Stack Trace (16.20 KB, text/rtf)
2017-10-11 19:52 UTC, clay.zuvich
Details
Demo Project (151.06 KB, application/zip)
2017-10-11 23:12 UTC, clay.zuvich
Details
ListView Bug (151.17 KB, application/zip)
2017-10-12 13:58 UTC, clay.zuvich
Details


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 on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description clay.zuvich 2017-10-11 19:51:50 UTC
Component
https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Maps.iOS/MapRenderer.cs

OS
iOS 11

Summary/Description

I am seeing a System.ArgumentNullException on the OnElementChanged/OnCollectionChanged of the MapRenderer on iOS.  The exception says "System.ArgumentNullException: Value cannot be null.
Parameter name: annotations".

It's hard to reproduce in a repo project since I think it's a timing issue; however, I have attached a stack trace.  I've looked at the renderer code on iOS, and I see the RemoveAnnotations is called in the OnElementChanged.OnCollectionChanged.Reset trigger (Line 278 is the trigger, and the code that throws the exception is Line 375).

For whatever reason, my guess is the MKMapView.Annotations is now returning NULL instead of an empty array.  I don't think there's any workaround for this issue in a custom renderer, and it's definitely a show stopper for anyone using maps. 

The way to reproduce it is to get the Annotations to return null, then it will throw the exception.
Comment 1 clay.zuvich 2017-10-11 19:52:36 UTC
Created attachment 25279 [details]
Stack Trace

Attached a stack trace.
Comment 2 clay.zuvich 2017-10-11 21:19:01 UTC
Ok good news.  I think it has something to do with the MKMapView being disposed, and then it's being referenced in the collection.  I did manual dispose of the MKMapView and I received the exact exception (ArgumentNull with annotations).

I'm not sure why the MKMapView is being disposed though.
Comment 3 clay.zuvich 2017-10-11 23:12:08 UTC
I downgraded to Forms 2.3, and the issue is no longer there.  It seems to be an issue with Forms 2.4.  From what I can tell, the MKMapView (Control) is disposed when OnElementChanged.OldElement is not null.  Then, the OnElementChanged.NewElement is triggered when map is created on a new page; however, the MKMapView is pooled and for some reason has been disposed.

I can't seem to trigger a Dispose in a demo app, but I can see it's an issue if the MapPool's MKMapView instance is disposed and then re-used.

I did attach a sample project which calls Dispose, and then you can see the ArgumentException the next time OnElementChanged is fired.  I deleted all of the packages.

Tested on iOS 11 simulator.
Comment 4 clay.zuvich 2017-10-11 23:12:46 UTC
Created attachment 25281 [details]
Demo Project
Comment 5 clay.zuvich 2017-10-12 13:56:05 UTC
Ok better news!  I got it to reproduce consistently.  The issue happens when using a custom renderer Map in a ListView header (which is what I'm doing in a project).

I've attached a project that reproduces the issue named "ListView Bug". I looked at the source code in Forms, and the following gist is suspect.

https://github.com/xamarin/Xamarin.Forms/commit/20bfa5b5038ef3357f6048b1cb13105033650069#diff-8a8fd41892c357f2e01d26820eeb2ea1
Comment 6 clay.zuvich 2017-10-12 13:58:32 UTC
Created attachment 25285 [details]
ListView Bug
Comment 7 clay.zuvich 2017-10-12 13:59:27 UTC
Steps to Reproduce

In order to reproduce in the project, tap on an item on the first page. Navigate back to the first page. Then tap on an item again which navigates to the next page.
Comment 8 David Ortinau [MSFT] 2017-10-13 14:28:39 UTC
Thanks for the detailed report! I'm also able to reproduce this issue, so marking it as a confirmed regression.

2.3.4.270: GOOD
2.4.0.282: BAD
2.4.1.146-nightly: BAD
Comment 10 Chris King 2017-11-17 00:56:50 UTC
Please do not dispose the map view in your default renderer. We pool the map view and disposing it corrupts the pool.
Comment 11 clay.zuvich 2017-12-08 16:54:06 UTC
The attached project named ListView Bug does not dispose of the Renderer.  It uses the Map in a ListView header which for some reason causes the Map to get disposed in the Pool.  

When that happens, the ArgumentException gets raised.  I don't think a null check will work in this scenario unless the map isn't being disposed anymore.
Comment 12 clay.zuvich 2017-12-08 17:15:37 UTC
I just verified this fix does not work in the current version of Xamarin Forms.  Can we please update this fix to not resolved? The map gets disposed and is blank in the header.  Please see the "ListView Bug" attached.
Comment 13 clay.zuvich 2017-12-08 20:18:11 UTC
Did some more research... It turns out that the following commit may have resolved the issue, but it was issued 9 days ago (a lot later than this fix).

https://github.com/xamarin/Xamarin.Forms/commit/cda14962711af8173b15ecd65bef589b23d85848#diff-198607f4ce97efcc515a271473524b08

Specifically the DisposeSubviews method fix.

Adding the null check does not resolve the issue at all (only hides it).  Not sure what the UI tests look like, but I think they may need to get reworked as well. Please let me know your findings.  Thanks.
Comment 14 clay.zuvich 2017-12-08 20:40:26 UTC
Nix that last comment.  Turns out that latest fix actually causes the Dispose method to never get called on the MapRenderer.  So, now with the latest fix, the MKMapView is never actually cached.
Comment 15 Rui Marinho 2018-01-04 17:38:46 UTC
Should be fixed on 2.5.0-sr4