Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d535ead2 authored by Zhentao Sun's avatar Zhentao Sun
Browse files

Fixed b/10512887.

This is an issue caused by multi-threading. If geofence provider service
is connected and disconnected immediately, the ServiceWatcher can return a
null service handle to the private thread used by GeofenceProxy, and
this can cause NPE and system crash.
This CL also fixed a hidden race conditions bug where mGeofenceHardware
is not synchronized between two threads.

Change-Id: I824642cd638fbb1e6799a5a1220b047ebc2556a1
parent 7be3a138
Loading
Loading
Loading
Loading
+45 −30
Original line number Diff line number Diff line
@@ -41,11 +41,15 @@ public final class GeofenceProxy {
    private static final String TAG = "GeofenceProxy";
    private static final String SERVICE_ACTION =
            "com.android.location.service.GeofenceProvider";
    private ServiceWatcher mServiceWatcher;
    private Context mContext;
    private final ServiceWatcher mServiceWatcher;
    private final Context mContext;
    private final IGpsGeofenceHardware mGpsGeofenceHardware;
    private final IFusedGeofenceHardware mFusedGeofenceHardware;

    private final Object mLock = new Object();

    // Access to mGeofenceHardware needs to be synchronized by mLock.
    private IGeofenceHardware mGeofenceHardware;
    private IGpsGeofenceHardware mGpsGeofenceHardware;
    private IFusedGeofenceHardware mFusedGeofenceHardware;

    private static final int GEOFENCE_PROVIDER_CONNECTED = 1;
    private static final int GEOFENCE_HARDWARE_CONNECTED = 2;
@@ -90,10 +94,6 @@ public final class GeofenceProxy {
        return mServiceWatcher.start();
    }

    private IGeofenceProvider getGeofenceProviderService() {
        return IGeofenceProvider.Stub.asInterface(mServiceWatcher.getBinder());
    }

    private void bindHardwareGeofence() {
        mContext.bindServiceAsUser(new Intent(mContext, GeofenceHardwareService.class),
                mServiceConnection, Context.BIND_AUTO_CREATE, UserHandle.OWNER);
@@ -102,26 +102,34 @@ public final class GeofenceProxy {
    private ServiceConnection mServiceConnection = new ServiceConnection() {
        @Override
        public void onServiceConnected(ComponentName name, IBinder service) {
            synchronized (mLock) {
                mGeofenceHardware = IGeofenceHardware.Stub.asInterface(service);
                mHandler.sendEmptyMessage(GEOFENCE_HARDWARE_CONNECTED);
            }
        }

        @Override
        public void onServiceDisconnected(ComponentName name) {
            synchronized (mLock) {
                mGeofenceHardware = null;
                mHandler.sendEmptyMessage(GEOFENCE_HARDWARE_DISCONNECTED);
            }
        }
    };

    private void setGeofenceHardwareInProvider() {
    private void setGeofenceHardwareInProviderLocked() {
        try {
            getGeofenceProviderService().setGeofenceHardware(mGeofenceHardware);
            IGeofenceProvider provider = IGeofenceProvider.Stub.asInterface(
                      mServiceWatcher.getBinder());
            if (provider != null) {
                provider.setGeofenceHardware(mGeofenceHardware);
            }
        } catch (RemoteException e) {
            Log.e(TAG, "Remote Exception: setGeofenceHardwareInProvider: " + e);
            Log.e(TAG, "Remote Exception: setGeofenceHardwareInProviderLocked: " + e);
        }
    }

    private void setGpsGeofence() {
    private void setGpsGeofenceLocked() {
        try {
            mGeofenceHardware.setGpsGeofenceHardware(mGpsGeofenceHardware);
        } catch (RemoteException e) {
@@ -129,7 +137,7 @@ public final class GeofenceProxy {
        }
    }

    private void setFusedGeofence() {
    private void setFusedGeofenceLocked() {
        try {
            mGeofenceHardware.setFusedGeofenceHardware(mFusedGeofenceHardware);
        } catch(RemoteException e) {
@@ -140,30 +148,37 @@ public final class GeofenceProxy {
    // This needs to be reworked, when more services get added,
    // Might need a state machine or add a framework utility class,
    private Handler mHandler = new Handler() {
        private boolean mGeofenceHardwareConnected = false;
        private boolean mGeofenceProviderConnected = false;


        @Override
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case GEOFENCE_PROVIDER_CONNECTED:
                    mGeofenceProviderConnected = true;
                    if (mGeofenceHardwareConnected) {
                        setGeofenceHardwareInProvider();
                    synchronized (mLock) {
                        if (mGeofenceHardware != null) {
                            setGeofenceHardwareInProviderLocked();
                        }
                        // else: the geofence provider will be notified when the connection to
                        // GeofenceHardwareService is established.
                    }
                    break;
                case GEOFENCE_HARDWARE_CONNECTED:
                    setGpsGeofence();
                    setFusedGeofence();
                    mGeofenceHardwareConnected = true;
                    if (mGeofenceProviderConnected) {
                        setGeofenceHardwareInProvider();
                    synchronized (mLock) {
                        // Theoretically this won't happen because once the GeofenceHardwareService
                        // is connected to, we won't lose connection to it because it's a system
                        // service. But this check does make the code more robust.
                        if (mGeofenceHardware != null) {
                            setGpsGeofenceLocked();
                            setFusedGeofenceLocked();
                            setGeofenceHardwareInProviderLocked();
                        }
                    }
                    break;
                case GEOFENCE_HARDWARE_DISCONNECTED:
                    mGeofenceHardwareConnected = false;
                    setGeofenceHardwareInProvider();
                    synchronized (mLock) {
                        if (mGeofenceHardware == null) {
                            setGeofenceHardwareInProviderLocked();
                        }
                    }
                    break;
            }
        }