Crash in GeofenceImplementation constructor when accessing Regions.Values.ToList
Closed this issue · 1 comments
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:
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());
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.
@jasonmereckixpo Thanks for fixing this issue.