Fixes ConcurrentModificationException when client is disconnected on …

…connect (#163)

* Fixes ConcurrentModificationException when client is disconnected on connect

Iterates a over copy of the connection callbacks set when the fused location service is connected or suspended to prevent a ConcurrentModificationException if callbacks are removed during iteration.

This issue occurs when there are multiple clients and a single client is disconnected in one of its `ConnectionCallbacks` methods.

Fixes #162

* Make copy `ArrayList` instead of `HashSet` to save memory

lost/src/main/java/com/mapzen/android/lost/internal/FusedLocationServiceConnectionManager.java

@@ -6,6 +6,7 @@
 import android.content.Context;
 import android.os.IBinder;
 
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -80,7 +81,8 @@ public void onServiceConnected(IBinder binder) {
       }
 
       if (!connectionCallbacks.isEmpty()) {
-        for (LostApiClient.ConnectionCallbacks callbacks : connectionCallbacks) {
+        final ArrayList<ConnectionCallbacks> copy = new ArrayList<>(connectionCallbacks);
+        for (LostApiClient.ConnectionCallbacks callbacks : copy) {
           callbacks.onConnected();
         }
       }
@@ -91,7 +93,8 @@ public void onServiceDisconnected() {
     if (connectState != ConnectState.IDLE) {
       connectState = ConnectState.IDLE;
       if (!connectionCallbacks.isEmpty()) {
-        for (LostApiClient.ConnectionCallbacks callbacks : connectionCallbacks) {
+        final ArrayList<ConnectionCallbacks> copy = new ArrayList<>(connectionCallbacks);
+        for (LostApiClient.ConnectionCallbacks callbacks : copy) {
           callbacks.onConnectionSuspended();
         }
       }

lost/src/test/java/com/mapzen/android/lost/internal/FusedLocationServiceConnectionManagerTest.java

@@ -165,6 +165,15 @@ public class FusedLocationServiceConnectionManagerTest {
     assertThat(eventCallbacks.connectedOnServiceConnected).isTrue();
   }
 
+  @Test public void onServiceConnected_shouldHandleCallbacksRemovedOnConnect() throws Exception {
+    LostApiClient.ConnectionCallbacks connectionCallbacks1 = new RemoveOnConnected();
+    LostApiClient.ConnectionCallbacks connectionCallbacks2 = new RemoveOnConnected();
+    connectionManager.connect(null, connectionCallbacks1);
+    connectionManager.addCallbacks(connectionCallbacks2);
+    connectionManager.onServiceConnected(null);
+    assertThat(connectionManager.getConnectionCallbacks()).isEmpty();
+  }
+
   @Test public void onServiceDisconnected_shouldCallConnectionCallback() {
     TestConnectionCallbacks connectionCallbacks = new TestConnectionCallbacks();
     connectionManager.connect(null, connectionCallbacks);
@@ -194,6 +203,15 @@ public class FusedLocationServiceConnectionManagerTest {
     assertThat(connectionCallbacks.isConnected()).isTrue();
   }
 
+  @Test public void onServiceDisconnected_shouldHandleCallbacksRemovedOnSuspend() throws Exception {
+    LostApiClient.ConnectionCallbacks connectionCallbacks1 = new RemoveOnSuspended();
+    LostApiClient.ConnectionCallbacks connectionCallbacks2 = new RemoveOnSuspended();
+    connectionManager.connect(null, connectionCallbacks1);
+    connectionManager.addCallbacks(connectionCallbacks2);
+    connectionManager.onServiceDisconnected();
+    assertThat(connectionManager.getConnectionCallbacks()).isEmpty();
+  }
+
   class TestEventCallbacks implements FusedLocationServiceConnectionManager.EventCallbacks {
 
     private boolean connected = false;
@@ -231,4 +249,22 @@ public void onDisconnect() {
       idleOnDisconnect = !connectionManager.isConnected() && !connectionManager.isConnecting();
     }
   }
+
+  private class RemoveOnConnected implements LostApiClient.ConnectionCallbacks {
+    @Override public void onConnected() {
+      connectionManager.removeCallbacks(this);
+    }
+
+    @Override public void onConnectionSuspended() {
+    }
+  }
+
+  private class RemoveOnSuspended implements LostApiClient.ConnectionCallbacks {
+    @Override public void onConnected() {
+    }
+
+    @Override public void onConnectionSuspended() {
+      connectionManager.removeCallbacks(this);
+    }
+  }
 }