Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix race condition generating READY and NOT_SUPPORTED statuses.

Browse Source 
The race condition only affects when the client registers for several (all) location listeners.
And the side efects are benign: only the measurement and navigation message status are incurrectly
being sent to the application, but there are no crashes or any real data from GPS being
misscommunicated.
Also:
- cache the last reported status to filter sending notifications when no changes have occurred
- do some cleanup and refactoring in the code changed

Change-Id: I0692e6b70847dc1ee092d7a05a2c6ba3cd9fa147
This commit is contained in:
destradaa
2015-01-15 18:36:01 -08:00
parent 34efbcedac
commit 13a60b0d41
6 changed files with 99 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
location/java/android/location/GpsNavigationMessageEvent.java
View File

@@ -36,18 +36,18 @@ public class GpsNavigationMessageEvent implements Parcelable {
* The system does not support tracking of GPS Navigation Messages. This status will not change
* in the future.
*/
public static int STATUS_NOT_SUPPORTED = 0;
public static final int STATUS_NOT_SUPPORTED = 0;
/**
* GPS Navigation Messages are successfully being tracked, it will receive updates once they are
* available.
*/
public static int STATUS_READY = 1;
public static final int STATUS_READY = 1;
/**
* GPS provider or Location is disabled, updated will not be received until they are enabled.
*/
public static int STATUS_GPS_LOCATION_DISABLED = 2;
public static final int STATUS_GPS_LOCATION_DISABLED = 2;
private final GpsNavigationMessage mNavigationMessage;

17
services/core/java/com/android/server/location/GpsLocationProvider.java
View File

@@ -25,7 +25,6 @@ import com.android.internal.location.ProviderRequest;
import com.android.internal.R;
import com.android.internal.telephony.Phone;
import com.android.internal.telephony.PhoneConstants;
import com.android.internal.telephony.TelephonyIntents;
import android.app.AlarmManager;
import android.app.AppOpsManager;
@@ -72,7 +71,6 @@ import android.provider.Settings;
import android.provider.Telephony.Carriers;
import android.provider.Telephony.Sms.Intents;
import android.telephony.SmsMessage;
import android.telephony.SubscriptionInfo;
import android.telephony.SubscriptionManager;
import android.telephony.SubscriptionManager.OnSubscriptionsChangedListener;
import android.telephony.TelephonyManager;
@@ -91,7 +89,6 @@ import java.io.StringReader;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Date;
import java.util.List;
import java.util.Map.Entry;
import java.util.Properties;
@@ -395,7 +392,7 @@ public class GpsLocationProvider implements LocationProviderInterface {
private final IGpsStatusProvider mGpsStatusProvider = new IGpsStatusProvider.Stub() {
@Override
public void addGpsStatusListener(IGpsStatusListener listener) throws RemoteException {
public void addGpsStatusListener(IGpsStatusListener listener) {
mListenerHelper.addListener(listener);
}
@@ -681,7 +678,7 @@ public class GpsLocationProvider implements LocationProviderInterface {
mListenerHelper = new GpsStatusListenerHelper(mHandler) {
@Override
protected boolean isAvailableInPlatform() {
return GpsLocationProvider.isSupported();
return isSupported();
}
@Override
@@ -1027,6 +1024,9 @@ public class GpsLocationProvider implements LocationProviderInterface {
if (mC2KServerHost != null) {
native_set_agps_server(AGPS_TYPE_C2K, mC2KServerHost, mC2KServerPort);
}
mGpsMeasurementsProvider.onGpsEnabledChanged();
mGpsNavigationMessageProvider.onGpsEnabledChanged();
} else {
synchronized (mLock) {
mEnabled = false;
@@ -1060,6 +1060,9 @@ public class GpsLocationProvider implements LocationProviderInterface {
// do this before releasing wakelock
native_cleanup();
mGpsMeasurementsProvider.onGpsEnabledChanged();
mGpsNavigationMessageProvider.onGpsEnabledChanged();
}
@Override
@@ -1479,9 +1482,7 @@ public class GpsLocationProvider implements LocationProviderInterface {
}
if (wasNavigating != mNavigating) {
mListenerHelper.onGpsEnabledChanged(mNavigating);
mGpsMeasurementsProvider.onGpsEnabledChanged(mNavigating);
mGpsNavigationMessageProvider.onGpsEnabledChanged(mNavigating);
mListenerHelper.onStatusChanged(mNavigating);
// send an intent to notify that the GPS has been enabled or disabled
Intent intent = new Intent(LocationManager.GPS_ENABLED_CHANGE_ACTION);

29
services/core/java/com/android/server/location/GpsMeasurementsProvider.java
View File

@@ -33,7 +33,7 @@ public abstract class GpsMeasurementsProvider
extends RemoteListenerHelper<IGpsMeasurementsListener> {
private static final String TAG = "GpsMeasurementsProvider";
public GpsMeasurementsProvider(Handler handler) {
protected GpsMeasurementsProvider(Handler handler) {
super(handler, TAG);
}
@@ -49,15 +49,19 @@ public abstract class GpsMeasurementsProvider
}
public void onCapabilitiesUpdated(boolean isGpsMeasurementsSupported) {
int status = isGpsMeasurementsSupported ?
GpsMeasurementsEvent.STATUS_READY :
GpsMeasurementsEvent.STATUS_NOT_SUPPORTED;
setSupported(isGpsMeasurementsSupported, new StatusChangedOperation(status));
setSupported(isGpsMeasurementsSupported);
updateResult();
}
public void onGpsEnabledChanged() {
if (tryUpdateRegistrationWithService()) {
updateResult();
}
}
@Override
protected ListenerOperation<IGpsMeasurementsListener> getHandlerOperation(int result) {
final int status;
int status;
switch (result) {
case RESULT_SUCCESS:
status = GpsMeasurementsEvent.STATUS_READY;
@@ -70,6 +74,8 @@ public abstract class GpsMeasurementsProvider
case RESULT_GPS_LOCATION_DISABLED:
status = GpsMeasurementsEvent.STATUS_GPS_LOCATION_DISABLED;
break;
case RESULT_UNKNOWN:
return null;
default:
Log.v(TAG, "Unhandled addListener result: " + result);
return null;
@@ -77,15 +83,8 @@ public abstract class GpsMeasurementsProvider
return new StatusChangedOperation(status);
}
@Override
protected void handleGpsEnabledChanged(boolean enabled) {
int status = enabled ?
GpsMeasurementsEvent.STATUS_READY :
GpsMeasurementsEvent.STATUS_GPS_LOCATION_DISABLED;
foreach(new StatusChangedOperation(status));
}
private class StatusChangedOperation implements ListenerOperation<IGpsMeasurementsListener> {
private static class StatusChangedOperation
implements ListenerOperation<IGpsMeasurementsListener> {
private final int mStatus;
public StatusChangedOperation(int status) {

28
services/core/java/com/android/server/location/GpsNavigationMessageProvider.java
View File

@@ -33,7 +33,7 @@ public abstract class GpsNavigationMessageProvider
extends RemoteListenerHelper<IGpsNavigationMessageListener> {
private static final String TAG = "GpsNavigationMessageProvider";
public GpsNavigationMessageProvider(Handler handler) {
protected GpsNavigationMessageProvider(Handler handler) {
super(handler, TAG);
}
@@ -50,15 +50,19 @@ public abstract class GpsNavigationMessageProvider
}
public void onCapabilitiesUpdated(boolean isGpsNavigationMessageSupported) {
int status = isGpsNavigationMessageSupported ?
GpsNavigationMessageEvent.STATUS_READY :
GpsNavigationMessageEvent.STATUS_NOT_SUPPORTED;
setSupported(isGpsNavigationMessageSupported, new StatusChangedOperation(status));
setSupported(isGpsNavigationMessageSupported);
updateResult();
}
public void onGpsEnabledChanged() {
if (tryUpdateRegistrationWithService()) {
updateResult();
}
}
@Override
protected ListenerOperation<IGpsNavigationMessageListener> getHandlerOperation(int result) {
final int status;
int status;
switch (result) {
case RESULT_SUCCESS:
status = GpsNavigationMessageEvent.STATUS_READY;
@@ -71,6 +75,8 @@ public abstract class GpsNavigationMessageProvider
case RESULT_GPS_LOCATION_DISABLED:
status = GpsNavigationMessageEvent.STATUS_GPS_LOCATION_DISABLED;
break;
case RESULT_UNKNOWN:
return null;
default:
Log.v(TAG, "Unhandled addListener result: " + result);
return null;
@@ -78,15 +84,7 @@ public abstract class GpsNavigationMessageProvider
return new StatusChangedOperation(status);
}
@Override
protected void handleGpsEnabledChanged(boolean enabled) {
int status = enabled ?
GpsNavigationMessageEvent.STATUS_READY :
GpsNavigationMessageEvent.STATUS_GPS_LOCATION_DISABLED;
foreach(new StatusChangedOperation(status));
}
private class StatusChangedOperation
private static class StatusChangedOperation
implements ListenerOperation<IGpsNavigationMessageListener> {
private final int mStatus;

16
services/core/java/com/android/server/location/GpsStatusListenerHelper.java
View File

@@ -24,14 +24,9 @@ import android.os.RemoteException;
* Implementation of a handler for {@link IGpsStatusListener}.
*/
abstract class GpsStatusListenerHelper extends RemoteListenerHelper<IGpsStatusListener> {
public GpsStatusListenerHelper(Handler handler) {
protected GpsStatusListenerHelper(Handler handler) {
super(handler, "GpsStatusListenerHelper");
Operation nullOperation = new Operation() {
@Override
public void execute(IGpsStatusListener iGpsStatusListener) throws RemoteException {}
};
setSupported(GpsLocationProvider.isSupported(), nullOperation);
setSupported(GpsLocationProvider.isSupported());
}
@Override
@@ -47,10 +42,9 @@ abstract class GpsStatusListenerHelper extends RemoteListenerHelper<IGpsStatusLi
return null;
}
@Override
protected void handleGpsEnabledChanged(boolean enabled) {
public void onStatusChanged(boolean isNavigating) {
Operation operation;
if (enabled) {
if (isNavigating) {
operation = new Operation() {
@Override
public void execute(IGpsStatusListener listener) throws RemoteException {
@@ -114,5 +108,5 @@ abstract class GpsStatusListenerHelper extends RemoteListenerHelper<IGpsStatusLi
foreach(operation);
}
private abstract class Operation implements ListenerOperation<IGpsStatusListener> { }
private interface Operation extends ListenerOperation<IGpsStatusListener> {}
}

80
services/core/java/com/android/server/location/RemoteListenerHelper.java
View File

@@ -26,26 +26,31 @@ import android.os.RemoteException;
import android.util.Log;
import java.util.HashMap;
import java.util.Map;
/**
* A helper class, that handles operations in remote listeners, and tracks for remote process death.
*/
abstract class RemoteListenerHelper<TListener extends IInterface> {
protected static final int RESULT_SUCCESS = 0;
protected static final int RESULT_NOT_AVAILABLE = 1;
protected static final int RESULT_NOT_SUPPORTED = 2;
protected static final int RESULT_GPS_LOCATION_DISABLED = 3;
protected static final int RESULT_INTERNAL_ERROR = 4;
protected static final int RESULT_UNKNOWN = 5;
private final Handler mHandler;
private final String mTag;
private final HashMap<IBinder, LinkedListener> mListenerMap = new HashMap<>();
private final Map<IBinder, LinkedListener> mListenerMap = new HashMap<>();
private boolean mIsRegistered;
private boolean mHasIsSupported;
private boolean mIsSupported;
private int mLastReportedResult = RESULT_UNKNOWN;
protected RemoteListenerHelper(Handler handler, String name) {
Preconditions.checkNotNull(name);
mHandler = handler;
@@ -110,33 +115,11 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
}
}
public void onGpsEnabledChanged(boolean enabled) {
// handle first the sub-class implementation, so any error in registration can take
// precedence
handleGpsEnabledChanged(enabled);
synchronized (mListenerMap) {
if (!enabled) {
tryUnregister();
return;
}
if (mListenerMap.isEmpty()) {
return;
}
if (tryRegister()) {
// registration was successful, there is no need to update the state
return;
}
ListenerOperation<TListener> operation = getHandlerOperation(RESULT_INTERNAL_ERROR);
foreachUnsafe(operation);
}
}
protected abstract boolean isAvailableInPlatform();
protected abstract boolean isGpsEnabled();
protected abstract boolean registerWithService();
protected abstract void unregisterFromService();
protected abstract ListenerOperation<TListener> getHandlerOperation(int result);
protected abstract void handleGpsEnabledChanged(boolean enabled);
protected interface ListenerOperation<TListener extends IInterface> {
void execute(TListener listener) throws RemoteException;
@@ -148,11 +131,40 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
}
}
protected void setSupported(boolean value, ListenerOperation<TListener> notifier) {
protected void setSupported(boolean value) {
synchronized (mListenerMap) {
mHasIsSupported = true;
mIsSupported = value;
foreachUnsafe(notifier);
}
}
protected boolean tryUpdateRegistrationWithService() {
synchronized (mListenerMap) {
if (!isGpsEnabled()) {
tryUnregister();
return true;
}
if (mListenerMap.isEmpty()) {
return true;
}
if (tryRegister()) {
// registration was successful, there is no need to update the state
return true;
}
ListenerOperation<TListener> operation = getHandlerOperation(RESULT_INTERNAL_ERROR);
foreachUnsafe(operation);
return false;
}
}
protected void updateResult() {
synchronized (mListenerMap) {
int newResult = calculateCurrentResultUnsafe();
if (mLastReportedResult == newResult) {
return;
}
foreachUnsafe(getHandlerOperation(newResult));
mLastReportedResult = newResult;
}
}
@@ -183,6 +195,24 @@ abstract class RemoteListenerHelper<TListener extends IInterface> {
mIsRegistered = false;
}
private int calculateCurrentResultUnsafe() {
// update statuses we already know about, starting from the ones that will never change
if (!isAvailableInPlatform()) {
return RESULT_NOT_AVAILABLE;
}
if (!mHasIsSupported || mListenerMap.isEmpty()) {
// we'll update once we have a supported status available
return RESULT_UNKNOWN;
}
if (!mIsSupported) {
return RESULT_NOT_SUPPORTED;
}
if (!isGpsEnabled()) {
return RESULT_GPS_LOCATION_DISABLED;
}
return RESULT_SUCCESS;
}
private class LinkedListener implements IBinder.DeathRecipient {
private final TListener mListener;