DO NOT MERGE Prevent isEnabled from mutating state

When isEnabled sees a user it has not encountered before it may send out
client updates, which can result in the client list and associated state
being mutated unexpectedly, causing ConcurrentModificationExceptions. In
order to prevent this, we now mutate the state asynchronously.

Bug: 171910679
Test: unable to verify this manually, as this is a near impossible bug
to reproduce intentionally

Change-Id: I64ccdcdd50294b875d4ed22de60bff0823812ad7
This commit is contained in:
Soonil Nagarkar
2020-11-06 11:46:16 -08:00
parent 69f60a9090
commit ff147d5052

View File

@@ -906,9 +906,26 @@ public class LocationManagerService extends ILocationManager.Stub {
if (enabled == null) { if (enabled == null) {
// this generally shouldn't occur, but might be possible due to race conditions // this generally shouldn't occur, but might be possible due to race conditions
// on when we are notified of new users // on when we are notified of new users
// hack to fix b/171910679. mutating the user enabled state within this method
// may cause unexpected changes to other state (for instance, this could cause
// provider enable/disable notifications to be sent to clients, which could
// result in a dead client being detected, which could result in the client
// being removed, which means that if this function is called while clients are
// being iterated over we have now unexpectedly mutated the iterated
// collection). instead, we return a correct value immediately here, and
// schedule the actual update for later. this has been completely rewritten and
// is no longer a problem in the next version of android.
enabled = mProvider.getState().allowed
&& mUserInfoHelper.isCurrentUserId(userId)
&& mSettingsHelper.isLocationEnabled(userId);
Log.w(TAG, mName + " provider saw user " + userId + " unexpectedly"); Log.w(TAG, mName + " provider saw user " + userId + " unexpectedly");
onEnabledChangedLocked(userId); mHandler.post(() -> {
enabled = Objects.requireNonNull(mEnabled.get(userId)); synchronized (mLock) {
onEnabledChangedLocked(userId);
}
});
} }
return enabled; return enabled;