Release wakelock in LocationManagerService if client thread is dead

- also put a timeout on the wakelock so that it won't be held forever.

Bug: 111321558
Fixes: 111321558
Test: Tested on device.
Change-Id: I36e90f73143ff12d8622dbbb3aad643b283d17ce

diff --git a/location/java/android/location/LocationManager.java b/location/java/android/location/LocationManager.java
index 6eb3d8d..ee9e732 100644 (file)
--- a/location/java/android/location/LocationManager.java
+++ b/location/java/android/location/LocationManager.java
@@ -22,7 +22,6 @@ import static android.Manifest.permission.LOCATION_HARDWARE;
 import static android.Manifest.permission.WRITE_SECURE_SETTINGS;
 
 import android.Manifest;
-import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.RequiresFeature;
 import android.annotation.RequiresPermission;
@@ -42,16 +41,11 @@ import android.os.Message;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.UserHandle;
-import android.provider.Settings;
-import android.text.TextUtils;
-import android.util.ArraySet;
 import android.util.Log;
 import com.android.internal.location.ProviderProperties;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
-import java.util.Set;
 
 /**
  * This class provides access to the system location services.  These
@@ -331,7 +325,7 @@ public class LocationManager {
             Message msg = Message.obtain();
             msg.what = TYPE_LOCATION_CHANGED;
             msg.obj = location;
-            mListenerHandler.sendMessage(msg);
+            sendCallbackMessage(msg);
         }
 
         @Override
@@ -345,7 +339,7 @@ public class LocationManager {
                 b.putBundle("extras", extras);
             }
             msg.obj = b;
-            mListenerHandler.sendMessage(msg);
+            sendCallbackMessage(msg);
         }
 
         @Override
@@ -353,7 +347,7 @@ public class LocationManager {
             Message msg = Message.obtain();
             msg.what = TYPE_PROVIDER_ENABLED;
             msg.obj = provider;
-            mListenerHandler.sendMessage(msg);
+            sendCallbackMessage(msg);
         }
 
         @Override
@@ -361,7 +355,13 @@ public class LocationManager {
             Message msg = Message.obtain();
             msg.what = TYPE_PROVIDER_DISABLED;
             msg.obj = provider;
-            mListenerHandler.sendMessage(msg);
+            sendCallbackMessage(msg);
+        }
+
+        private void sendCallbackMessage(Message msg) {
+            if (!mListenerHandler.sendMessage(msg)) {
+                locationCallbackFinished();
+            }
         }
 
         private void _handleMessage(Message msg) {
@@ -384,6 +384,10 @@ public class LocationManager {
                     mListener.onProviderDisabled((String) msg.obj);
                     break;
             }
+            locationCallbackFinished();
+        }
+
+        private void locationCallbackFinished() {
             try {
                 mService.locationCallbackFinished(this);
             } catch (RemoteException e) {

diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java
index 776cf47..232c151 100644 (file)
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -795,6 +795,7 @@ public class LocationManagerService extends ILocationManager.Stub {
      * location updates.
      */
     private final class Receiver implements IBinder.DeathRecipient, PendingIntent.OnFinished {
+        private static final long WAKELOCK_TIMEOUT_MILLIS = 60 * 1000;
         final Identity mIdentity;
         final int mAllowedResolutionLevel;  // resolution level allowed to receiver
 
@@ -838,6 +839,10 @@ public class LocationManagerService extends ILocationManager.Stub {
                 workSource = new WorkSource(mIdentity.mUid, mIdentity.mPackageName);
             }
             mWakeLock.setWorkSource(workSource);
+
+            // For a non-reference counted wakelock, each acquire will reset the timeout, and we
+            // only need to release it once.
+            mWakeLock.setReferenceCounted(false);
         }
 
         @Override
@@ -1099,9 +1104,8 @@ public class LocationManagerService extends ILocationManager.Stub {
         // this must be called while synchronized by caller in a synchronized block
         // containing the sending of the broadcaset
         private void incrementPendingBroadcastsLocked() {
-            if (mPendingBroadcasts++ == 0) {
-                mWakeLock.acquire();
-            }
+            mPendingBroadcasts++;
+            mWakeLock.acquire(WAKELOCK_TIMEOUT_MILLIS);
         }
 
         private void decrementPendingBroadcastsLocked() {