From 5df43c11e5fbbd665496fa2a5b5d077d7024f8b9 Mon Sep 17 00:00:00 2001
From: Soonil Nagarkar <sooniln@google.com>
Date: Mon, 2 Mar 2020 15:17:04 -0800
Subject: 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
---
 .../java/android/location/LocationManager.java     | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

(limited to 'location/java')

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;
             }
         }
 
-- 
cgit v1.2.3-59-g8ed1b