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

Commit e731ca89 authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

Refactor ServiceWatcher

Make thread safe and lock free and fixes a variety of minor bugs. Most
of location code is under the synchronization strategy of 'throw
everything behind one massive lock'. This is a bad idea for a variety
of reasons and makes it very difficult to reason about thread safety.
Correcting this will make code easier to read and understand.

Test: Tested manually on device.

Change-Id: I42ad9b4009eabc04227008d6d562dca557087d1f
parent 20e5558d
Loading
Loading
Loading
Loading
+5 −12
Original line number Diff line number Diff line
@@ -541,7 +541,7 @@ public class LocationManagerService extends ILocationManager.Stub {
        }
    }

    private void ensureFallbackFusedProviderPresentLocked(ArrayList<String> pkgs) {
    private void ensureFallbackFusedProviderPresentLocked(String[] pkgs) {
        PackageManager pm = mContext.getPackageManager();
        String systemPackageName = mContext.getPackageName();
        ArrayList<HashSet<Signature>> sigSets = ServiceWatcher.getSignatureSets(mContext, pkgs);
@@ -646,16 +646,14 @@ public class LocationManagerService extends ILocationManager.Stub {
        that matches the signature of at least one package on this list.
        */
        Resources resources = mContext.getResources();
        ArrayList<String> providerPackageNames = new ArrayList<>();
        String[] pkgs = resources.getStringArray(
                com.android.internal.R.array.config_locationProviderPackageNames);
        if (D) {
            Log.d(TAG, "certificates for location providers pulled from: " +
                    Arrays.toString(pkgs));
        }
        if (pkgs != null) providerPackageNames.addAll(Arrays.asList(pkgs));

        ensureFallbackFusedProviderPresentLocked(providerPackageNames);
        ensureFallbackFusedProviderPresentLocked(pkgs);

        // bind to network provider
        LocationProviderProxy networkProvider = LocationProviderProxy.createAndBind(
@@ -664,8 +662,7 @@ public class LocationManagerService extends ILocationManager.Stub {
                NETWORK_LOCATION_SERVICE_ACTION,
                com.android.internal.R.bool.config_enableNetworkLocationOverlay,
                com.android.internal.R.string.config_networkLocationProviderPackageName,
                com.android.internal.R.array.config_locationProviderPackageNames,
                mLocationHandler);
                com.android.internal.R.array.config_locationProviderPackageNames);
        if (networkProvider != null) {
            mRealProviders.put(LocationManager.NETWORK_PROVIDER, networkProvider);
            mProxyProviders.add(networkProvider);
@@ -681,8 +678,7 @@ public class LocationManagerService extends ILocationManager.Stub {
                FUSED_LOCATION_SERVICE_ACTION,
                com.android.internal.R.bool.config_enableFusedLocationOverlay,
                com.android.internal.R.string.config_fusedLocationProviderPackageName,
                com.android.internal.R.array.config_locationProviderPackageNames,
                mLocationHandler);
                com.android.internal.R.array.config_locationProviderPackageNames);
        if (fusedLocationProvider != null) {
            addProviderLocked(fusedLocationProvider);
            mProxyProviders.add(fusedLocationProvider);
@@ -697,8 +693,7 @@ public class LocationManagerService extends ILocationManager.Stub {
        mGeocodeProvider = GeocoderProxy.createAndBind(mContext,
                com.android.internal.R.bool.config_enableGeocoderOverlay,
                com.android.internal.R.string.config_geocoderProviderPackageName,
                com.android.internal.R.array.config_locationProviderPackageNames,
                mLocationHandler);
                com.android.internal.R.array.config_locationProviderPackageNames);
        if (mGeocodeProvider == null) {
            Slog.e(TAG, "no geocoder provider found");
        }
@@ -708,7 +703,6 @@ public class LocationManagerService extends ILocationManager.Stub {
                mContext, com.android.internal.R.bool.config_enableGeofenceOverlay,
                com.android.internal.R.string.config_geofenceProviderPackageName,
                com.android.internal.R.array.config_locationProviderPackageNames,
                mLocationHandler,
                mGpsGeofenceProxy,
                null);
        if (provider == null) {
@@ -725,7 +719,6 @@ public class LocationManagerService extends ILocationManager.Stub {
        }
        ActivityRecognitionProxy proxy = ActivityRecognitionProxy.createAndBind(
                mContext,
                mLocationHandler,
                activityRecognitionHardwareIsSupported,
                activityRecognitionHardware,
                com.android.internal.R.bool.config_enableActivityRecognitionHardwareOverlay,
+262 −276

File changed.

Preview size limit exceeded, changes collapsed.

+53 −92
Original line number Diff line number Diff line
@@ -16,58 +16,25 @@

package com.android.server.location;

import com.android.server.ServiceWatcher;

import android.content.Context;
import android.hardware.location.ActivityRecognitionHardware;
import android.hardware.location.IActivityRecognitionHardwareClient;
import android.hardware.location.IActivityRecognitionHardwareWatcher;
import android.os.Handler;
import android.os.IBinder;
import android.os.RemoteException;
import android.util.Log;

import com.android.internal.os.BackgroundThread;
import com.android.server.ServiceWatcher;

/**
 * Proxy class to bind GmsCore to the ActivityRecognitionHardware.
 *
 * @hide
 */
public class ActivityRecognitionProxy {
    private static final String TAG = "ActivityRecognitionProxy";

    private final ServiceWatcher mServiceWatcher;
    private final boolean mIsSupported;
    private final ActivityRecognitionHardware mInstance;

    private ActivityRecognitionProxy(
            Context context,
            Handler handler,
            boolean activityRecognitionHardwareIsSupported,
            ActivityRecognitionHardware activityRecognitionHardware,
            int overlaySwitchResId,
            int defaultServicePackageNameResId,
            int initialPackageNameResId) {
        mIsSupported = activityRecognitionHardwareIsSupported;
        mInstance = activityRecognitionHardware;

        Runnable newServiceWork = new Runnable() {
            @Override
            public void run() {
                bindProvider();
            }
        };

        // prepare the connection to the provider
        mServiceWatcher = new ServiceWatcher(
                context,
                TAG,
                "com.android.location.service.ActivityRecognitionProvider",
                overlaySwitchResId,
                defaultServicePackageNameResId,
                initialPackageNameResId,
                newServiceWork,
                handler);
    }
    private static final String TAG = "ActivityRecognitionProxy";

    /**
     * Creates an instance of the proxy and binds it to the appropriate FusedProvider.
@@ -76,7 +43,6 @@ public class ActivityRecognitionProxy {
     */
    public static ActivityRecognitionProxy createAndBind(
            Context context,
            Handler handler,
            boolean activityRecognitionHardwareIsSupported,
            ActivityRecognitionHardware activityRecognitionHardware,
            int overlaySwitchResId,
@@ -84,74 +50,69 @@ public class ActivityRecognitionProxy {
            int initialPackageNameResId) {
        ActivityRecognitionProxy activityRecognitionProxy = new ActivityRecognitionProxy(
                context,
                handler,
                activityRecognitionHardwareIsSupported,
                activityRecognitionHardware,
                overlaySwitchResId,
                defaultServicePackageNameResId,
                initialPackageNameResId);

        // try to bind the provider
        if (!activityRecognitionProxy.mServiceWatcher.start()) {
            Log.e(TAG, "ServiceWatcher could not start.");
        if (activityRecognitionProxy.mServiceWatcher.start()) {
            return activityRecognitionProxy;
        } else {
            return null;
        }
        return activityRecognitionProxy;
    }

    /**
     * Helper function to bind the FusedLocationHardware to the appropriate FusedProvider instance.
     */
    private void bindProvider() {
        if (!mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
    private final ServiceWatcher mServiceWatcher;
    private final boolean mIsSupported;
    private final ActivityRecognitionHardware mInstance;

    private ActivityRecognitionProxy(
            Context context,
            boolean activityRecognitionHardwareIsSupported,
            ActivityRecognitionHardware activityRecognitionHardware,
            int overlaySwitchResId,
            int defaultServicePackageNameResId,
            int initialPackageNameResId) {
        mIsSupported = activityRecognitionHardwareIsSupported;
        mInstance = activityRecognitionHardware;

        mServiceWatcher = new ServiceWatcher(
                context,
                TAG,
                "com.android.location.service.ActivityRecognitionProvider",
                overlaySwitchResId,
                defaultServicePackageNameResId,
                initialPackageNameResId,
                BackgroundThread.getHandler()) {
            @Override
            public void run(IBinder binder) {
                String descriptor;
                try {
                    descriptor = binder.getInterfaceDescriptor();
                } catch (RemoteException e) {
                    Log.e(TAG, "Unable to get interface descriptor.", e);
                    return;
            protected void onBind() {
                runOnBinder(ActivityRecognitionProxy.this::initializeService);
            }
        };
    }

                if (IActivityRecognitionHardwareWatcher.class.getCanonicalName()
                        .equals(descriptor)) {
    private void initializeService(IBinder binder) {
        try {
            String descriptor = binder.getInterfaceDescriptor();

            if (IActivityRecognitionHardwareWatcher.class.getCanonicalName().equals(
                    descriptor)) {
                IActivityRecognitionHardwareWatcher watcher =
                        IActivityRecognitionHardwareWatcher.Stub.asInterface(binder);
                    if (watcher == null) {
                        Log.e(TAG, "No watcher found on connection.");
                        return;
                    }
                    if (mInstance == null) {
                        // to keep backwards compatibility do not update the watcher when there is
                        // no instance available, or it will cause an NPE
                        Log.d(TAG, "AR HW instance not available, binding will be a no-op.");
                        return;
                    }
                    try {
                if (mInstance != null) {
                    watcher.onInstanceChanged(mInstance);
                    } catch (RemoteException e) {
                        Log.e(TAG, "Error delivering hardware interface to watcher.", e);
                }
            } else if (IActivityRecognitionHardwareClient.class.getCanonicalName()
                    .equals(descriptor)) {
                IActivityRecognitionHardwareClient client =
                        IActivityRecognitionHardwareClient.Stub.asInterface(binder);
                    if (client == null) {
                        Log.e(TAG, "No client found on connection.");
                        return;
                    }
                    try {
                client.onAvailabilityChanged(mIsSupported, mInstance);
                    } catch (RemoteException e) {
                        Log.e(TAG, "Error delivering hardware interface to client.", e);
                    }
            } else {
                Log.e(TAG, "Invalid descriptor found on connection: " + descriptor);
            }
            }
        })) {
            Log.e(TAG, "Null binder found on connection.");
        } catch (RemoteException e) {
            Log.w(TAG, e);
        }
    }
}
+27 −35
Original line number Diff line number Diff line
@@ -20,12 +20,12 @@ import android.content.Context;
import android.location.Address;
import android.location.GeocoderParams;
import android.location.IGeocodeProvider;
import android.os.Handler;
import android.os.IBinder;
import android.os.RemoteException;
import android.util.Log;

import com.android.internal.os.BackgroundThread;
import com.android.server.ServiceWatcher;

import java.util.List;

/**
@@ -36,14 +36,13 @@ public class GeocoderProxy {

    private static final String SERVICE_ACTION = "com.android.location.service.GeocodeProvider";

    private final Context mContext;
    private final ServiceWatcher mServiceWatcher;

    public static GeocoderProxy createAndBind(Context context,
            int overlaySwitchResId, int defaultServicePackageNameResId,
            int initialPackageNamesResId, Handler handler) {
            int initialPackageNamesResId) {
        GeocoderProxy proxy = new GeocoderProxy(context, overlaySwitchResId,
            defaultServicePackageNameResId, initialPackageNamesResId, handler);
                defaultServicePackageNameResId, initialPackageNamesResId);
        if (proxy.bind()) {
            return proxy;
        } else {
@@ -53,11 +52,10 @@ public class GeocoderProxy {

    private GeocoderProxy(Context context,
            int overlaySwitchResId, int defaultServicePackageNameResId,
            int initialPackageNamesResId, Handler handler) {
        mContext = context;

        mServiceWatcher = new ServiceWatcher(mContext, TAG, SERVICE_ACTION, overlaySwitchResId,
            defaultServicePackageNameResId, initialPackageNamesResId, null, handler);
            int initialPackageNamesResId) {
        mServiceWatcher = new ServiceWatcher(context, TAG, SERVICE_ACTION, overlaySwitchResId,
                defaultServicePackageNameResId, initialPackageNamesResId,
                BackgroundThread.getHandler());
    }

    private boolean bind() {
@@ -65,15 +63,13 @@ public class GeocoderProxy {
    }

    public String getConnectedPackageName() {
        return mServiceWatcher.getBestPackageName();
        return mServiceWatcher.getCurrentPackageName();
    }

    public String getFromLocation(double latitude, double longitude, int maxResults,
            GeocoderParams params, List<Address> addrs) {
        final String[] result = new String[]{"Service not Available"};
        mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
            @Override
            public void run(IBinder binder) {
        mServiceWatcher.runOnBinder(binder -> {
            IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
            try {
                result[0] = provider.getFromLocation(
@@ -81,7 +77,6 @@ public class GeocoderProxy {
            } catch (RemoteException e) {
                Log.w(TAG, e);
            }
            }
        });
        return result[0];
    }
@@ -91,9 +86,7 @@ public class GeocoderProxy {
            double upperRightLatitude, double upperRightLongitude, int maxResults,
            GeocoderParams params, List<Address> addrs) {
        final String[] result = new String[]{"Service not Available"};
        mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
            @Override
            public void run(IBinder binder) {
        mServiceWatcher.runOnBinder(binder -> {
            IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
            try {
                result[0] = provider.getFromLocationName(locationName, lowerLeftLatitude,
@@ -102,7 +95,6 @@ public class GeocoderProxy {
            } catch (RemoteException e) {
                Log.w(TAG, e);
            }
            }
        });
        return result[0];
    }
+59 −115
Original line number Diff line number Diff line
@@ -15,61 +15,60 @@
 */
package com.android.server.location;

import android.annotation.Nullable;
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.content.ServiceConnection;
import android.hardware.location.GeofenceHardwareService;
import android.hardware.location.IGeofenceHardware;
import android.location.IFusedGeofenceHardware;
import android.location.IGeofenceProvider;
import android.location.IGpsGeofenceHardware;
import android.location.IFusedGeofenceHardware;
import android.content.Context;
import android.os.Handler;
import android.os.IBinder;
import android.os.Message;
import android.os.RemoteException;
import android.os.UserHandle;
import android.util.Log;

import com.android.internal.os.BackgroundThread;
import com.android.server.ServiceWatcher;

/**
 * @hide
 */
public final class GeofenceProxy {

    private static final String TAG = "GeofenceProxy";
    private static final String SERVICE_ACTION =
            "com.android.location.service.GeofenceProvider";
    private final ServiceWatcher mServiceWatcher;
    private static final String SERVICE_ACTION = "com.android.location.service.GeofenceProvider";

    private final Context mContext;
    private final ServiceWatcher mServiceWatcher;

    @Nullable
    private final IGpsGeofenceHardware mGpsGeofenceHardware;
    @Nullable
    private final IFusedGeofenceHardware mFusedGeofenceHardware;

    private final Object mLock = new Object();

    // Access to mGeofenceHardware needs to be synchronized by mLock.
    private IGeofenceHardware mGeofenceHardware;
    private volatile IGeofenceHardware mGeofenceHardware;

    private static final int GEOFENCE_PROVIDER_CONNECTED = 1;
    private static final int GEOFENCE_HARDWARE_CONNECTED = 2;
    private static final int GEOFENCE_HARDWARE_DISCONNECTED = 3;
    private static final int GEOFENCE_GPS_HARDWARE_CONNECTED = 4;
    private static final int GEOFENCE_GPS_HARDWARE_DISCONNECTED = 5;

    private Runnable mRunnable = new Runnable() {
        @Override
        public void run() {
            mHandler.sendEmptyMessage(GEOFENCE_PROVIDER_CONNECTED);
    private final ServiceWatcher.BinderRunner mUpdateGeofenceHardware = (binder) -> {
        IGeofenceProvider provider = IGeofenceProvider.Stub.asInterface(binder);
        try {
            provider.setGeofenceHardware(mGeofenceHardware);
        } catch (RemoteException e) {
            Log.w(TAG, e);
        }
    };

    public static GeofenceProxy createAndBind(Context context,
            int overlaySwitchResId, int defaultServicePackageNameResId,
            int initialPackageNamesResId, Handler handler, IGpsGeofenceHardware gpsGeofence,
            IFusedGeofenceHardware fusedGeofenceHardware) {
            int initialPackageNamesResId, @Nullable IGpsGeofenceHardware gpsGeofence,
            @Nullable IFusedGeofenceHardware fusedGeofenceHardware) {
        GeofenceProxy proxy = new GeofenceProxy(context, overlaySwitchResId,
            defaultServicePackageNameResId, initialPackageNamesResId, handler, gpsGeofence,
                defaultServicePackageNameResId, initialPackageNamesResId, gpsGeofence,
                fusedGeofenceHardware);
        if (proxy.bindGeofenceProvider()) {

        if (proxy.bind()) {
            return proxy;
        } else {
            return null;
@@ -78,111 +77,56 @@ public final class GeofenceProxy {

    private GeofenceProxy(Context context,
            int overlaySwitchResId, int defaultServicePackageNameResId,
            int initialPackageNamesResId, Handler handler, IGpsGeofenceHardware gpsGeofence,
            IFusedGeofenceHardware fusedGeofenceHardware) {
            int initialPackageNamesResId, @Nullable IGpsGeofenceHardware gpsGeofence,
            @Nullable IFusedGeofenceHardware fusedGeofenceHardware) {
        mContext = context;
        mServiceWatcher = new ServiceWatcher(context, TAG, SERVICE_ACTION, overlaySwitchResId,
            defaultServicePackageNameResId, initialPackageNamesResId, mRunnable, handler);
                defaultServicePackageNameResId, initialPackageNamesResId,
                BackgroundThread.getHandler()) {
            @Override
            protected void onBind() {
                runOnBinder(mUpdateGeofenceHardware);
            }
        };

        mGpsGeofenceHardware = gpsGeofence;
        mFusedGeofenceHardware = fusedGeofenceHardware;
        bindHardwareGeofence();
    }

    private boolean bindGeofenceProvider() {
        return mServiceWatcher.start();
        mGeofenceHardware = null;
    }

    private void bindHardwareGeofence() {
    private boolean bind() {
        if (mServiceWatcher.start()) {
            mContext.bindServiceAsUser(new Intent(mContext, GeofenceHardwareService.class),
                mServiceConnection, Context.BIND_AUTO_CREATE, UserHandle.SYSTEM);
                    new GeofenceProxyServiceConnection(), Context.BIND_AUTO_CREATE,
                    UserHandle.SYSTEM);
            return true;
        }

    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);
            }
        return false;
    }

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

    private void setGeofenceHardwareInProviderLocked() {
        mServiceWatcher.runOnBinder(new ServiceWatcher.BinderRunner() {
        @Override
            public void run(IBinder binder) {
                final IGeofenceProvider provider = IGeofenceProvider.Stub.asInterface(binder);
                try {
                    provider.setGeofenceHardware(mGeofenceHardware);
                } catch (RemoteException e) {
                    Log.e(TAG, "Remote Exception: setGeofenceHardwareInProviderLocked: " + e);
                }
            }
        });
    }
        public void onServiceConnected(ComponentName name, IBinder service) {
            IGeofenceHardware geofenceHardware = IGeofenceHardware.Stub.asInterface(service);

    private void setGpsGeofenceLocked() {
            try {
            if (mGpsGeofenceHardware != null) {
                mGeofenceHardware.setGpsGeofenceHardware(mGpsGeofenceHardware);
            }
        } catch (RemoteException e) {
            Log.e(TAG, "Error while connecting to GeofenceHardwareService");
        }
    }
                geofenceHardware.setGpsGeofenceHardware(mGpsGeofenceHardware);
                geofenceHardware.setFusedGeofenceHardware(mFusedGeofenceHardware);

    private void setFusedGeofenceLocked() {
        try {
            mGeofenceHardware.setFusedGeofenceHardware(mFusedGeofenceHardware);
        } catch(RemoteException e) {
            Log.e(TAG, "Error while connecting to GeofenceHardwareService");
                mGeofenceHardware = geofenceHardware;
                mServiceWatcher.runOnBinder(mUpdateGeofenceHardware);
            } catch (Exception e) {
                Log.w(TAG, e);
            }
        }

    // 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() {

        @Override
        public void handleMessage(Message msg) {
            switch (msg.what) {
                case GEOFENCE_PROVIDER_CONNECTED:
                    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:
                    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:
                    synchronized (mLock) {
                        if (mGeofenceHardware == null) {
                            setGeofenceHardwareInProviderLocked();
                        }
                    }
                    break;
        public void onServiceDisconnected(ComponentName name) {
            mGeofenceHardware = null;
            mServiceWatcher.runOnBinder(mUpdateGeofenceHardware);
        }
    }
    };
}
Loading