CB-14020: (ios) Fix "Collection was mutated while being enumerated" crash (#104)

elijahr · jcesarmobile · commit ba455956a576 · 2018-07-12T12:17:17.000+02:00
* Avoid concurrent modification on callbacks list.

Doesn't quite prevent it, but should be way better.

* Actually synchronize things
diff --git a/src/ios/CDVLocation.m b/src/ios/CDVLocation.m
@@ -6,9 +6,9 @@ Licensed to the Apache Software Foundation (ASF) under one
  to you under the Apache License, Version 2.0 (the
  "License"); you may not use this file except in compliance
  with the License.  You may obtain a copy of the License at
-
+ 
  http://www.apache.org/licenses/LICENSE-2.0
-
+ 
  Unless required by applicable law or agreed to in writing,
  software distributed under the License is distributed on an
  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -65,7 +65,7 @@ - (void)pluginInitialize
 - (BOOL)isAuthorized
 {
     BOOL authorizationStatusClassPropertyAvailable = [CLLocationManager respondsToSelector:@selector(authorizationStatus)]; // iOS 4.2+
-
+    
     if (authorizationStatusClassPropertyAvailable) {
         NSUInteger authStatus = [CLLocationManager authorizationStatus];
 #ifdef __IPHONE_8_0
@@ -75,7 +75,7 @@ - (BOOL)isAuthorized
 #endif
         return (authStatus == kCLAuthorizationStatusAuthorizedAlways) || (authStatus == kCLAuthorizationStatusNotDetermined);
     }
-
+    
     // by default, assume YES (for iOS < 4.2)
     return YES;
 }
@@ -84,7 +84,7 @@ - (BOOL)isLocationServicesEnabled
 {
     BOOL locationServicesEnabledInstancePropertyAvailable = [self.locationManager respondsToSelector:@selector(locationServicesEnabled)]; // iOS 3.x
     BOOL locationServicesEnabledClassPropertyAvailable = [CLLocationManager respondsToSelector:@selector(locationServicesEnabled)]; // iOS 4.x
-
+    
     if (locationServicesEnabledClassPropertyAvailable) { // iOS 4.x
         return [CLLocationManager locationServicesEnabled];
     } else {
@@ -112,10 +112,10 @@ - (void)startLocation:(BOOL)enableHighAccuracy
         }
         // PERMISSIONDENIED is only PositionError that makes sense when authorization denied
         [self returnLocationError:PERMISSIONDENIED withMessage:message];
-
+        
         return;
     }
-
+    
 #ifdef __IPHONE_8_0
     NSUInteger code = [CLLocationManager authorizationStatus];
     if (code == kCLAuthorizationStatusNotDetermined && ([self.locationManager respondsToSelector:@selector(requestAlwaysAuthorization)] || [self.locationManager respondsToSelector:@selector(requestWhenInUseAuthorization)])) { //iOS8+
@@ -130,7 +130,7 @@ - (void)startLocation:(BOOL)enableHighAccuracy
         return;
     }
 #endif
-
+    
     // Tell the location manager to start notifying us of location updates. We
     // first stop, and then start the updating to ensure we get at least one
     // update, even if our location did not change.
@@ -157,7 +157,7 @@ - (void)_stopLocation
         if (![self isLocationServicesEnabled]) {
             return;
         }
-
+        
         [self.locationManager stopUpdatingLocation];
         __locationStarted = NO;
         __highAccuracyEnabled = NO;
@@ -169,14 +169,16 @@ - (void)locationManager:(CLLocationManager*)manager
            fromLocation:(CLLocation*)oldLocation
 {
     CDVLocationData* cData = self.locationData;
-
+    
     cData.locationInfo = newLocation;
-    if (self.locationData.locationCallbacks.count > 0) {
-        for (NSString* callbackId in self.locationData.locationCallbacks) {
-            [self returnLocationInfo:callbackId andKeepCallback:NO];
+    @synchronized (self.locationData.locationCallbacks) {
+        if (self.locationData.locationCallbacks.count > 0) {
+            for (NSString* callbackId in self.locationData.locationCallbacks) {
+                [self returnLocationInfo:callbackId andKeepCallback:NO];
+            }
+            
+            [self.locationData.locationCallbacks removeAllObjects];
         }
-
-        [self.locationData.locationCallbacks removeAllObjects];
     }
     if (self.locationData.watchCallbacks.count > 0) {
         for (NSString* timerId in self.locationData.watchCallbacks) {
@@ -193,7 +195,7 @@ - (void)getLocation:(CDVInvokedUrlCommand*)command
     [self.commandDelegate runInBackground:^{
         NSString* callbackId = command.callbackId;
         BOOL enableHighAccuracy = [[command argumentAtIndex:0] boolValue];
-
+        
         if ([self isLocationServicesEnabled] == NO) {
             NSMutableDictionary* posError = [NSMutableDictionary dictionaryWithCapacity:2];
             [posError setObject:[NSNumber numberWithInt:PERMISSIONDENIED] forKey:@"code"];
@@ -205,14 +207,18 @@ - (void)getLocation:(CDVInvokedUrlCommand*)command
                 self.locationData = [[CDVLocationData alloc] init];
             }
             CDVLocationData* lData = self.locationData;
-            if (!lData.locationCallbacks) {
-                lData.locationCallbacks = [NSMutableArray arrayWithCapacity:1];
+            @synchronized (self.locationData.locationCallbacks) {
+                if (!lData.locationCallbacks) {
+                    lData.locationCallbacks = [NSMutableArray arrayWithCapacity:1];
+                }
             }
-
+            
             if (!__locationStarted || (__highAccuracyEnabled != enableHighAccuracy)) {
                 // add the callbackId into the array so we can call back when get data
-                if (callbackId != nil) {
-                    [lData.locationCallbacks addObject:callbackId];
+                @synchronized (self.locationData.locationCallbacks) {
+                    if (callbackId != nil) {
+                        [lData.locationCallbacks addObject:callbackId];
+                    }
                 }
                 // Tell the location manager to start notifying us of heading updates
                 [self startLocation:enableHighAccuracy];
@@ -228,19 +234,19 @@ - (void)addWatch:(CDVInvokedUrlCommand*)command
     NSString* callbackId = command.callbackId;
     NSString* timerId = [command argumentAtIndex:0];
     BOOL enableHighAccuracy = [[command argumentAtIndex:1] boolValue];
-
+    
     if (!self.locationData) {
         self.locationData = [[CDVLocationData alloc] init];
     }
     CDVLocationData* lData = self.locationData;
-
+    
     if (!lData.watchCallbacks) {
         lData.watchCallbacks = [NSMutableDictionary dictionaryWithCapacity:1];
     }
-
+    
     // add the callbackId into the dictionary so we can call back whenever get data
     [lData.watchCallbacks setObject:callbackId forKey:timerId];
-
+    
     if ([self isLocationServicesEnabled] == NO) {
         NSMutableDictionary* posError = [NSMutableDictionary dictionaryWithCapacity:2];
         [posError setObject:[NSNumber numberWithInt:PERMISSIONDENIED] forKey:@"code"];
@@ -258,7 +264,7 @@ - (void)addWatch:(CDVInvokedUrlCommand*)command
 - (void)clearWatch:(CDVInvokedUrlCommand*)command
 {
     NSString* timerId = [command argumentAtIndex:0];
-
+    
     if (self.locationData && self.locationData.watchCallbacks && [self.locationData.watchCallbacks objectForKey:timerId]) {
         [self.locationData.watchCallbacks removeObjectForKey:timerId];
         if([self.locationData.watchCallbacks count] == 0) {
@@ -276,7 +282,7 @@ - (void)returnLocationInfo:(NSString*)callbackId andKeepCallback:(BOOL)keepCallb
 {
     CDVPluginResult* result = nil;
     CDVLocationData* lData = self.locationData;
-
+    
     if (lData && !lData.locationInfo) {
         // return error
         result = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageToErrorObject:POSITIONUNAVAILABLE];
@@ -292,7 +298,7 @@ - (void)returnLocationInfo:(NSString*)callbackId andKeepCallback:(BOOL)keepCallb
         [returnInfo setObject:[NSNumber numberWithDouble:lInfo.altitude] forKey:@"altitude"];
         [returnInfo setObject:[NSNumber numberWithDouble:lInfo.coordinate.latitude] forKey:@"latitude"];
         [returnInfo setObject:[NSNumber numberWithDouble:lInfo.coordinate.longitude] forKey:@"longitude"];
-
+        
         result = [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsDictionary:returnInfo];
         [result setKeepCallbackAsBool:keepCallback];
     }
@@ -304,17 +310,20 @@ - (void)returnLocationInfo:(NSString*)callbackId andKeepCallback:(BOOL)keepCallb
 - (void)returnLocationError:(NSUInteger)errorCode withMessage:(NSString*)message
 {
     NSMutableDictionary* posError = [NSMutableDictionary dictionaryWithCapacity:2];
-
+    
     [posError setObject:[NSNumber numberWithUnsignedInteger:errorCode] forKey:@"code"];
     [posError setObject:message ? message:@"" forKey:@"message"];
     CDVPluginResult* result = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR messageAsDictionary:posError];
-
-    for (NSString* callbackId in self.locationData.locationCallbacks) {
-        [self.commandDelegate sendPluginResult:result callbackId:callbackId];
+    
+    
+    @synchronized (self.locationData.locationCallbacks) {
+        for (NSString* callbackId in self.locationData.locationCallbacks) {
+            [self.commandDelegate sendPluginResult:result callbackId:callbackId];
+        }
+        
+        [self.locationData.locationCallbacks removeAllObjects];
     }
-
-    [self.locationData.locationCallbacks removeAllObjects];
-
+    
     for (NSString* callbackId in self.locationData.watchCallbacks) {
         [self.commandDelegate sendPluginResult:result callbackId:callbackId];
     }
@@ -323,7 +332,7 @@ - (void)returnLocationError:(NSUInteger)errorCode withMessage:(NSString*)message
 - (void)locationManager:(CLLocationManager*)manager didFailWithError:(NSError*)error
 {
     NSLog(@"locationManager::didFailWithError %@", [error localizedFailureReason]);
-
+    
     CDVLocationData* lData = self.locationData;
     if (lData && __locationStarted) {
         // TODO: probably have to once over the various error codes and return one of:
@@ -336,10 +345,10 @@ - (void)locationManager:(CLLocationManager*)manager didFailWithError:(NSError*)e
         }
         [self returnLocationError:positionError withMessage:[error localizedDescription]];
     }
-
+    
     if (error.code != kCLErrorLocationUnknown) {
-      [self.locationManager stopUpdatingLocation];
-      __locationStarted = NO;
+        [self.locationManager stopUpdatingLocation];
+        __locationStarted = NO;
     }
 }
 
