Bug 1325021 - Safely removing the listener in the collection to prevent concurrency issues. r?sebastian draft
authorcnevinc <cnevinc@livemail.tw>
Sun, 02 Apr 2017 18:01:45 +0800
changeset 554753 79a518c3dee30586d775f937b04c1a7deca6e472
parent 554473 5bd82c644e1fdc15b92ca9e34b31841151b44957
child 622429 5864b3762b7d19b519deded670f7d6c31b3c91d1
push id52042
push userbmo:cnevinchen@gmail.com
push dateSun, 02 Apr 2017 10:29:25 +0000
reviewerssebastian
bugs1325021
milestone55.0a1
Bug 1325021 - Safely removing the listener in the collection to prevent concurrency issues. r?sebastian MozReview-Commit-ID: AxPnucWLD57
mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
@@ -20,16 +20,17 @@ import org.json.JSONObject;
 
 import android.os.Bundle;
 import android.os.Handler;
 import android.util.Log;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 @RobocopTarget
 public final class EventDispatcher extends JNIObject {
     private static final String LOGTAG = "GeckoEventDispatcher";
@@ -152,23 +153,37 @@ public final class EventDispatcher exten
                                         final String[] events) {
         synchronized (listenersMap) {
             for (final String event : events) {
                 if (event == null) {
                     continue;
                 }
                 List<T> listeners = listenersMap.get(event);
                 if ((listeners == null ||
-                     !listeners.remove(listener)) && !AppConstants.RELEASE_OR_BETA) {
+                     !safelyRemove(listener, listeners)) && !AppConstants.RELEASE_OR_BETA) {
                     throw new IllegalArgumentException(event + " was not registered");
                 }
             }
         }
     }
 
+    // See https://docs.oracle.com/javase/tutorial/collections/interfaces/collection.html
+    private <T> boolean safelyRemove(T listener, List<T> listeners) {
+        boolean found = false;
+        synchronized (listener) {
+            for (Iterator<?> it = listeners.iterator(); it.hasNext(); ) {
+                if ((it.next() == listener)) {
+                    it.remove();
+                    found = true;
+                }
+            }
+            return found;
+        }
+    }
+
     public void registerGeckoThreadListener(final BundleEventListener listener,
                                             final String... events) {
         checkNotRegisteredElsewhere(mGeckoThreadListeners, events);
 
         // For listeners running on the Gecko thread, we want to notify the listeners
         // outside of our synchronized block, because the listeners may take an
         // indeterminate amount of time to run. Therefore, to ensure concurrency when
         // iterating the list outside of the synchronized block, we use a