summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSoonil Nagarkar <sooniln@google.com>2020-03-02 15:17:04 -0800
committerSoonil Nagarkar <sooniln@google.com>2020-03-02 16:42:20 -0800
commit5df43c11e5fbbd665496fa2a5b5d077d7024f8b9 (patch)
tree3490a22df43894d11c9213dd29478bd612691357
parenta903ed0d7add0b625c5f1c30cdbb28c5472c0a1c (diff)
Quick fix for possible deadlock in LMS
LMS dispatches in-process location events while holding locks, which can lead to deadlock if combined with another lock. This went from a possibility to a definitive deadlock with the introduction of the getCurrentLocation cancel() API. This fix prevents that deadlock by not holding a lock while canceling. It is still technically possible for other components to introduce deadlock by using their own locks within the system process, but as there are very few location clients inside the system server, and none of them appear to be at risk right now, a complete fix is delayed until S. Bug: 149780283 Test: presubmits Change-Id: I9d202adc65380881969d276c470ef1bf52828e82
-rw-r--r--location/java/android/location/LocationManager.java24
1 files changed, 15 insertions, 9 deletions
diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java
index 6028a8a90141..e2309178b297 100644
--- a/location/java/android/location/LocationManager.java
+++ b/location/java/android/location/LocationManager.java
@@ -2562,22 +2562,28 @@ public class LocationManager {
mRemoteCancellationSignal = remoteCancellationSignal;
}
- public synchronized void cancel() {
- mExecutor = null;
- mConsumer = null;
+ public void cancel() {
+ ICancellationSignal cancellationSignal;
+ synchronized (this) {
+ mExecutor = null;
+ mConsumer = null;
- if (mAlarmManager != null) {
- mAlarmManager.cancel(this);
- mAlarmManager = null;
+ if (mAlarmManager != null) {
+ mAlarmManager.cancel(this);
+ mAlarmManager = null;
+ }
+
+ // ensure only one cancel event will go through
+ cancellationSignal = mRemoteCancellationSignal;
+ mRemoteCancellationSignal = null;
}
- if (mRemoteCancellationSignal != null) {
+ if (cancellationSignal != null) {
try {
- mRemoteCancellationSignal.cancel();
+ cancellationSignal.cancel();
} catch (RemoteException e) {
// ignore
}
- mRemoteCancellationSignal = null;
}
}