commit 377c25b390164b9241502ea4eb31b552f964bce2
author Soonil Nagarkar <sooniln@google.com> Thu Mar 19 04:19:06 2020 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Thu Mar 19 04:19:06 2020 +0000
tree bd6dedf5ee0e484870e0785593b9cfa7c62a5320
parent 073f98fd1af9d54164674252b1b129c5b9ca2515
parent 8aea09fecd307d8a89ee5814501fa358655a457f

DO NOT MERGE Don't lock listener delivery am: 8aea09fecd

Change-Id: I0cc66c4170d9ef195442c9bc84380b14491e0bf4

location/java/android/location/AbstractListenerManager.java

diff --git a/location/java/android/location/AbstractListenerManager.java b/location/java/android/location/AbstractListenerManager.java
index 3dc7cfc..36b8689 100644
--- a/location/java/android/location/AbstractListenerManager.java
+++ b/location/java/android/location/AbstractListenerManager.java
@@ -85,11 +85,13 @@
         }
     }
 
-    @GuardedBy("mListeners")
-    private final ArrayMap<Object, Registration<TRequest, TListener>> mListeners =
+    private final Object mLock = new Object();
+
+    @GuardedBy("mLock")
+    private volatile ArrayMap<Object, Registration<TRequest, TListener>> mListeners =
             new ArrayMap<>();
 
-    @GuardedBy("mListeners")
+    @GuardedBy("mLock")
     @Nullable
     private TRequest mMergedRequest;
 
@@ -129,10 +131,16 @@
             throws RemoteException {
         Preconditions.checkNotNull(registration);
 
-        synchronized (mListeners) {
+        synchronized (mLock) {
             boolean initialRequest = mListeners.isEmpty();
 
-            Registration<TRequest, TListener> oldRegistration = mListeners.put(key, registration);
+            ArrayMap<Object, Registration<TRequest, TListener>> newListeners = new ArrayMap<>(
+                    mListeners.size() + 1);
+            newListeners.putAll(mListeners);
+            Registration<TRequest, TListener> oldRegistration = newListeners.put(key,
+                    registration);
+            mListeners = newListeners;
+
             if (oldRegistration != null) {
                 oldRegistration.unregister();
             }
@@ -151,8 +159,12 @@
     }
 
     public void removeListener(Object listener) throws RemoteException {
-        synchronized (mListeners) {
-            Registration<TRequest, TListener> oldRegistration = mListeners.remove(listener);
+        synchronized (mLock) {
+            ArrayMap<Object, Registration<TRequest, TListener>> newListeners = new ArrayMap<>(
+                    mListeners);
+            Registration<TRequest, TListener> oldRegistration = newListeners.remove(listener);
+            mListeners = newListeners;
+
             if (oldRegistration == null) {
                 return;
             }
@@ -190,18 +202,16 @@
     }
 
     protected void execute(Consumer<TListener> operation) {
-        synchronized (mListeners) {
-            for (Registration<TRequest, TListener> registration : mListeners.values()) {
-                registration.execute(operation);
-            }
+        for (Registration<TRequest, TListener> registration : mListeners.values()) {
+            registration.execute(operation);
         }
     }
 
-    @GuardedBy("mListeners")
+    @GuardedBy("mLock")
     @SuppressWarnings("unchecked")
     @Nullable
     private TRequest mergeRequests() {
-        Preconditions.checkState(Thread.holdsLock(mListeners));
+        Preconditions.checkState(Thread.holdsLock(mLock));
 
         if (mListeners.isEmpty()) {
             return null;