Crash in GeofenceImplementation constructor when accessing Regions.Values.ToList

[jasonmereckixpo]

Our app has reported a crash, the exception is `

System.ArgumentException: Destination array is not long enough to copy all the items in the collection. Check array index and length.

with the following stack trace

Dictionary`2+ValueCollection[TKey,TValue].CopyTo (TValue[] array, System.Int32 index)
System.Collections.Generic.List`1[T]..ctor (System.Collections.Generic.IEnumerable`1[T] collection) [0x0003b] in <d4a23bbd2f544c30a48c44dd622ce09f>:0
Enumerable.ToList[TSource] (System.Collections.Generic.IEnumerable`1[T] source)
<.ctor>b__42_0 (System.Boolean locationIsEnabled)
GeofenceImplementation+<>c__DisplayClass43_0.<IsLocationEnabled>b__0 (Android.Gms.Location.LocationSettingsResult locationSettingsResult)
ResultCallback`1[TResult].OnResult (Java.Lang.Object result)

Note that System.ArgumentException is a documented exception thrown from the Dictionary.ValueCollection.CopyTo method:

https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.valuecollection.copyto?view=netframework-4.8#System_Collections_Generic_Dictionary_2_ValueCollection_CopyTo__1___System_Int32_

Although our version is a little older, the same pattern in the GeofencePlugin code exists in the current version. I'm fairly certain the issue is when the constructor calls StartMonitoring(Regions.Values.ToList());

https://github.com/CrossGeeks/GeofencePlugin/blob/b5de5e77e7666ae5b82a6d1d7891f87016dda2f1/src/Plugin.Geofence/GeofenceImplementation.android.cs#L128

Inside StartMonitoring, access to the mRegions instance is protected by a lock. However, the call to Regions.Values.ToList() (which seems to be a reference to mRegions) will be evaluated before passing the results to the StartMonitoring method and is not protected by the same lock. As a result, one thread may modify the underlying Dictionary inside the lock while another thread is evaluating Regions.Values.ToList() outside the lock.

Instead, change the method to be a no-arg StartMonitoring method, don't evaluate and pass the list. Then inside that method, evaluate the list inside the code block protected by the lock.

[rdelrosario]

@jasonmereckixpo Thanks for fixing this issue.