Fix the potential ConcurrentModificationException in unregisterListener (#27)

Author: arun251

launchdarkly-android-client/build.gradle

@@ -4,6 +4,7 @@ apply plugin: 'signing'
 apply plugin: 'org.ajoberstar.github-pages'
 // make sure this line comes *after* you apply the Android plugin
 apply plugin: 'com.getkeepsafe.dexcount'
+apply plugin: 'io.codearte.nexus-staging'
 
 allprojects {
     group = 'com.launchdarkly'
@@ -77,6 +78,7 @@ buildscript {
         classpath 'org.ajoberstar:gradle-git:1.5.0-rc.1'
         classpath 'com.github.jengelman.gradle.plugins:shadow:1.2.3'
         classpath 'com.getkeepsafe.dexcount:dexcount-gradle-plugin:0.6.3'
+        classpath "io.codearte.gradle.nexus:gradle-nexus-staging-plugin:0.8.0"
     }
 }
 
@@ -117,6 +119,10 @@ signing {
     sign configurations.archives
 }
 
+nexusStaging {
+    packageGroup = "com.launchdarkly"
+}
+
 uploadArchives {
     repositories {
         mavenDeployer {

launchdarkly-android-client/src/androidTest/java/com/launchdarkly/android/UserManagerTest.java

@@ -3,6 +3,7 @@
 import android.content.SharedPreferences;
 import android.support.test.rule.ActivityTestRule;
 import android.support.test.runner.AndroidJUnit4;
+import android.util.Pair;
 
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -18,12 +19,16 @@
 import org.junit.runner.RunWith;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 
 import static com.google.common.util.concurrent.Futures.immediateFailedFuture;
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertTrue;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.reset;
 
@@ -110,6 +115,42 @@ public void TestCanStoreExactly5Users() throws InterruptedException {
         assertFlagValue(flagKey, user5);
     }
 
+    @Test
+    public void TestRegisterUnregisterListener() {
+        FeatureFlagChangeListener listener = new FeatureFlagChangeListener() {
+            @Override
+            public void onFeatureFlagChange(String flagKey) {
+            }
+        };
+
+        userManager.registerListener("key", listener);
+        Collection<Pair<FeatureFlagChangeListener, SharedPreferences.OnSharedPreferenceChangeListener>> listeners = userManager.getListenersByKey("key");
+        assertNotNull(listeners);
+        assertFalse(listeners.isEmpty());
+
+        userManager.unregisterListener("key", listener);
+        listeners = userManager.getListenersByKey("key");
+        assertNotNull(listeners);
+        assertTrue(listeners.isEmpty());
+    }
+
+    @Test
+    public void TestUnregisterListenerWithDuplicates() {
+        FeatureFlagChangeListener listener = new FeatureFlagChangeListener() {
+            @Override
+            public void onFeatureFlagChange(String flagKey) {
+            }
+        };
+
+        userManager.registerListener("key", listener);
+        userManager.registerListener("key", listener);
+        userManager.unregisterListener("key", listener);
+
+        Collection<Pair<FeatureFlagChangeListener, SharedPreferences.OnSharedPreferenceChangeListener>> listeners = userManager.getListenersByKey("key");
+        assertNotNull(listeners);
+        assertTrue(listeners.isEmpty());
+    }
+
     private Future<Void> setUser(String userKey, JsonObject flags) {
         LDUser user = new LDUser.Builder(userKey).build();
         ListenableFuture<JsonObject> jsonObjectFuture = Futures.immediateFuture(flags);

launchdarkly-android-client/src/main/java/com/launchdarkly/android/UserManager.java

@@ -10,7 +10,7 @@
 import android.util.Pair;
 
 import com.google.common.base.Function;
-import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
@@ -19,10 +19,13 @@
 import com.google.gson.JsonObject;
 
 import java.io.File;
+
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -49,7 +52,7 @@ class UserManager {
 
     private final Application application;
     // Maintains references enabling (de)registration of listeners for realtime updates
-    private final Multimap<String, Pair<FeatureFlagChangeListener, OnSharedPreferenceChangeListener>> listeners = ArrayListMultimap.create();
+    private final Multimap<String, Pair<FeatureFlagChangeListener, OnSharedPreferenceChangeListener>> listeners = HashMultimap.create();
 
     // The current user- we'll always fetch this user from the response we get from the api
     private SharedPreferences currentUserSharedPrefs;
@@ -153,6 +156,10 @@ public Void apply(JsonObject input) {
         });
     }
 
+    Collection<Pair<FeatureFlagChangeListener, OnSharedPreferenceChangeListener>> getListenersByKey(String key) {
+        return listeners.get(key);
+    }
+
     void registerListener(final String key, final FeatureFlagChangeListener listener) {
         OnSharedPreferenceChangeListener sharedPrefsListener = new OnSharedPreferenceChangeListener() {
             @Override
@@ -172,11 +179,13 @@ public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, Strin
 
     void unregisterListener(String key, FeatureFlagChangeListener listener) {
         synchronized (listeners) {
-            for (Pair<FeatureFlagChangeListener, OnSharedPreferenceChangeListener> pair : listeners.get(key)) {
+            Iterator<Pair<FeatureFlagChangeListener, OnSharedPreferenceChangeListener>> it = listeners.get(key).iterator();
+            while (it.hasNext()) {
+                Pair<FeatureFlagChangeListener, OnSharedPreferenceChangeListener> pair = it.next();
                 if (pair.first.equals(listener)) {
                     Log.d(TAG, "Removing listener for key: [" + key + "]");
                     activeUserSharedPrefs.unregisterOnSharedPreferenceChangeListener(pair.second);
-                    listeners.remove(key, pair);
+                    it.remove();
                 }
             }
         }