commit a831c0b539cdc120655856074d4621e8e60a843b
author Hugo Hudson <hugohudson@google.com> Sat Aug 13 11:50:15 2011 +0100
committer Hugo Hudson <hugohudson@google.com> Sat Aug 13 11:53:57 2011 +0100
tree 04c426ef3306fb0cad868ce7054594313ba1920f
parent 8a854eb9235364ad0cad93425546be346460bb28

Use CopyOnWriteArrayList for listeners.

- We were previously using a list of Listener objects, specifically a
  LinkedList, and synchronizing on the list to maintain thread-safety.
- This cl replaces the LinkedList with a CopyOnWriteArrayList, which is
  specifically made for this kind of situation.

A bit more background:
- During the notify period, we had the classic anti-pattern of holding
  the lock whilst calling foreign methods (the listener
  onServiceCompleted methods).
- It's very evil to call foreign methods with a lock held, since this
  can easily lead to deadlock.  In this case, if any of our Listener
  implementations or any future Listener implementations yet to be
  written were to block until some other thread registers or unregisters
  a listener, we will have deadlock.
- CopyOnWriteArrayList needs no explicit synchronization - see its
  javadoc.

Change-Id: If2251e740a9f266e28956ac5acc52054debb3461

src/com/android/contacts/ContactSaveService.java

diff --git a/src/com/android/contacts/ContactSaveService.java b/src/com/android/contacts/ContactSaveService.java
index 3bb330f..78c4b18 100644
--- a/src/com/android/contacts/ContactSaveService.java
+++ b/src/com/android/contacts/ContactSaveService.java
@@ -53,8 +53,8 @@
 
 import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * A service responsible for saving changes to the content provider.
@@ -133,7 +133,8 @@
         public void onServiceCompleted(Intent callbackIntent);
     }
 
-    private static final LinkedList<Listener> sListeners = new LinkedList<Listener>();
+    private static final CopyOnWriteArrayList<Listener> sListeners =
+            new CopyOnWriteArrayList<Listener>();
 
     private Handler mMainHandler;
 
@@ -148,15 +149,11 @@
             throw new ClassCastException("Only activities can be registered to"
                     + " receive callback from " + ContactSaveService.class.getName());
         }
-        synchronized (sListeners) {
-            sListeners.addFirst(listener);
-        }
+        sListeners.add(0, listener);
     }
 
     public static void unregisterListener(Listener listener) {
-        synchronized (sListeners) {
-            sListeners.remove(listener);
-        }
+        sListeners.remove(listener);
     }
 
     @Override
@@ -975,13 +972,11 @@
         // TODO: this assumes that if there are multiple instances of the same
         // activity registered, the last one registered is the one waiting for
         // the callback. Validity of this assumption needs to be verified.
-        synchronized (sListeners) {
-            for (Listener listener : sListeners) {
-                if (callbackIntent.getComponent().equals(
-                        ((Activity) listener).getIntent().getComponent())) {
-                    listener.onServiceCompleted(callbackIntent);
-                    return;
-                }
+        for (Listener listener : sListeners) {
+            if (callbackIntent.getComponent().equals(
+                    ((Activity) listener).getIntent().getComponent())) {
+                listener.onServiceCompleted(callbackIntent);
+                return;
             }
         }
     }