diff options
| -rw-r--r-- | service/src/com/android/car/CarPropertyService.java | 293 |
1 files changed, 172 insertions, 121 deletions
diff --git a/service/src/com/android/car/CarPropertyService.java b/service/src/com/android/car/CarPropertyService.java index e8f4aab424..90e404caa0 100644 --- a/service/src/com/android/car/CarPropertyService.java +++ b/service/src/com/android/car/CarPropertyService.java @@ -31,6 +31,7 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; import android.os.RemoteException; +import android.util.ArrayMap; import android.util.IndentingPrintWriter; import android.util.Pair; import android.util.Slog; @@ -38,16 +39,13 @@ import android.util.SparseArray; import com.android.car.hal.PropertyHalService; import com.android.internal.annotations.GuardedBy; +import com.android.server.utils.Slogf; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; /** * This class implements the binder interface for ICarProperty.aidl to make it easier to create @@ -59,12 +57,13 @@ public class CarPropertyService extends ICarProperty.Stub private static final boolean DBG = true; private static final String TAG = CarLog.tagFor(CarPropertyService.class); private final Context mContext; - private final Map<IBinder, Client> mClientMap = new ConcurrentHashMap<>(); private final PropertyHalService mHal; - private boolean mListenerIsSet = false; - private final Map<Integer, List<Client>> mPropIdClientMap = new ConcurrentHashMap<>(); private final Object mLock = new Object(); @GuardedBy("mLock") + private final Map<IBinder, Client> mClientMap = new ArrayMap<>(); + @GuardedBy("mLock") + private final SparseArray<List<Client>> mPropIdClientMap = new SparseArray<>(); + @GuardedBy("mLock") private final SparseArray<SparseArray<Client>> mSetOperationClientMap = new SparseArray<>(); private final HandlerThread mHandlerThread = CarServiceUtils.getHandlerThread(getClass().getSimpleName()); @@ -83,11 +82,16 @@ public class CarPropertyService extends ICarProperty.Stub mContext = context; } - // Helper class to keep track of listeners to this service + // Helper class to keep track of listeners to this service. private class Client implements IBinder.DeathRecipient { private final ICarPropertyEventListener mListener; private final IBinder mListenerBinder; - private final SparseArray<Float> mRateMap = new SparseArray<Float>(); // key is propId + private final Object mLock = new Object(); + // propId->rate map. + @GuardedBy("mLock") + private final SparseArray<Float> mRateMap = new SparseArray<Float>(); + @GuardedBy("mLock") + private boolean mIsDead = false; Client(ICarPropertyEventListener listener) { mListener = listener; @@ -96,56 +100,85 @@ public class CarPropertyService extends ICarProperty.Stub try { mListenerBinder.linkToDeath(this, 0); } catch (RemoteException e) { - throw new IllegalStateException("Client already dead", e); + mIsDead = true; } - mClientMap.put(mListenerBinder, this); - } - - void addProperty(int propId, float rate) { - mRateMap.put(propId, rate); } /** - * Client died. Remove the listener from HAL service and unregister if this is the last - * client. + * Returns whether this client is already dead. + * + * Caller should not assume this client is alive after getting True response because the + * binder might die after this function checks the status. Caller should only use this + * function to fail early. */ - @Override - public void binderDied() { - if (DBG) { - Slog.d(TAG, "binderDied " + mListenerBinder); + boolean isDead() { + synchronized (mLock) { + return mIsDead; } + } - for (int i = 0; i < mRateMap.size(); i++) { - int propId = mRateMap.keyAt(i); - CarPropertyService.this.unregisterListenerBinderLocked(propId, mListenerBinder); + void addProperty(int propId, float rate) { + synchronized (mLock) { + if (mIsDead) { + return; + } + mRateMap.put(propId, rate); } - this.release(); } - ICarPropertyEventListener getListener() { - return mListener; + float getRate(int propId) { + synchronized (mLock) { + // Return 0 if no key found, since that is the slowest rate. + return mRateMap.get(propId, 0.0f); + } } - IBinder getListenerBinder() { - return mListenerBinder; + int removeProperty(int propId) { + synchronized (mLock) { + mRateMap.remove(propId); + return mRateMap.size(); + } } - float getRate(int propId) { - // Return 0 if no key found, since that is the slowest rate. - return mRateMap.get(propId, (float) 0); - } + /** + * Handler to be called when client died. + * + * Remove the listener from HAL service and unregister if this is the last client. + */ + @Override + public void binderDied() { + List<Integer> propIds = new ArrayList<>(); + synchronized (mLock) { + mIsDead = true; + + if (DBG) { + Slogf.d(TAG, "binderDied %s", mListenerBinder); + } + + // Because we set mIsDead to true here, we are sure mRateMap would not have new + // elements. The propIds here is going to cover all the prop Ids that we need to + // unregister. + for (int i = 0; i < mRateMap.size(); i++) { + propIds.add(mRateMap.keyAt(i)); + } + } - void release() { - mListenerBinder.unlinkToDeath(this, 0); - mClientMap.remove(mListenerBinder); + CarPropertyService.this.unregisterListenerBinderForProps(propIds, mListenerBinder); } - void removeProperty(int propId) { - mRateMap.remove(propId); - if (mRateMap.size() == 0) { - // Last property was released, remove the client. - this.release(); + /** + * Calls onEvent function on the listener if the binder is alive. + * + * There is still chance when onEvent might fail because binderDied is not called before + * this function. + */ + void onEvent(List<CarPropertyEvent> events) throws RemoteException { + synchronized (mLock) { + if (mIsDead) { + return; + } } + mListener.onEvent(events); } } @@ -159,18 +192,15 @@ public class CarPropertyService extends ICarProperty.Stub if (DBG) { Slog.d(TAG, "cache CarPropertyConfigs " + mConfigs.size()); } + mHal.setListener(this); } @Override public void release() { - for (Client c : mClientMap.values()) { - c.release(); - } - mClientMap.clear(); - mPropIdClientMap.clear(); - mHal.setListener(null); - mListenerIsSet = false; synchronized (mLock) { + mClientMap.clear(); + mPropIdClientMap.clear(); + mHal.setListener(null); mSetOperationClientMap.clear(); } } @@ -180,15 +210,14 @@ public class CarPropertyService extends ICarProperty.Stub writer.println("*CarPropertyService*"); writer.increaseIndent(); synchronized (mLock) { - writer.println(String.format("Listener is set for PropertyHalService: %s", - mListenerIsSet)); writer.println(String.format("There are %d clients using CarPropertyService.", mClientMap.size())); writer.println("Properties registered: "); writer.increaseIndent(); - for (int propId : mPropIdClientMap.keySet()) { + for (int i = 0; i < mPropIdClientMap.size(); i++) { + int propId = mPropIdClientMap.keyAt(i); writer.println("propId: 0x" + toHexString(propId) - + " is registered by " + mPropIdClientMap.get(propId).size() + + " is registered by " + mPropIdClientMap.valueAt(i).size() + " client(s)."); } writer.decreaseIndent(); @@ -234,21 +263,22 @@ public class CarPropertyService extends ICarProperty.Stub Client client = mClientMap.get(listenerBinder); if (client == null) { client = new Client(listener); + if (client.isDead()) { + Slogf.w(TAG, "the ICarPropertyEventListener is already dead"); + return; + } + mClientMap.put(listenerBinder, client); } client.addProperty(propId, rate); // Insert the client into the propId --> clients map List<Client> clients = mPropIdClientMap.get(propId); if (clients == null) { - clients = new CopyOnWriteArrayList<Client>(); + clients = new ArrayList<Client>(); mPropIdClientMap.put(propId, clients); } if (!clients.contains(client)) { clients.add(client); } - // Set the HAL listener if necessary - if (!mListenerIsSet) { - mHal.setListener(this); - } // Set the new rate if (rate > mHal.getSampleRate(propId)) { mHal.subscribeProperty(propId, rate); @@ -262,7 +292,7 @@ public class CarPropertyService extends ICarProperty.Stub } private void getAndDispatchPropertyInitValue(CarPropertyConfig config, Client client) { - List<CarPropertyEvent> events = new LinkedList<>(); + List<CarPropertyEvent> events = new ArrayList<>(); int propId = config.getPropertyId(); if (config.isGlobalProperty()) { CarPropertyValue value = mHal.getPropertySafe(propId, 0); @@ -282,11 +312,11 @@ public class CarPropertyService extends ICarProperty.Stub } } try { - client.getListener().onEvent(events); + client.onEvent(events); } catch (RemoteException ex) { // If we cannot send a record, its likely the connection snapped. Let the binder // death handle the situation. - Slog.e(TAG, "onEvent calling failed: " + ex); + Slogf.e(TAG, "onEvent calling failed", ex); } } @@ -302,54 +332,68 @@ public class CarPropertyService extends ICarProperty.Stub } IBinder listenerBinder = listener.asBinder(); - synchronized (mLock) { - unregisterListenerBinderLocked(propId, listenerBinder); - } + unregisterListenerBinderForProps(List.of(propId), listenerBinder); } + @GuardedBy("mLock") private void unregisterListenerBinderLocked(int propId, IBinder listenerBinder) { + float updateMaxRate = 0f; Client client = mClientMap.get(listenerBinder); List<Client> propertyClients = mPropIdClientMap.get(propId); - synchronized (mLock) { - if (mConfigs.get(propId) == null) { - // Do not attempt to register an invalid propId - Slog.e(TAG, "unregisterListener: propId is not in config list:0x" + toHexString( - propId)); - return; - } + if (mConfigs.get(propId) == null) { + // Do not attempt to unregister an invalid propId + Slogf.e(TAG, "unregisterListener: propId is not in config list:0x%s", + toHexString(propId)); + return; } if ((client == null) || (propertyClients == null)) { - Slog.e(TAG, "unregisterListenerBinderLocked: Listener was not previously registered."); + Slogf.e(TAG, "unregisterListenerBinderLocked: Listener was not previously " + + "registered."); } else { if (propertyClients.remove(client)) { - client.removeProperty(propId); + int propLeft = client.removeProperty(propId); + if (propLeft == 0) { + mClientMap.remove(listenerBinder); + } clearSetOperationRecorderLocked(propId, client); + } else { - Slog.e(TAG, "unregisterListenerBinderLocked: Listener was not registered for " - + "propId=0x" + toHexString(propId)); + Slogf.e(TAG, "unregisterListenerBinderLocked: Listener was not registered for " + + "propId=0x" + toHexString(propId)); } if (propertyClients.isEmpty()) { // Last listener for this property unsubscribed. Clean up - mHal.unsubscribeProperty(propId); mPropIdClientMap.remove(propId); mSetOperationClientMap.remove(propId); - if (mPropIdClientMap.isEmpty()) { - // No more properties are subscribed. Turn off the listener. - mHal.setListener(null); - mListenerIsSet = false; - } } else { // Other listeners are still subscribed. Calculate the new rate - float maxRate = 0; - for (Client c : propertyClients) { + for (int i = 0; i < propertyClients.size(); i++) { + Client c = propertyClients.get(i); float rate = c.getRate(propId); - if (rate > maxRate) { - maxRate = rate; - } + updateMaxRate = Math.max(rate, updateMaxRate); } - // Set the new rate - mHal.subscribeProperty(propId, maxRate); + } + } + if (Float.compare(updateMaxRate, 0f) == 0) { + // Unsubscribe property if we did not find any other client register to this property + mHal.unsubscribeProperty(propId); + } else if (Float.compare(updateMaxRate, mHal.getSampleRate(propId)) != 0) { + try { + // Only reset the sample rate if needed + mHal.subscribeProperty(propId, updateMaxRate); + } catch (IllegalArgumentException e) { + Slogf.e(TAG, "failed to subscribe to propId=0x" + toHexString(propId) + + ", error: " + e); + } + } + } + + private void unregisterListenerBinderForProps(List<Integer> propIds, IBinder listenerBinder) { + synchronized (mLock) { + for (int i = 0; i < propIds.size(); i++) { + int propId = propIds.get(i); + unregisterListenerBinderLocked(propId, listenerBinder); } } } @@ -484,7 +528,11 @@ public class CarPropertyService extends ICarProperty.Stub if (client == null) { client = new Client(listener); } - updateSetOperationRecorder(propId, prop.getAreaId(), client); + if (client.isDead()) { + Slogf.w(TAG, "the ICarPropertyEventListener is already dead"); + return; + } + updateSetOperationRecorderLocked(propId, prop.getAreaId(), client); } } @@ -509,8 +557,9 @@ public class CarPropertyService extends ICarProperty.Stub ICarImpl.assertPermission(mContext, propertyWritePermission); } - // Updates recorder for set operation. - private void updateSetOperationRecorder(int propId, int areaId, Client client) { + // Updates recorder for set operation. + @GuardedBy("mLock") + private void updateSetOperationRecorderLocked(int propId, int areaId, Client client) { if (mSetOperationClientMap.get(propId) != null) { mSetOperationClientMap.get(propId).put(areaId, client); } else { @@ -544,38 +593,40 @@ public class CarPropertyService extends ICarProperty.Stub // Implement PropertyHalListener interface @Override public void onPropertyChange(List<CarPropertyEvent> events) { - Map<IBinder, Pair<ICarPropertyEventListener, List<CarPropertyEvent>>> eventsToDispatch = - new HashMap<>(); + Map<Client, List<CarPropertyEvent>> eventsToDispatch = new ArrayMap<>(); - for (CarPropertyEvent event : events) { - int propId = event.getCarPropertyValue().getPropertyId(); - List<Client> clients = mPropIdClientMap.get(propId); - if (clients == null) { - Slog.e(TAG, "onPropertyChange: no listener registered for propId=0x" - + toHexString(propId)); - continue; - } + synchronized (mLock) { + for (int i = 0; i < events.size(); i++) { + CarPropertyEvent event = events.get(i); + int propId = event.getCarPropertyValue().getPropertyId(); + List<Client> clients = mPropIdClientMap.get(propId); + if (clients == null) { + Slogf.e(TAG, "onPropertyChange: no listener registered for propId=0x%s", + toHexString(propId)); + continue; + } - for (Client c : clients) { - IBinder listenerBinder = c.getListenerBinder(); - Pair<ICarPropertyEventListener, List<CarPropertyEvent>> p = - eventsToDispatch.get(listenerBinder); - if (p == null) { - // Initialize the linked list for the listener - p = new Pair<>(c.getListener(), new LinkedList<CarPropertyEvent>()); - eventsToDispatch.put(listenerBinder, p); + for (int j = 0; j < clients.size(); j++) { + Client c = clients.get(j); + List<CarPropertyEvent> p = eventsToDispatch.get(c); + if (p == null) { + // Initialize the linked list for the listener + p = new ArrayList<CarPropertyEvent>(); + eventsToDispatch.put(c, p); + } + p.add(event); } - p.second.add(event); } - } - // Parse the dispatch list to send events - for (Pair<ICarPropertyEventListener, List<CarPropertyEvent>> p: eventsToDispatch.values()) { - try { - p.first.onEvent(p.second); - } catch (RemoteException ex) { - // If we cannot send a record, its likely the connection snapped. Let binder - // death handle the situation. - Slog.e(TAG, "onEvent calling failed: " + ex); + + // Parse the dispatch list to send events + for (Client client : eventsToDispatch.keySet()) { + try { + client.onEvent(eventsToDispatch.get(client)); + } catch (RemoteException ex) { + // If we cannot send a record, its likely the connection snapped. Let binder + // death handle the situation. + Slogf.e(TAG, "onEvent calling failed", ex); + } } } } @@ -601,13 +652,13 @@ public class CarPropertyService extends ICarProperty.Stub private void dispatchToLastClient(int property, int areaId, int errorCode, Client lastOperatedClient) { try { - List<CarPropertyEvent> eventList = new LinkedList<>(); + List<CarPropertyEvent> eventList = new ArrayList<>(); eventList.add( CarPropertyEvent.createErrorEventWithErrorCode(property, areaId, errorCode)); - lastOperatedClient.getListener().onEvent(eventList); + lastOperatedClient.onEvent(eventList); } catch (RemoteException ex) { - Slog.e(TAG, "onEvent calling failed: " + ex); + Slogf.e(TAG, "onEvent calling failed", ex); } } } |