Merge "Fix RemoteListenerHelper vs. HAL deadlock" into oc-dr1-dev

services/core/java/com/android/server/LocationManagerService.java

@@ -243,8 +243,9 @@ public class LocationManagerService extends ILocationManager.Stub {
 
     private GnssLocationProvider.GnssSystemInfoProvider mGnssSystemInfoProvider;
 
-    private GnssLocationProvider.GnssBatchingProvider mGnssBatchingProvider;
     private GnssLocationProvider.GnssMetricsProvider mGnssMetricsProvider;
+
+    private GnssLocationProvider.GnssBatchingProvider mGnssBatchingProvider;
     private IBatchedLocationCallback mGnssBatchingCallback;
     private LinkedCallback mGnssBatchingDeathCallback;
     private boolean mGnssBatchingInProgress = false;

services/core/java/com/android/server/location/GnssLocationProvider.java

@@ -1754,20 +1754,32 @@ public class GnssLocationProvider implements LocationProviderInterface {
     }
 
     /**
-     * called from native code - Gps measurements callback
+     * called from native code - GNSS measurements callback
      */
     private void reportMeasurementData(GnssMeasurementsEvent event) {
         if (!mItarSpeedLimitExceeded) {
-            mGnssMeasurementsProvider.onMeasurementsAvailable(event);
+            // send to handler to allow native to return quickly
+            mHandler.post(new Runnable() {
+                @Override
+                public void run() {
+                    mGnssMeasurementsProvider.onMeasurementsAvailable(event);
+                }
+            });
         }
     }
 
     /**
-     * called from native code - GPS navigation message callback
+     * called from native code - GNSS navigation message callback
      */
     private void reportNavigationMessage(GnssNavigationMessage event) {
         if (!mItarSpeedLimitExceeded) {
-            mGnssNavigationMessageProvider.onNavigationMessageAvailable(event);
+            // send to handler to allow native to return quickly
+            mHandler.post(new Runnable() {
+                @Override
+                public void run() {
+                    mGnssNavigationMessageProvider.onNavigationMessageAvailable(event);
+                }
+            });
         }
     }
 

services/core/java/com/android/server/location/GnssMeasurementsProvider.java

@@ -54,9 +54,8 @@ public abstract class GnssMeasurementsProvider
     }
 
     public void onGpsEnabledChanged() {
-        if (tryUpdateRegistrationWithService()) {
-            updateResult();
-        }
+        tryUpdateRegistrationWithService();
+        updateResult();
     }
 
     @Override

services/core/java/com/android/server/location/GnssNavigationMessageProvider.java

@@ -55,9 +55,8 @@ public abstract class GnssNavigationMessageProvider
     }
 
     public void onGpsEnabledChanged() {
-        if (tryUpdateRegistrationWithService()) {
-            updateResult();
-        }
+        tryUpdateRegistrationWithService();
+        updateResult();
     }
 
     @Override

services/core/java/com/android/server/location/RemoteListenerHelper.java

@@ -25,6 +25,7 @@ import android.os.IInterface;
 import android.os.RemoteException;
 import android.util.Log;
 
+import java.lang.Runnable;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -45,7 +46,7 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
 
     private final Map<IBinder, LinkedListener> mListenerMap = new HashMap<>();
 
-    private boolean mIsRegistered;
+    private boolean mIsRegistered;  // must access only on handler thread
     private boolean mHasIsSupported;
     private boolean mIsSupported;
 
@@ -83,12 +84,12 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
             } else if (mHasIsSupported && !mIsSupported) {
                 result = RESULT_NOT_SUPPORTED;
             } else if (!isGpsEnabled()) {
-                result = RESULT_GPS_LOCATION_DISABLED;
-            } else if (!tryRegister()) {
                 // only attempt to register if GPS is enabled, otherwise we will register once GPS
                 // becomes available
-                result = RESULT_INTERNAL_ERROR;
+                result = RESULT_GPS_LOCATION_DISABLED;
             } else if (mHasIsSupported && mIsSupported) {
+                tryRegister();
+                // initially presume success, possible internal error could follow asynchornously
                 result = RESULT_SUCCESS;
             } else {
                 // at this point if the supported flag is not set, the notification will be sent
@@ -117,8 +118,8 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
 
     protected abstract boolean isAvailableInPlatform();
     protected abstract boolean isGpsEnabled();
-    protected abstract boolean registerWithService();
-    protected abstract void unregisterFromService();
+    protected abstract boolean registerWithService(); // must access only on handler thread
+    protected abstract void unregisterFromService(); // must access only on handler thread
     protected abstract ListenerOperation<TListener> getHandlerOperation(int result);
 
     protected interface ListenerOperation<TListener extends IInterface> {
@@ -138,22 +139,16 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
         }
     }
 
-    protected boolean tryUpdateRegistrationWithService() {
+    protected void tryUpdateRegistrationWithService() {
         synchronized (mListenerMap) {
             if (!isGpsEnabled()) {
                 tryUnregister();
-                return true;
+                return;
             }
             if (mListenerMap.isEmpty()) {
-                return true;
+                return;
             }
-            if (tryRegister()) {
-                // registration was successful, there is no need to update the state
-                return true;
-            }
-            ListenerOperation<TListener> operation = getHandlerOperation(RESULT_INTERNAL_ERROR);
-            foreachUnsafe(operation);
-            return false;
+            tryRegister();
         }
     }
 
@@ -180,19 +175,40 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
         }
     }
 
-    private boolean tryRegister() {
-        if (!mIsRegistered) {
-            mIsRegistered = registerWithService();
-        }
-        return mIsRegistered;
+    private void tryRegister() {
+        mHandler.post(new Runnable() {
+            @Override
+            public void run() {
+                if (!mIsRegistered) {
+                    mIsRegistered = registerWithService();
+                }
+                if (!mIsRegistered) {
+                    // post back a failure
+                    mHandler.post(new Runnable() {
+                        @Override
+                        public void run() {
+                            synchronized (mListenerMap) {
+                                ListenerOperation<TListener> operation = getHandlerOperation(RESULT_INTERNAL_ERROR);
+                                foreachUnsafe(operation);
+                            }
+                        }
+                    });
+                }
+            }
+        });
     }
 
     private void tryUnregister() {
-        if (!mIsRegistered) {
-            return;
-        }
-        unregisterFromService();
-        mIsRegistered = false;
+        mHandler.post(new Runnable() {
+            @Override
+            public void run() {
+                if (!mIsRegistered) {
+                    return;
+                }
+                unregisterFromService();
+                mIsRegistered = false;
+            }
+        });
     }
 
     private int calculateCurrentResultUnsafe() {