Bug 745540 - Properly handle multithreaded access to the listener list. r=blassey
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 17 Apr 2012 11:30:34 -0400
changeset 91862 55f527f49d546342968ff99f5d4e14a15de557e5
parent 91861 df9ea73ec1f4a437152e7a75c198f80803c4410b
child 91863 1588b18050f408c386d5eb494de28b84d38e1ba7
push id22480
push useremorley@mozilla.com
push dateWed, 18 Apr 2012 00:48:48 +0000
treeherdermozilla-central@93dfd98900ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblassey
bugs745540
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 745540 - Properly handle multithreaded access to the listener list. r=blassey
mobile/android/base/GeckoAppShell.java
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -111,17 +111,18 @@ public class GeckoAppShell
     static private int sFreeSpace = -1;
     static File sHomeDir = null;
     static private int sDensityDpi = 0;
     private static Boolean sSQLiteLibsLoaded = false;
     private static Boolean sNSSLibsLoaded = false;
     private static Boolean sLibsSetup = false;
     private static File sGREDir = null;
 
-    private static HashMap<String, ArrayList<GeckoEventListener>> mEventListeners;
+    private static Map<String, CopyOnWriteArrayList<GeckoEventListener>> mEventListeners
+            = new HashMap<String, CopyOnWriteArrayList<GeckoEventListener>>();
 
     /* Is the value in sVibrationEndTime valid? */
     private static boolean sVibrationMaybePlaying = false;
 
     /* Time (in System.nanoTime() units) when the currently-playing vibration
      * is scheduled to end.  This value is valid only when
      * sVibrationMaybePlaying is true. */
     private static long sVibrationEndTime = 0;
@@ -1655,26 +1656,37 @@ public class GeckoAppShell
         if (sCamera != null) {
             sCamera.stopPreview();
             sCamera.release();
             sCamera = null;
             sCameraBuffer = null;
         }
     }
 
-    /* This method is referenced by Robocop via reflection. */
+    /**
+     * Adds a listener for a gecko event.
+     * This method is thread-safe and may be called at any time. In particular, calling it
+     * with an event that is currently being processed has the properly-defined behaviour that
+     * any added listeners will not be invoked on the event currently being processed, but
+     * will be invoked on future events of that type.
+     *
+     * This method is referenced by Robocop via reflection.
+     */
     public static void registerGeckoEventListener(String event, GeckoEventListener listener) {
-        if (mEventListeners == null)
-            mEventListeners = new HashMap<String, ArrayList<GeckoEventListener>>();
-
-        if (!mEventListeners.containsKey(event))
-            mEventListeners.put(event, new ArrayList<GeckoEventListener>());
-
-        ArrayList<GeckoEventListener> listeners = mEventListeners.get(event);
-        listeners.add(listener);
+        synchronized (mEventListeners) {
+            CopyOnWriteArrayList<GeckoEventListener> listeners = mEventListeners.get(event);
+            if (listeners == null) {
+                // create a CopyOnWriteArrayList so that we can modify it
+                // concurrently with iterating through it in handleGeckoMessage.
+                // Otherwise we could end up throwing a ConcurrentModificationException.
+                listeners = new CopyOnWriteArrayList<GeckoEventListener>();
+            }
+            listeners.add(listener);
+            mEventListeners.put(event, listeners);
+        }
     }
 
     static SynchronousQueue<Date> sTracerQueue = new SynchronousQueue<Date>();
     public static void fireAndWaitForTracerEvent() {
         getMainHandler().post(new Runnable() { 
                 public void run() {
                     try {
                         sTracerQueue.put(new Date());
@@ -1685,26 +1697,36 @@ public class GeckoAppShell
         });
         try {
             sTracerQueue.take();
         } catch(InterruptedException ie) {
             Log.w(LOGTAG, "exception firing tracer", ie);
         }
     }
 
-    /* This method is referenced by Robocop via reflection. */
+    /**
+     * Remove a previously-registered listener for a gecko event.
+     * This method is thread-safe and may be called at any time. In particular, calling it
+     * with an event that is currently being processed has the properly-defined behaviour that
+     * any removed listeners will still be invoked on the event currently being processed, but
+     * will not be invoked on future events of that type.
+     *
+     * This method is referenced by Robocop via reflection.
+     */
     public static void unregisterGeckoEventListener(String event, GeckoEventListener listener) {
-        if (mEventListeners == null)
-            return;
-
-        if (!mEventListeners.containsKey(event))
-            return;
-
-        ArrayList<GeckoEventListener> listeners = mEventListeners.get(event);
-        listeners.remove(listener);
+        synchronized (mEventListeners) {
+            CopyOnWriteArrayList<GeckoEventListener> listeners = mEventListeners.get(event);
+            if (listeners == null) {
+                return;
+            }
+            listeners.remove(listener);
+            if (listeners.size() == 0) {
+                mEventListeners.remove(event);
+            }
+        }
     }
 
     /*
      * Battery API related methods.
      */
     public static void enableBatteryNotifications() {
         GeckoBatteryManager.enableNotifications();
     }
@@ -1741,20 +1763,21 @@ public class GeckoAppShell
                 AccessibilityManager accessibilityManager =
                     (AccessibilityManager) GeckoApp.mAppContext.getSystemService(Context.ACCESSIBILITY_SERVICE);
                 try {
                     ret.put("enabled", accessibilityManager.isEnabled());
                 } catch (Exception ex) { }
                 return ret.toString();
             }
 
-            if (mEventListeners == null)
-                return "";
+            CopyOnWriteArrayList<GeckoEventListener> listeners;
+            synchronized (mEventListeners) {
+                listeners = mEventListeners.get(type);
+            }
 
-            ArrayList<GeckoEventListener> listeners = mEventListeners.get(type);
             if (listeners == null)
                 return "";
 
             String response = null;
 
             for (GeckoEventListener listener : listeners) {
                 listener.handleMessage(type, geckoObject);
                 if (listener instanceof GeckoEventResponder) {