GnssMeasurementsListener - Fix indexing, add & remove

Safer indexing open requests by IBinder instead of IInterface
Covers GnssNavigationMessage as well.
Also dumping currently open GnssMeasurement & NavMsg requests

Bug: 71917458
Test: Able to repro issue consistently before, fixed after
 logs look as expected as well, on Pixel 2

Change-Id: I2e4722a23355295a5f597541f31a3b77534cadd7

diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java
index ea748db..6c63f43 100644 (file)
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -232,10 +232,9 @@ public class LocationManagerService extends ILocationManager.Stub {
 
     private final ArraySet<String> mBackgroundThrottlePackageWhitelist = new ArraySet<>();
 
-    private final ArrayMap<IGnssMeasurementsListener, Identity> mGnssMeasurementsListeners =
-            new ArrayMap<>();
+    private final ArrayMap<IBinder, Identity> mGnssMeasurementsListeners = new ArrayMap<>();
 
-    private final ArrayMap<IGnssNavigationMessageListener, Identity>
+    private final ArrayMap<IBinder, Identity>
             mGnssNavigationMessageListeners = new ArrayMap<>();
 
     // current active user on the device - other users are denied location data
@@ -438,23 +437,23 @@ public class LocationManagerService extends ILocationManager.Stub {
                 applyRequirementsLocked(provider);
             }
 
-            for (Entry<IGnssMeasurementsListener, Identity> entry
-                    : mGnssMeasurementsListeners.entrySet()) {
+            for (Entry<IBinder, Identity> entry : mGnssMeasurementsListeners.entrySet()) {
                 if (entry.getValue().mUid == uid) {
                     if (D) {
                         Log.d(TAG, "gnss measurements listener from uid " + uid
                                 + " is now " + (foreground ? "foreground" : "background)"));
                     }
                     if (foreground || isThrottlingExemptLocked(entry.getValue())) {
-                        mGnssMeasurementsProvider.addListener(entry.getKey());
+                        mGnssMeasurementsProvider.addListener(
+                                IGnssMeasurementsListener.Stub.asInterface(entry.getKey()));
                     } else {
-                        mGnssMeasurementsProvider.removeListener(entry.getKey());
+                        mGnssMeasurementsProvider.removeListener(
+                                IGnssMeasurementsListener.Stub.asInterface(entry.getKey()));
                     }
                 }
             }
 
-            for (Entry<IGnssNavigationMessageListener, Identity> entry
-                    : mGnssNavigationMessageListeners.entrySet()) {
+            for (Entry<IBinder, Identity> entry : mGnssNavigationMessageListeners.entrySet()) {
                 if (entry.getValue().mUid == uid) {
                     if (D) {
                         Log.d(TAG, "gnss navigation message listener from uid "
@@ -462,9 +461,11 @@ public class LocationManagerService extends ILocationManager.Stub {
                                 + (foreground ? "foreground" : "background)"));
                     }
                     if (foreground || isThrottlingExemptLocked(entry.getValue())) {
-                        mGnssNavigationMessageProvider.addListener(entry.getKey());
+                        mGnssNavigationMessageProvider.addListener(
+                                IGnssNavigationMessageListener.Stub.asInterface(entry.getKey()));
                     } else {
-                        mGnssNavigationMessageProvider.removeListener(entry.getKey());
+                        mGnssNavigationMessageProvider.removeListener(
+                                IGnssNavigationMessageListener.Stub.asInterface(entry.getKey()));
                     }
                 }
             }
@@ -2401,7 +2402,7 @@ public class LocationManagerService extends ILocationManager.Stub {
         synchronized (mLock) {
             Identity callerIdentity
                     = new Identity(Binder.getCallingUid(), Binder.getCallingPid(), packageName);
-            mGnssMeasurementsListeners.put(listener, callerIdentity);
+            mGnssMeasurementsListeners.put(listener.asBinder(), callerIdentity);
             long identity = Binder.clearCallingIdentity();
             try {
                 if (isThrottlingExemptLocked(callerIdentity)
@@ -2421,7 +2422,7 @@ public class LocationManagerService extends ILocationManager.Stub {
     public void removeGnssMeasurementsListener(IGnssMeasurementsListener listener) {
         if (mGnssMeasurementsProvider != null) {
             synchronized (mLock) {
-                mGnssMeasurementsListeners.remove(listener);
+                mGnssMeasurementsListeners.remove(listener.asBinder());
                 mGnssMeasurementsProvider.removeListener(listener);
             }
         }
@@ -2438,7 +2439,7 @@ public class LocationManagerService extends ILocationManager.Stub {
         synchronized (mLock) {
             Identity callerIdentity
                     = new Identity(Binder.getCallingUid(), Binder.getCallingPid(), packageName);
-            mGnssNavigationMessageListeners.put(listener, callerIdentity);
+            mGnssNavigationMessageListeners.put(listener.asBinder(), callerIdentity);
             long identity = Binder.clearCallingIdentity();
             try {
                 if (isThrottlingExemptLocked(callerIdentity)
@@ -2458,7 +2459,7 @@ public class LocationManagerService extends ILocationManager.Stub {
     public void removeGnssNavigationMessageListener(IGnssNavigationMessageListener listener) {
         if (mGnssNavigationMessageProvider != null) {
             synchronized (mLock) {
-                mGnssNavigationMessageListeners.remove(listener);
+                mGnssNavigationMessageListeners.remove(listener.asBinder());
                 mGnssNavigationMessageProvider.removeListener(listener);
             }
         }
@@ -3180,6 +3181,16 @@ public class LocationManagerService extends ILocationManager.Stub {
                     pw.println("      " + record);
                 }
             }
+            pw.println("  Active GnssMeasurement Listeners:");
+            for (Identity identity : mGnssMeasurementsListeners.values()) {
+                pw.println("    " + identity.mPid + " " + identity.mUid + " "
+                        + identity.mPackageName + ": " + isThrottlingExemptLocked(identity));
+            }
+            pw.println("  Active GnssNavigationMessage Listeners:");
+            for (Identity identity : mGnssNavigationMessageListeners.values()) {
+                pw.println("    " + identity.mPid + " " + identity.mUid + " "
+                        + identity.mPackageName + ": " + isThrottlingExemptLocked(identity));
+            }
             pw.println("  Overlay Provider Packages:");
             for (LocationProviderInterface provider : mProviders) {
                 if (provider instanceof LocationProviderProxy) {

diff --git a/services/core/java/com/android/server/location/GnssLocationProvider.java b/services/core/java/com/android/server/location/GnssLocationProvider.java
index e158819..6dc5403 100644 (file)
--- a/services/core/java/com/android/server/location/GnssLocationProvider.java
+++ b/services/core/java/com/android/server/location/GnssLocationProvider.java
@@ -2628,6 +2628,10 @@ public class GnssLocationProvider implements LocationProviderInterface {
         s.append("  mStarted=").append(mStarted).append('\n');
         s.append("  mFixInterval=").append(mFixInterval).append('\n');
         s.append("  mLowPowerMode=").append(mLowPowerMode).append('\n');
+        s.append("  mGnssMeasurementsProvider.isRegistered()=")
+                .append(mGnssMeasurementsProvider.isRegistered()).append('\n');
+        s.append("  mGnssNavigationMessageProvider.isRegistered()=")
+                .append(mGnssNavigationMessageProvider.isRegistered()).append('\n');
         s.append("  mDisableGps (battery saver mode)=").append(mDisableGps).append('\n');
         s.append("  mEngineCapabilities=0x").append(Integer.toHexString(mEngineCapabilities));
         s.append(" ( ");

diff --git a/services/core/java/com/android/server/location/RemoteListenerHelper.java b/services/core/java/com/android/server/location/RemoteListenerHelper.java
index 58a9516..fcdb9d1 100644 (file)
--- a/services/core/java/com/android/server/location/RemoteListenerHelper.java
+++ b/services/core/java/com/android/server/location/RemoteListenerHelper.java
@@ -46,7 +46,8 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
 
     private final Map<IBinder, LinkedListener> mListenerMap = new HashMap<>();
 
-    private boolean mIsRegistered;  // must access only on handler thread
+    private volatile boolean mIsRegistered;  // must access only on handler thread, or read-only
+
     private boolean mHasIsSupported;
     private boolean mIsSupported;
 
@@ -58,6 +59,11 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
         mTag = name;
     }
 
+    // read-only access for a dump() thread assured via volatile
+    public boolean isRegistered() {
+        return mIsRegistered;
+    }
+
     public boolean addListener(@NonNull TListener listener) {
         Preconditions.checkNotNull(listener, "Attempted to register a 'null' listener.");
         IBinder binder = listener.asBinder();