Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't obscure system_server crashes

Browse Source 
Location client usage within the system_server process that uses a
direct executor may obscure crashes. Any crash that happens on location
delivery will cause a secondary crash during cleanup within location
code, hiding the original error and causing false-positives around
wakelock cleanup. Ensure that that the original error is not hidden and
that cleanup functions properly.

Test: manual
Change-Id: If930b2d02d852ee04e6617b072f8e790f161734a
This commit is contained in:
Soonil Nagarkar
2022-02-28 11:53:29 -08:00
parent 05b801bd46
commit 519fa36921
2 changed files with 58 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
services/core/java/com/android/server/location/geofence/GeofenceManager.java
View File

@@ -19,7 +19,6 @@ package com.android.server.location.geofence;
import static android.location.LocationManager.FUSED_PROVIDER;
import static android.location.LocationManager.KEY_PROXIMITY_ENTERING;
import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
import static com.android.server.location.LocationPermissions.PERMISSION_FINE;
import android.annotation.Nullable;
@@ -41,6 +40,7 @@ import android.stats.location.LocationStatsEnums;
import android.util.ArraySet;
import com.android.internal.annotations.GuardedBy;
import com.android.server.FgThread;
import com.android.server.PendingIntentUtils;
import com.android.server.location.LocationPermissions;
import com.android.server.location.injector.Injector;
@@ -396,7 +396,7 @@ public class GeofenceManager extends
protected boolean registerWithService(LocationRequest locationRequest,
Collection<GeofenceRegistration> registrations) {
getLocationManager().requestLocationUpdates(FUSED_PROVIDER, locationRequest,
DIRECT_EXECUTOR, this);
FgThread.getExecutor(), this);
return true;
}

63
services/core/java/com/android/server/location/provider/LocationProviderManager.java
View File

@@ -201,18 +201,54 @@ public class LocationProviderManager extends
@Override
public void deliverOnLocationChanged(LocationResult locationResult,
@Nullable IRemoteCallback onCompleteCallback) throws RemoteException {
mListener.onLocationChanged(locationResult.asList(), onCompleteCallback);
try {
mListener.onLocationChanged(locationResult.asList(), onCompleteCallback);
} catch (RuntimeException e) {
// the only way a runtime exception can be thrown here is if the client is in the
// system server process (so that the binder call is executed directly, rather than
// asynchronously in another process), and the client is using a direct executor (so
// any client exceptions bubble directly back to us). we move any exception onto
// another thread so that it can't cause further problems
RuntimeException wrapper = new RuntimeException(e);
FgThread.getExecutor().execute(() -> {
throw wrapper;
});
}
}
@Override
public void deliverOnFlushComplete(int requestCode) throws RemoteException {
mListener.onFlushComplete(requestCode);
try {
mListener.onFlushComplete(requestCode);
} catch (RuntimeException e) {
// the only way a runtime exception can be thrown here is if the client is in the
// system server process (so that the binder call is executed directly, rather than
// asynchronously in another process), and the client is using a direct executor (so
// any client exceptions bubble directly back to us). we move any exception onto
// another thread so that it can't cause further problems
RuntimeException wrapper = new RuntimeException(e);
FgThread.getExecutor().execute(() -> {
throw wrapper;
});
}
}
@Override
public void deliverOnProviderEnabledChanged(String provider, boolean enabled)
throws RemoteException {
mListener.onProviderEnabledChanged(provider, enabled);
try {
mListener.onProviderEnabledChanged(provider, enabled);
} catch (RuntimeException e) {
// the only way a runtime exception can be thrown here is if the client is in the
// system server process (so that the binder call is executed directly, rather than
// asynchronously in another process), and the client is using a direct executor (so
// any client exceptions bubble directly back to us). we move any exception onto
// another thread so that it can't cause further problems
RuntimeException wrapper = new RuntimeException(e);
FgThread.getExecutor().execute(() -> {
throw wrapper;
});
}
}
}
@@ -294,10 +330,23 @@ public class LocationProviderManager extends
throws RemoteException {
// ILocationCallback doesn't currently support completion callbacks
Preconditions.checkState(onCompleteCallback == null);
if (locationResult != null) {
mCallback.onLocation(locationResult.getLastLocation());
} else {
mCallback.onLocation(null);
try {
if (locationResult != null) {
mCallback.onLocation(locationResult.getLastLocation());
} else {
mCallback.onLocation(null);
}
} catch (RuntimeException e) {
// the only way a runtime exception can be thrown here is if the client is in the
// system server process (so that the binder call is executed directly, rather than
// asynchronously in another process), and the client is using a direct executor (so
// any client exceptions bubble directly back to us). we move any exception onto
// another thread so that it can't cause further problems
RuntimeException wrapper = new RuntimeException(e);
FgThread.getExecutor().execute(() -> {
throw wrapper;
});
}
}