Make calls for geocoder one way
All calls out of system server must be oneway to prevent the system from
being blocked by external code. Geocode has been the very last interface
not to conform to this - this is now fixed.
Bug: 146677166
Test: atest GeocoderTest
Change-Id: Ib04d36a9b759f1737ebc2806e49f0bf1e53cea7c
diff --git a/location/java/android/location/Geocoder.java b/location/java/android/location/Geocoder.java
index ac7eb8b..704cdbf 100644
--- a/location/java/android/location/Geocoder.java
+++ b/location/java/android/location/Geocoder.java
@@ -17,16 +17,16 @@
package android.location;
import android.content.Context;
-import android.location.Address;
-import android.os.RemoteException;
import android.os.IBinder;
+import android.os.RemoteException;
import android.os.ServiceManager;
-import android.util.Log;
import java.io.IOException;
-import java.util.Locale;
-import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
+import java.util.Locale;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
/**
* A class for handling geocoding and reverse geocoding. Geocoding is
@@ -45,10 +45,11 @@
* exists.
*/
public final class Geocoder {
- private static final String TAG = "Geocoder";
- private GeocoderParams mParams;
- private ILocationManager mService;
+ private static final long TIMEOUT_MS = 60000;
+
+ private final GeocoderParams mParams;
+ private final ILocationManager mService;
/**
* Returns true if the Geocoder methods getFromLocation and
@@ -62,8 +63,7 @@
try {
return lm.geocoderIsPresent();
} catch (RemoteException e) {
- Log.e(TAG, "isPresent: got RemoteException", e);
- return false;
+ throw e.rethrowFromSystemServer();
}
}
@@ -129,17 +129,11 @@
throw new IllegalArgumentException("longitude == " + longitude);
}
try {
- List<Address> results = new ArrayList<Address>();
- String ex = mService.getFromLocation(latitude, longitude, maxResults,
- mParams, results);
- if (ex != null) {
- throw new IOException(ex);
- } else {
- return results;
- }
+ GeocodeListener listener = new GeocodeListener();
+ mService.getFromLocation(latitude, longitude, maxResults, mParams, listener);
+ return listener.getResults();
} catch (RemoteException e) {
- Log.e(TAG, "getFromLocation: got RemoteException", e);
- return null;
+ throw e.rethrowFromSystemServer();
}
}
@@ -170,18 +164,13 @@
if (locationName == null) {
throw new IllegalArgumentException("locationName == null");
}
+
try {
- List<Address> results = new ArrayList<Address>();
- String ex = mService.getFromLocationName(locationName,
- 0, 0, 0, 0, maxResults, mParams, results);
- if (ex != null) {
- throw new IOException(ex);
- } else {
- return results;
- }
+ GeocodeListener listener = new GeocodeListener();
+ mService.getFromLocationName(locationName, 0, 0, 0, 0, maxResults, mParams, listener);
+ return listener.getResults();
} catch (RemoteException e) {
- Log.e(TAG, "getFromLocationName: got RemoteException", e);
- return null;
+ throw e.rethrowFromSystemServer();
}
}
@@ -242,19 +231,46 @@
throw new IllegalArgumentException("upperRightLongitude == "
+ upperRightLongitude);
}
+
try {
- ArrayList<Address> result = new ArrayList<Address>();
- String ex = mService.getFromLocationName(locationName,
- lowerLeftLatitude, lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
- maxResults, mParams, result);
- if (ex != null) {
- throw new IOException(ex);
- } else {
- return result;
- }
+ GeocodeListener listener = new GeocodeListener();
+ mService.getFromLocationName(locationName, lowerLeftLatitude, lowerLeftLongitude,
+ upperRightLatitude, upperRightLongitude, maxResults, mParams, listener);
+ return listener.getResults();
} catch (RemoteException e) {
- Log.e(TAG, "getFromLocationName: got RemoteException", e);
- return null;
+ throw e.rethrowFromSystemServer();
+ }
+ }
+
+ private static class GeocodeListener extends IGeocodeListener.Stub {
+ private final CountDownLatch mLatch = new CountDownLatch(1);
+
+ private String mError = null;
+ private List<Address> mResults = Collections.emptyList();
+
+ GeocodeListener() {}
+
+ @Override
+ public void onResults(String error, List<Address> results) {
+ mError = error;
+ mResults = results;
+ mLatch.countDown();
+ }
+
+ public List<Address> getResults() throws IOException {
+ try {
+ if (!mLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
+ mError = "Service not Available";
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+
+ if (mError != null) {
+ throw new IOException(mError);
+ } else {
+ return mResults;
+ }
}
}
}
diff --git a/location/java/android/location/IGeocodeListener.aidl b/location/java/android/location/IGeocodeListener.aidl
new file mode 100644
index 0000000..8e10411
--- /dev/null
+++ b/location/java/android/location/IGeocodeListener.aidl
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.location;
+
+import android.location.Address;
+
+/**
+ * An interface for returning geocode results.
+ *
+ * {@hide}
+ */
+interface IGeocodeListener {
+
+ oneway void onResults(String error, in List<Address> results);
+}
diff --git a/location/java/android/location/IGeocodeProvider.aidl b/location/java/android/location/IGeocodeProvider.aidl
index 7eaf7b8..e661ca6 100644
--- a/location/java/android/location/IGeocodeProvider.aidl
+++ b/location/java/android/location/IGeocodeProvider.aidl
@@ -17,6 +17,7 @@
package android.location;
import android.location.Address;
+import android.location.IGeocodeListener;
import android.location.GeocoderParams;
/**
@@ -26,13 +27,7 @@
*/
interface IGeocodeProvider {
- @UnsupportedAppUsage
- String getFromLocation(double latitude, double longitude, int maxResults,
- in GeocoderParams params, out List<Address> addrs);
-
- @UnsupportedAppUsage
- String getFromLocationName(String locationName,
- double lowerLeftLatitude, double lowerLeftLongitude,
- double upperRightLatitude, double upperRightLongitude, int maxResults,
- in GeocoderParams params, out List<Address> addrs);
+ oneway void getFromLocation(double latitude, double longitude, int maxResults, in GeocoderParams params, in IGeocodeListener listener);
+ oneway void getFromLocationName(String locationName, double lowerLeftLatitude, double lowerLeftLongitude, double upperRightLatitude,
+ double upperRightLongitude, int maxResults, in GeocoderParams params, in IGeocodeListener listener);
}
diff --git a/location/java/android/location/ILocationManager.aidl b/location/java/android/location/ILocationManager.aidl
index 6be770e..b7346308 100644
--- a/location/java/android/location/ILocationManager.aidl
+++ b/location/java/android/location/ILocationManager.aidl
@@ -24,6 +24,7 @@
import android.location.GnssMeasurementCorrections;
import android.location.GnssRequest;
import android.location.IBatchedLocationCallback;
+import android.location.IGeocodeListener;
import android.location.IGnssAntennaInfoListener;
import android.location.IGnssMeasurementsListener;
import android.location.IGnssStatusListener;
@@ -58,12 +59,12 @@
void removeGeofence(in Geofence fence, in PendingIntent intent, String packageName);
boolean geocoderIsPresent();
- String getFromLocation(double latitude, double longitude, int maxResults,
- in GeocoderParams params, out List<Address> addrs);
- String getFromLocationName(String locationName,
+ void getFromLocation(double latitude, double longitude, int maxResults,
+ in GeocoderParams params, in IGeocodeListener listener);
+ void getFromLocationName(String locationName,
double lowerLeftLatitude, double lowerLeftLongitude,
double upperRightLatitude, double upperRightLongitude, int maxResults,
- in GeocoderParams params, out List<Address> addrs);
+ in GeocoderParams params, in IGeocodeListener listener);
long getGnssCapabilities();
int getGnssYearOfHardware();
diff --git a/location/lib/java/com/android/location/provider/GeocodeProvider.java b/location/lib/java/com/android/location/provider/GeocodeProvider.java
index f7f3d82..05d7935 100644
--- a/location/lib/java/com/android/location/provider/GeocodeProvider.java
+++ b/location/lib/java/com/android/location/provider/GeocodeProvider.java
@@ -16,12 +16,14 @@
package com.android.location.provider;
-import android.os.IBinder;
-
import android.location.Address;
import android.location.GeocoderParams;
+import android.location.IGeocodeListener;
import android.location.IGeocodeProvider;
+import android.os.IBinder;
+import android.os.RemoteException;
+import java.util.ArrayList;
import java.util.List;
/**
@@ -38,19 +40,32 @@
public abstract class GeocodeProvider {
private IGeocodeProvider.Stub mProvider = new IGeocodeProvider.Stub() {
- public String getFromLocation(double latitude, double longitude, int maxResults,
- GeocoderParams params, List<Address> addrs) {
- return GeocodeProvider.this.onGetFromLocation(latitude, longitude, maxResults,
- params, addrs);
+ @Override
+ public void getFromLocation(double latitude, double longitude, int maxResults,
+ GeocoderParams params, IGeocodeListener listener) {
+ List<Address> results = new ArrayList<>();
+ String error = onGetFromLocation(latitude, longitude, maxResults, params, results);
+ try {
+ listener.onResults(error, results);
+ } catch (RemoteException e) {
+ // ignore
+ }
}
- public String getFromLocationName(String locationName,
+ @Override
+ public void getFromLocationName(String locationName,
double lowerLeftLatitude, double lowerLeftLongitude,
double upperRightLatitude, double upperRightLongitude, int maxResults,
- GeocoderParams params, List<Address> addrs) {
- return GeocodeProvider.this.onGetFromLocationName(locationName, lowerLeftLatitude,
+ GeocoderParams params, IGeocodeListener listener) {
+ List<Address> results = new ArrayList<>();
+ String error = onGetFromLocationName(locationName, lowerLeftLatitude,
lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
- maxResults, params, addrs);
+ maxResults, params, results);
+ try {
+ listener.onResults(error, results);
+ } catch (RemoteException e) {
+ // ignore
+ }
}
};
diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java
index 55ccc8a..2c4309f 100644
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -41,7 +41,6 @@
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
-import android.location.Address;
import android.location.Criteria;
import android.location.GeocoderParams;
import android.location.Geofence;
@@ -49,6 +48,7 @@
import android.location.GnssMeasurementCorrections;
import android.location.GnssRequest;
import android.location.IBatchedLocationCallback;
+import android.location.IGeocodeListener;
import android.location.IGnssAntennaInfoListener;
import android.location.IGnssMeasurementsListener;
import android.location.IGnssNavigationMessageListener;
@@ -2503,27 +2503,37 @@
}
@Override
- public String getFromLocation(double latitude, double longitude, int maxResults,
- GeocoderParams params, List<Address> addrs) {
+ public void getFromLocation(double latitude, double longitude, int maxResults,
+ GeocoderParams params, IGeocodeListener listener) {
if (mGeocodeProvider != null) {
- return mGeocodeProvider.getFromLocation(latitude, longitude, maxResults,
- params, addrs);
+ mGeocodeProvider.getFromLocation(latitude, longitude, maxResults,
+ params, listener);
+ } else {
+ try {
+ listener.onResults(null, Collections.emptyList());
+ } catch (RemoteException e) {
+ // ignore
+ }
}
- return null;
}
@Override
- public String getFromLocationName(String locationName,
+ public void getFromLocationName(String locationName,
double lowerLeftLatitude, double lowerLeftLongitude,
double upperRightLatitude, double upperRightLongitude, int maxResults,
- GeocoderParams params, List<Address> addrs) {
+ GeocoderParams params, IGeocodeListener listener) {
if (mGeocodeProvider != null) {
- return mGeocodeProvider.getFromLocationName(locationName, lowerLeftLatitude,
+ mGeocodeProvider.getFromLocationName(locationName, lowerLeftLatitude,
lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
- maxResults, params, addrs);
+ maxResults, params, listener);
+ } else {
+ try {
+ listener.onResults(null, Collections.emptyList());
+ } catch (RemoteException e) {
+ // ignore
+ }
}
- return null;
}
// Mock Providers
diff --git a/services/core/java/com/android/server/ServiceWatcher.java b/services/core/java/com/android/server/ServiceWatcher.java
index b43ae36..953fb8c 100644
--- a/services/core/java/com/android/server/ServiceWatcher.java
+++ b/services/core/java/com/android/server/ServiceWatcher.java
@@ -53,11 +53,6 @@
import java.io.PrintWriter;
import java.util.List;
import java.util.Objects;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.FutureTask;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
/**
* Maintains a binding to the best service that matches the given intent information. Bind and
@@ -71,12 +66,12 @@
private static final String EXTRA_SERVICE_VERSION = "serviceVersion";
private static final String EXTRA_SERVICE_IS_MULTIUSER = "serviceIsMultiuser";
- private static final long BLOCKING_BINDER_TIMEOUT_MS = 30 * 1000;
-
/** Function to run on binder interface. */
public interface BinderRunner {
/** Called to run client code with the binder. */
void run(IBinder binder) throws RemoteException;
+ /** Called if an error occurred and the function could not be run. */
+ default void onError() {}
}
/**
@@ -416,6 +411,7 @@
public final void runOnBinder(BinderRunner runner) {
mHandler.post(() -> {
if (mBinder == null) {
+ runner.onError();
return;
}
@@ -425,70 +421,11 @@
// binders may propagate some specific non-RemoteExceptions from the other side
// through the binder as well - we cannot allow those to crash the system server
Log.e(TAG, getLogPrefix() + " exception running on " + mServiceInfo, e);
+ runner.onError();
}
});
}
- /**
- * Runs the given function synchronously if currently connected, and returns the default value
- * if not currently connected or if any exception is thrown. Do not obtain any locks within the
- * BlockingBinderRunner, or risk deadlock. The default value will be returned if there is no
- * service connection when this is run, if a RemoteException occurs, or if the operation times
- * out.
- *
- * @deprecated Using this function is an indication that your AIDL API is broken. Calls from
- * system server to outside MUST be one-way, and so cannot return any result, and this
- * method should not be needed or used. Use a separate callback interface to allow outside
- * components to return results back to the system server.
- */
- @Deprecated
- public final <T> T runOnBinderBlocking(BlockingBinderRunner<T> runner, T defaultValue) {
- try {
- return runOnHandlerBlocking(() -> {
- if (mBinder == null) {
- return defaultValue;
- }
-
- try {
- return runner.run(mBinder);
- } catch (RuntimeException | RemoteException e) {
- // binders may propagate some specific non-RemoteExceptions from the other side
- // through the binder as well - we cannot allow those to crash the system server
- Log.e(TAG, getLogPrefix() + " exception running on " + mServiceInfo, e);
- return defaultValue;
- }
- });
- } catch (InterruptedException | TimeoutException e) {
- return defaultValue;
- }
- }
-
- private <T> T runOnHandlerBlocking(Callable<T> c)
- throws InterruptedException, TimeoutException {
- if (Looper.myLooper() == mHandler.getLooper()) {
- try {
- return c.call();
- } catch (Exception e) {
- // Function cannot throw exception, this should never happen
- throw new IllegalStateException(e);
- }
- } else {
- FutureTask<T> task = new FutureTask<>(c);
- mHandler.post(task);
- try {
- // timeout will unblock callers, in particular if the caller is a binder thread to
- // help reduce binder contention. this will still result in blocking the handler
- // thread which may result in ANRs, but should make problems slightly more rare.
- // the underlying solution is simply not to use this API at all, but that would
- // require large refactors to very legacy code.
- return task.get(BLOCKING_BINDER_TIMEOUT_MS, TimeUnit.MILLISECONDS);
- } catch (ExecutionException e) {
- // Function cannot throw exception, this should never happen
- throw new IllegalStateException(e);
- }
- }
- }
-
private String getLogPrefix() {
return "[" + mIntent.getAction() + "]";
}
diff --git a/services/core/java/com/android/server/location/GeocoderProxy.java b/services/core/java/com/android/server/location/GeocoderProxy.java
index 536f95a..a809aad 100644
--- a/services/core/java/com/android/server/location/GeocoderProxy.java
+++ b/services/core/java/com/android/server/location/GeocoderProxy.java
@@ -18,14 +18,16 @@
import android.annotation.Nullable;
import android.content.Context;
-import android.location.Address;
import android.location.GeocoderParams;
+import android.location.IGeocodeListener;
import android.location.IGeocodeProvider;
+import android.os.IBinder;
+import android.os.RemoteException;
import com.android.internal.os.BackgroundThread;
import com.android.server.ServiceWatcher;
-import java.util.List;
+import java.util.Collections;
/**
* Proxy for IGeocodeProvider implementations.
@@ -63,23 +65,53 @@
return mServiceWatcher.register();
}
- public String getFromLocation(double latitude, double longitude, int maxResults,
- GeocoderParams params, List<Address> addrs) {
- return mServiceWatcher.runOnBinderBlocking(binder -> {
- IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
- return provider.getFromLocation(latitude, longitude, maxResults, params, addrs);
- }, "Service not Available");
+ /**
+ * Geocodes stuff.
+ */
+ public void getFromLocation(double latitude, double longitude, int maxResults,
+ GeocoderParams params, IGeocodeListener listener) {
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) throws RemoteException {
+ IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
+ provider.getFromLocation(latitude, longitude, maxResults, params, listener);
+ }
+
+ @Override
+ public void onError() {
+ try {
+ listener.onResults("Service not Available", Collections.emptyList());
+ } catch (RemoteException e) {
+ // ignore
+ }
+ }
+ });
}
- public String getFromLocationName(String locationName,
+ /**
+ * Geocodes stuff.
+ */
+ public void getFromLocationName(String locationName,
double lowerLeftLatitude, double lowerLeftLongitude,
double upperRightLatitude, double upperRightLongitude, int maxResults,
- GeocoderParams params, List<Address> addrs) {
- return mServiceWatcher.runOnBinderBlocking(binder -> {
- IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
- return provider.getFromLocationName(locationName, lowerLeftLatitude,
- lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
- maxResults, params, addrs);
- }, "Service not Available");
+ GeocoderParams params, IGeocodeListener listener) {
+ mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
+ @Override
+ public void run(IBinder binder) throws RemoteException {
+ IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
+ provider.getFromLocationName(locationName, lowerLeftLatitude,
+ lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
+ maxResults, params, listener);
+ }
+
+ @Override
+ public void onError() {
+ try {
+ listener.onResults("Service not Available", Collections.emptyList());
+ } catch (RemoteException e) {
+ // ignore
+ }
+ }
+ });
}
}