diff options
author | Dianne Hackborn <hackbod@google.com> | 2013-06-05 14:53:33 -0700 |
---|---|---|
committer | Dianne Hackborn <hackbod@google.com> | 2013-06-05 14:53:33 -0700 |
commit | f5fdca9dc1528a7f5acec04c2f2a1b99e8f4b338 (patch) | |
tree | e8d0b50e523c5defee16206cc11cd536a5fcf7fc | |
parent | 379517b0d4d47b45b52b40f09ba0eb1a039cb773 (diff) | |
download | frameworks_base-f5fdca9dc1528a7f5acec04c2f2a1b99e8f4b338.zip frameworks_base-f5fdca9dc1528a7f5acec04c2f2a1b99e8f4b338.tar.gz frameworks_base-f5fdca9dc1528a7f5acec04c2f2a1b99e8f4b338.tar.bz2 |
Maybe fix issue #9296868: Crash in system process
There were some paths in LocationManagerService where
mRecivers was being accessed/modified without the lock held.
Update method names to indicate they need to be called with
lock held to make it more clear in the future when such a
problem may happen.
Change-Id: Ie2a9d019155ac7cedd1db298caca75b8fe382ca7
-rw-r--r-- | services/java/com/android/server/LocationManagerService.java | 51 |
1 files changed, 28 insertions, 23 deletions
diff --git a/services/java/com/android/server/LocationManagerService.java b/services/java/com/android/server/LocationManagerService.java index c4ed465..f784030 100644 --- a/services/java/com/android/server/LocationManagerService.java +++ b/services/java/com/android/server/LocationManagerService.java @@ -685,19 +685,21 @@ public class LocationManagerService extends ILocationManager.Stub { @Override public void locationCallbackFinished(ILocationListener listener) { - //Do not use getReceiver here as that will add the ILocationListener to + //Do not use getReceiverLocked here as that will add the ILocationListener to //the receiver list if it is not found. If it is not found then the //LocationListener was removed when it had a pending broadcast and should //not be added back. - IBinder binder = listener.asBinder(); - Receiver receiver = mReceivers.get(binder); - if (receiver != null) { - synchronized (receiver) { - // so wakelock calls will succeed - long identity = Binder.clearCallingIdentity(); - receiver.decrementPendingBroadcastsLocked(); - Binder.restoreCallingIdentity(identity); - } + synchronized (mLock) { + IBinder binder = listener.asBinder(); + Receiver receiver = mReceivers.get(binder); + if (receiver != null) { + synchronized (receiver) { + // so wakelock calls will succeed + long identity = Binder.clearCallingIdentity(); + receiver.decrementPendingBroadcastsLocked(); + Binder.restoreCallingIdentity(identity); + } + } } } @@ -1179,7 +1181,8 @@ public class LocationManagerService extends ILocationManager.Stub { } } - private Receiver getReceiver(ILocationListener listener, int pid, int uid, String packageName) { + private Receiver getReceiverLocked(ILocationListener listener, int pid, int uid, + String packageName) { IBinder binder = listener.asBinder(); Receiver receiver = mReceivers.get(binder); if (receiver == null) { @@ -1196,7 +1199,7 @@ public class LocationManagerService extends ILocationManager.Stub { return receiver; } - private Receiver getReceiver(PendingIntent intent, int pid, int uid, String packageName) { + private Receiver getReceiverLocked(PendingIntent intent, int pid, int uid, String packageName) { Receiver receiver = mReceivers.get(intent); if (receiver == null) { receiver = new Receiver(null, intent, pid, uid, packageName); @@ -1260,7 +1263,7 @@ public class LocationManagerService extends ILocationManager.Stub { } } - private Receiver checkListenerOrIntent(ILocationListener listener, PendingIntent intent, + private Receiver checkListenerOrIntentLocked(ILocationListener listener, PendingIntent intent, int pid, int uid, String packageName) { if (intent == null && listener == null) { throw new IllegalArgumentException("need either listener or intent"); @@ -1268,9 +1271,9 @@ public class LocationManagerService extends ILocationManager.Stub { throw new IllegalArgumentException("cannot register both listener and intent"); } else if (intent != null) { checkPendingIntent(intent); - return getReceiver(intent, pid, uid, packageName); + return getReceiverLocked(intent, pid, uid, packageName); } else { - return getReceiver(listener, pid, uid, packageName); + return getReceiverLocked(listener, pid, uid, packageName); } } @@ -1292,9 +1295,10 @@ public class LocationManagerService extends ILocationManager.Stub { // We don't check for MODE_IGNORED here; we will do that when we go to deliver // a location. checkLocationAccess(uid, packageName, allowedResolutionLevel); - Receiver recevier = checkListenerOrIntent(listener, intent, pid, uid, packageName); synchronized (mLock) { + Receiver recevier = checkListenerOrIntentLocked(listener, intent, pid, uid, + packageName); requestLocationUpdatesLocked(sanitizedRequest, recevier, pid, uid, packageName); } } finally { @@ -1341,16 +1345,17 @@ public class LocationManagerService extends ILocationManager.Stub { final int pid = Binder.getCallingPid(); final int uid = Binder.getCallingUid(); - Receiver receiver = checkListenerOrIntent(listener, intent, pid, uid, packageName); - // providers may use public location API's, need to clear identity - long identity = Binder.clearCallingIdentity(); - try { - synchronized (mLock) { + synchronized (mLock) { + Receiver receiver = checkListenerOrIntentLocked(listener, intent, pid, uid, packageName); + + // providers may use public location API's, need to clear identity + long identity = Binder.clearCallingIdentity(); + try { removeUpdatesLocked(receiver); + } finally { + Binder.restoreCallingIdentity(identity); } - } finally { - Binder.restoreCallingIdentity(identity); } } |