Merge "Fixing sync in Car(Sensor|Diagnostic)Manager" into oc-dev am: ad042e580f

am: dfb123f051

Change-Id: I596c0fc2c9a6ded065a4f6cfd4f23739e11fe210

3 files changed, 33 insertions, 24 deletions

diff --git a/car-lib/src/android/car/hardware/CarDiagnosticManager.java b/car-lib/src/android/car/hardware/CarDiagnosticManager.java
index d3ad137405..6d8b5ba58a 100644
--- a/car-lib/src/android/car/hardware/CarDiagnosticManager.java
+++ b/car-lib/src/android/car/hardware/CarDiagnosticManager.java
@@ -33,6 +33,7 @@ import com.android.car.internal.CarRatedListeners;
 import com.android.car.internal.SingleMessageHandler;
 
 import java.lang.ref.WeakReference;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
 
@@ -60,8 +61,10 @@ public final class CarDiagnosticManager implements CarManagerBase {
             MSG_DIAGNOSTIC_EVENTS) {
             @Override
             protected void handleEvent(CarDiagnosticEvent event) {
-                CarDiagnosticListeners listeners =
-                    mActiveListeners.get(event.frameType);
+                CarDiagnosticListeners listeners;
+                synchronized (mActiveListeners) {
+                    listeners = mActiveListeners.get(event.frameType);
+                }
                 if (listeners != null) {
                     listeners.onDiagnosticEvent(event);
                 }
@@ -292,7 +295,12 @@ public final class CarDiagnosticManager implements CarManagerBase {
             final CarDiagnosticEvent eventToDispatch = hasVendorExtensionPermission ?
                     event :
                     event.withVendorSensorsRemoved();
-            getListeners().forEach(new Consumer<OnDiagnosticEventListener>() {
+            List<OnDiagnosticEventListener> listeners;
+            synchronized (mActiveListeners) {
+                listeners = new ArrayList<>(getListeners());
+            }
+            listeners.forEach(new Consumer<OnDiagnosticEventListener>() {
+
                 @Override
                 public void accept(OnDiagnosticEventListener listener) {
                     listener.onDiagnosticEvent(eventToDispatch);
diff --git a/car-lib/src/android/car/hardware/CarSensorManager.java b/car-lib/src/android/car/hardware/CarSensorManager.java
index 0878e1f8d2..6e81b08da0 100644
--- a/car-lib/src/android/car/hardware/CarSensorManager.java
+++ b/car-lib/src/android/car/hardware/CarSensorManager.java
@@ -26,10 +26,11 @@ import android.car.CarManagerBase;
 import android.car.CarNotConnectedException;
 import android.content.Context;
 import android.os.Handler;
-import android.os.Handler.Callback;
 import android.os.IBinder;
 import android.os.RemoteException;
 import android.util.Log;
+import android.util.SparseArray;
+import android.util.SparseIntArray;
 
 import com.android.car.internal.CarRatedListeners;
 import com.android.car.internal.SingleMessageHandler;
@@ -37,7 +38,7 @@ import com.android.car.internal.SingleMessageHandler;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.ref.WeakReference;
-import java.util.HashMap;
+import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.function.Consumer;
@@ -192,8 +193,7 @@ public final class CarSensorManager implements CarManagerBase {
      * To keep record of locally active sensors. Key is sensor type. This is used as a basic lock
      * for all client accesses.
      */
-    private final HashMap<Integer, CarSensorListeners> mActiveSensorListeners =
-            new HashMap<Integer, CarSensorListeners>();
+    private final SparseArray<CarSensorListeners> mActiveSensorListeners = new SparseArray<>();
 
     /** Handles call back into clients. */
     private final SingleMessageHandler<CarSensorEvent> mHandlerCallback;
@@ -206,8 +206,10 @@ public final class CarSensorManager implements CarManagerBase {
                 MSG_SENSOR_EVENTS) {
             @Override
             protected void handleEvent(CarSensorEvent event) {
-                CarSensorListeners listeners =
-                    mActiveSensorListeners.get(event.sensorType);
+                CarSensorListeners listeners;
+                synchronized (mActiveSensorListeners) {
+                    listeners = mActiveSensorListeners.get(event.sensorType);
+                }
                 if (listeners != null) {
                     listeners.onSensorChanged(event);
                 }
@@ -344,10 +346,8 @@ public final class CarSensorManager implements CarManagerBase {
     public void unregisterListener(OnSensorChangedListener listener) {
         //TODO: removing listener should reset update rate, bug: 32060307
         synchronized(mActiveSensorListeners) {
-            Iterator<Integer> sensorIterator = mActiveSensorListeners.keySet().iterator();
-            while (sensorIterator.hasNext()) {
-                Integer sensor = sensorIterator.next();
-                doUnregisterListenerLocked(listener, sensor, sensorIterator);
+            for (int i = 0; i < mActiveSensorListeners.size(); i++) {
+                doUnregisterListenerLocked(listener, mActiveSensorListeners.keyAt(i));
             }
         }
     }
@@ -360,12 +360,11 @@ public final class CarSensorManager implements CarManagerBase {
      */
     public void unregisterListener(OnSensorChangedListener listener, @SensorType int sensorType) {
         synchronized(mActiveSensorListeners) {
-            doUnregisterListenerLocked(listener, sensorType, null);
+            doUnregisterListenerLocked(listener, sensorType);
         }
     }
 
-    private void doUnregisterListenerLocked(OnSensorChangedListener listener, Integer sensor,
-            Iterator<Integer> sensorIterator) {
+    private void doUnregisterListenerLocked(OnSensorChangedListener listener, Integer sensor) {
         CarSensorListeners listeners = mActiveSensorListeners.get(sensor);
         if (listeners != null) {
             boolean needsServerUpdate = false;
@@ -379,11 +378,7 @@ public final class CarSensorManager implements CarManagerBase {
                 } catch (RemoteException e) {
                     //ignore
                 }
-                if (sensorIterator == null) {
-                    mActiveSensorListeners.remove(sensor);
-                } else {
-                    sensorIterator.remove();
-                }
+                mActiveSensorListeners.remove(sensor);
             } else if (needsServerUpdate) {
                 try {
                     registerOrUpdateSensorListener(sensor, listeners.getRate());
@@ -479,7 +474,11 @@ public final class CarSensorManager implements CarManagerBase {
                 return;
             }
             mLastUpdateTime = updateTime;
-            getListeners().forEach(new Consumer<OnSensorChangedListener>() {
+            List<OnSensorChangedListener> listeners;
+            synchronized (mActiveSensorListeners) {
+                listeners = new ArrayList<>(getListeners());
+            }
+            listeners.forEach(new Consumer<OnSensorChangedListener>() {
                 @Override
                 public void accept(OnSensorChangedListener listener) {
                     listener.onSensorChanged(event);
diff --git a/car-lib/src/com/android/car/internal/CarRatedListeners.java b/car-lib/src/com/android/car/internal/CarRatedListeners.java
index 4d976dc7b2..6b26e612c1 100644
--- a/car-lib/src/com/android/car/internal/CarRatedListeners.java
+++ b/car-lib/src/com/android/car/internal/CarRatedListeners.java
@@ -16,6 +16,7 @@
 
 package com.android.car.internal;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -25,7 +26,8 @@ import java.util.Map;
  * @hide
  */
 public class CarRatedListeners<EventListenerType> {
-    private final Map<EventListenerType, Integer> mListenersToRate = new HashMap<>();
+    private final Map<EventListenerType, Integer> mListenersToRate = new HashMap<>(4);
+
     private int mUpdateRate;
 
     protected long mLastUpdateTime = -1;
@@ -83,7 +85,7 @@ public class CarRatedListeners<EventListenerType> {
         return false;
     }
 
-    public Iterable<EventListenerType> getListeners() {
+    public Collection<EventListenerType> getListeners() {
         return mListenersToRate.keySet();
     }
 }