Merge "Use callback to run binder service to avoid race"
diff --git a/services/core/java/com/android/server/ServiceWatcher.java b/services/core/java/com/android/server/ServiceWatcher.java
index 2ff036b..f20ca43 100644
--- a/services/core/java/com/android/server/ServiceWatcher.java
+++ b/services/core/java/com/android/server/ServiceWatcher.java
@@ -16,6 +16,7 @@
package com.android.server;
+import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.BroadcastReceiver;
import android.content.ComponentName;
@@ -397,9 +398,29 @@
}
}
- public @Nullable IBinder getBinder() {
+ /**
+ * The runner that runs on the binder retrieved from {@link ServiceWatcher}.
+ */
+ public interface BinderRunner {
+ /**
+ * Runs on the retrieved binder.
+ * @param binder the binder retrieved from the {@link ServiceWatcher}.
+ */
+ public void run(@NonNull IBinder binder);
+ }
+
+ /**
+ * Retrieves the binder from {@link ServiceWatcher} and runs it.
+ * @return whether a valid service exists.
+ */
+ public boolean runOnBinder(@NonNull BinderRunner runner) {
synchronized (mLock) {
- return mBoundService;
+ if (mBoundService == null) {
+ return false;
+ } else {
+ runner.run(mBoundService);
+ return true;
+ }
}
}
diff --git a/services/core/java/com/android/server/location/ActivityRecognitionProxy.java b/services/core/java/com/android/server/location/ActivityRecognitionProxy.java
index 55222dc..f82b976 100644
--- a/services/core/java/com/android/server/location/ActivityRecognitionProxy.java
+++ b/services/core/java/com/android/server/location/ActivityRecognitionProxy.java
@@ -103,51 +103,55 @@
* Helper function to bind the FusedLocationHardware to the appropriate FusedProvider instance.
*/
private void bindProvider() {
- IBinder binder = mServiceWatcher.getBinder();
- if (binder == null) {
- Log.e(TAG, "Null binder found on connection.");
- return;
- }
- String descriptor;
- try {
- descriptor = binder.getInterfaceDescriptor();
- } catch (RemoteException e) {
- Log.e(TAG, "Unable to get interface descriptor.", e);
- return;
- }
+ if (!mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ String descriptor;
+ try {
+ descriptor = binder.getInterfaceDescriptor();
+ } catch (RemoteException e) {
+ Log.e(TAG, "Unable to get interface descriptor.", e);
+ return;
+ }
- if (IActivityRecognitionHardwareWatcher.class.getCanonicalName().equals(descriptor)) {
- IActivityRecognitionHardwareWatcher watcher =
- IActivityRecognitionHardwareWatcher.Stub.asInterface(binder);
- if (watcher == null) {
- Log.e(TAG, "No watcher found on connection.");
- return;
+ if (IActivityRecognitionHardwareWatcher.class.getCanonicalName()
+ .equals(descriptor)) {
+ IActivityRecognitionHardwareWatcher watcher =
+ IActivityRecognitionHardwareWatcher.Stub.asInterface(binder);
+ if (watcher == null) {
+ Log.e(TAG, "No watcher found on connection.");
+ return;
+ }
+ if (mInstance == null) {
+ // to keep backwards compatibility do not update the watcher when there is
+ // no instance available, or it will cause an NPE
+ Log.d(TAG, "AR HW instance not available, binding will be a no-op.");
+ return;
+ }
+ try {
+ watcher.onInstanceChanged(mInstance);
+ } catch (RemoteException e) {
+ Log.e(TAG, "Error delivering hardware interface to watcher.", e);
+ }
+ } else if (IActivityRecognitionHardwareClient.class.getCanonicalName()
+ .equals(descriptor)) {
+ IActivityRecognitionHardwareClient client =
+ IActivityRecognitionHardwareClient.Stub.asInterface(binder);
+ if (client == null) {
+ Log.e(TAG, "No client found on connection.");
+ return;
+ }
+ try {
+ client.onAvailabilityChanged(mIsSupported, mInstance);
+ } catch (RemoteException e) {
+ Log.e(TAG, "Error delivering hardware interface to client.", e);
+ }
+ } else {
+ Log.e(TAG, "Invalid descriptor found on connection: " + descriptor);
+ }
}
- if (mInstance == null) {
- // to keep backwards compatibility do not update the watcher when there is no
- // instance available, or it will cause an NPE
- Log.d(TAG, "AR HW instance not available, binding will be a no-op.");
- return;
- }
- try {
- watcher.onInstanceChanged(mInstance);
- } catch (RemoteException e) {
- Log.e(TAG, "Error delivering hardware interface to watcher.", e);
- }
- } else if (IActivityRecognitionHardwareClient.class.getCanonicalName().equals(descriptor)) {
- IActivityRecognitionHardwareClient client =
- IActivityRecognitionHardwareClient.Stub.asInterface(binder);
- if (client == null) {
- Log.e(TAG, "No client found on connection.");
- return;
- }
- try {
- client.onAvailabilityChanged(mIsSupported, mInstance);
- } catch (RemoteException e) {
- Log.e(TAG, "Error delivering hardware interface to client.", e);
- }
- } else {
- Log.e(TAG, "Invalid descriptor found on connection: " + descriptor);
+ })) {
+ Log.e(TAG, "Null binder found on connection.");
}
}
}
diff --git a/services/core/java/com/android/server/location/FusedProxy.java b/services/core/java/com/android/server/location/FusedProxy.java
index f7fac77..b90f037 100644
--- a/services/core/java/com/android/server/location/FusedProxy.java
+++ b/services/core/java/com/android/server/location/FusedProxy.java
@@ -23,6 +23,7 @@
import android.hardware.location.IFusedLocationHardware;
import android.location.IFusedProvider;
import android.os.Handler;
+import android.os.IBinder;
import android.os.RemoteException;
import android.util.Log;
@@ -112,17 +113,18 @@
* @param locationHardware The FusedLocationHardware instance to use for the binding operation.
*/
private void bindProvider(IFusedLocationHardware locationHardware) {
- IFusedProvider provider = IFusedProvider.Stub.asInterface(mServiceWatcher.getBinder());
-
- if (provider == null) {
+ if (!mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ IFusedProvider provider = IFusedProvider.Stub.asInterface(binder);
+ try {
+ provider.onFusedLocationHardwareChange(locationHardware);
+ } catch (RemoteException e) {
+ Log.e(TAG, e.toString());
+ }
+ }
+ })) {
Log.e(TAG, "No instance of FusedProvider found on FusedLocationHardware connected.");
- return;
- }
-
- try {
- provider.onFusedLocationHardwareChange(locationHardware);
- } catch (RemoteException e) {
- Log.e(TAG, e.toString());
}
}
}
diff --git a/services/core/java/com/android/server/location/GeocoderProxy.java b/services/core/java/com/android/server/location/GeocoderProxy.java
index 422b94b..9ad4aa1 100644
--- a/services/core/java/com/android/server/location/GeocoderProxy.java
+++ b/services/core/java/com/android/server/location/GeocoderProxy.java
@@ -21,6 +21,7 @@
import android.location.GeocoderParams;
import android.location.IGeocodeProvider;
import android.os.Handler;
+import android.os.IBinder;
import android.os.RemoteException;
import android.util.Log;
@@ -63,42 +64,47 @@
return mServiceWatcher.start();
}
- private IGeocodeProvider getService() {
- return IGeocodeProvider.Stub.asInterface(mServiceWatcher.getBinder());
- }
-
public String getConnectedPackageName() {
return mServiceWatcher.getBestPackageName();
}
public String getFromLocation(double latitude, double longitude, int maxResults,
GeocoderParams params, List<Address> addrs) {
- IGeocodeProvider provider = getService();
- if (provider != null) {
- try {
- return provider.getFromLocation(latitude, longitude, maxResults, params, addrs);
- } catch (RemoteException e) {
- Log.w(TAG, e);
+ final String[] result = new String[] {"Service not Available"};
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
+ try {
+ result[0] = provider.getFromLocation(
+ latitude, longitude, maxResults, params, addrs);
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ }
}
- }
- return "Service not Available";
+ });
+ return result[0];
}
public String getFromLocationName(String locationName,
double lowerLeftLatitude, double lowerLeftLongitude,
double upperRightLatitude, double upperRightLongitude, int maxResults,
GeocoderParams params, List<Address> addrs) {
- IGeocodeProvider provider = getService();
- if (provider != null) {
- try {
- return provider.getFromLocationName(locationName, lowerLeftLatitude,
- lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
- maxResults, params, addrs);
- } catch (RemoteException e) {
- Log.w(TAG, e);
+ final String[] result = new String[] {"Service not Available"};
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
+ try {
+ result[0] = provider.getFromLocationName(locationName, lowerLeftLatitude,
+ lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
+ maxResults, params, addrs);
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ }
}
- }
- return "Service not Available";
+ });
+ return result[0];
}
}
diff --git a/services/core/java/com/android/server/location/GeofenceProxy.java b/services/core/java/com/android/server/location/GeofenceProxy.java
index b3a0010..eb47b2f 100644
--- a/services/core/java/com/android/server/location/GeofenceProxy.java
+++ b/services/core/java/com/android/server/location/GeofenceProxy.java
@@ -116,15 +116,17 @@
};
private void setGeofenceHardwareInProviderLocked() {
- try {
- IGeofenceProvider provider = IGeofenceProvider.Stub.asInterface(
- mServiceWatcher.getBinder());
- if (provider != null) {
- provider.setGeofenceHardware(mGeofenceHardware);
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ final IGeofenceProvider provider = IGeofenceProvider.Stub.asInterface(binder);
+ try {
+ provider.setGeofenceHardware(mGeofenceHardware);
+ } catch (RemoteException e) {
+ Log.e(TAG, "Remote Exception: setGeofenceHardwareInProviderLocked: " + e);
+ }
}
- } catch (RemoteException e) {
- Log.e(TAG, "Remote Exception: setGeofenceHardwareInProviderLocked: " + e);
- }
+ });
}
private void setGpsGeofenceLocked() {
diff --git a/services/core/java/com/android/server/location/LocationProviderProxy.java b/services/core/java/com/android/server/location/LocationProviderProxy.java
index b44087c..16eae62 100644
--- a/services/core/java/com/android/server/location/LocationProviderProxy.java
+++ b/services/core/java/com/android/server/location/LocationProviderProxy.java
@@ -24,6 +24,7 @@
import android.location.LocationProvider;
import android.os.Bundle;
import android.os.Handler;
+import android.os.IBinder;
import android.os.RemoteException;
import android.os.WorkSource;
import android.util.Log;
@@ -82,10 +83,6 @@
return mServiceWatcher.start();
}
- private ILocationProvider getService() {
- return ILocationProvider.Stub.asInterface(mServiceWatcher.getBinder());
- }
-
public String getConnectedPackageName() {
return mServiceWatcher.getBestPackageName();
}
@@ -101,43 +98,46 @@
if (D) Log.d(TAG, "applying state to connected service");
boolean enabled;
- ProviderProperties properties = null;
+ final ProviderProperties[] properties = new ProviderProperties[1];
ProviderRequest request;
WorkSource source;
- ILocationProvider service;
synchronized (mLock) {
enabled = mEnabled;
request = mRequest;
source = mWorksource;
- service = getService();
}
- if (service == null) return;
- try {
- // load properties from provider
- properties = service.getProperties();
- if (properties == null) {
- Log.e(TAG, mServiceWatcher.getBestPackageName() +
- " has invalid locatino provider properties");
- }
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ // load properties from provider
+ properties[0] = service.getProperties();
+ if (properties[0] == null) {
+ Log.e(TAG, mServiceWatcher.getBestPackageName() +
+ " has invalid location provider properties");
+ }
- // apply current state to new service
- if (enabled) {
- service.enable();
- if (request != null) {
- service.setRequest(request, source);
+ // apply current state to new service
+ if (enabled) {
+ service.enable();
+ if (request != null) {
+ service.setRequest(request, source);
+ }
+ }
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
}
}
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
+ });
synchronized (mLock) {
- mProperties = properties;
+ mProperties = properties[0];
}
}
};
@@ -159,17 +159,20 @@
synchronized (mLock) {
mEnabled = true;
}
- ILocationProvider service = getService();
- if (service == null) return;
-
- try {
- service.enable();
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ service.enable();
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
+ }
+ }
+ });
}
@Override
@@ -177,17 +180,20 @@
synchronized (mLock) {
mEnabled = false;
}
- ILocationProvider service = getService();
- if (service == null) return;
-
- try {
- service.disable();
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ service.disable();
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
+ }
+ }
+ });
}
@Override
@@ -203,17 +209,20 @@
mRequest = request;
mWorksource = source;
}
- ILocationProvider service = getService();
- if (service == null) return;
-
- try {
- service.setRequest(request, source);
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ service.setRequest(request, source);
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
+ }
+ }
+ });
}
@Override
@@ -223,66 +232,78 @@
pw.append(" pkg=").append(mServiceWatcher.getBestPackageName());
pw.append(" version=").append("" + mServiceWatcher.getBestVersion());
pw.append('\n');
-
- ILocationProvider service = getService();
- if (service == null) {
+ if (!mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ TransferPipe.dumpAsync(service.asBinder(), fd, args);
+ } catch (IOException | RemoteException e) {
+ pw.println("Failed to dump location provider: " + e);
+ }
+ }
+ })) {
pw.println("service down (null)");
- return;
- }
- pw.flush();
-
- try {
- TransferPipe.dumpAsync(service.asBinder(), fd, args);
- } catch (IOException | RemoteException e) {
- pw.println("Failed to dump location provider: " + e);
}
}
@Override
public int getStatus(Bundle extras) {
- ILocationProvider service = getService();
- if (service == null) return LocationProvider.TEMPORARILY_UNAVAILABLE;
-
- try {
- return service.getStatus(extras);
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
- return LocationProvider.TEMPORARILY_UNAVAILABLE;
+ final int[] result = new int[] {LocationProvider.TEMPORARILY_UNAVAILABLE};
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ result[0] = service.getStatus(extras);
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
+ }
+ }
+ });
+ return result[0];
}
@Override
public long getStatusUpdateTime() {
- ILocationProvider service = getService();
- if (service == null) return 0;
-
- try {
- return service.getStatusUpdateTime();
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
- return 0;
+ final long[] result = new long[] {0L};
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ result[0] = service.getStatusUpdateTime();
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
+ }
+ }
+ });
+ return result[0];
}
@Override
public boolean sendExtraCommand(String command, Bundle extras) {
- ILocationProvider service = getService();
- if (service == null) return false;
-
- try {
- return service.sendExtraCommand(command, extras);
- } catch (RemoteException e) {
- Log.w(TAG, e);
- } catch (Exception e) {
- // never let remote service crash system server
- Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
- }
- return false;
+ final boolean[] result = new boolean[] {false};
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) {
+ ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
+ try {
+ result[0] = service.sendExtraCommand(command, extras);
+ } catch (RemoteException e) {
+ Log.w(TAG, e);
+ } catch (Exception e) {
+ // never let remote service crash system server
+ Log.e(TAG, "Exception from " + mServiceWatcher.getBestPackageName(), e);
+ }
+ }
+ });
+ return result[0];
}
}