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

Commit 5cd731a2 authored by Victoria Lease's avatar Victoria Lease
Browse files

prevent concurrency issues during LocationManager init 

systemReady() was returning before the LocationManagerService was
actually ready. Applications making LocationManager transactions
during their startup could possibly hit a race condition with the
yet-uninitialised LocationManagerService.

To guarantee that LocationManagerService is actually ready before
returning from systemReady(), we simply do the startup work on the
thread that called systemReady(), rather than spin up a secondary
thread to do the work asynchronously.

LocationWorkerHandler still needs a thread to do its work on, so
rather than have it run on the secondary thread that was
previously used for systemReady()'s work, we create a HandlerThread
for it.

Additionally, LocationManagerService.init() really needed to grab
lock for some of the things it was doing. I moved all of the code
that could benefit from mutex protection to a single section of
systemReady() and wrapped it up with a lock while I was at it.

Bug: 7723944
Change-Id: I51d480e2781622c3a14769c3a2019a2407dcfd8a
parent 4c7f809d

services/java/com/android/server/LocationManagerService.java

+39 −37
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ import android.location.LocationRequest; 
import android.os.Binder;

import android.os.Bundle;

import android.os.Handler;

import android.os.HandlerThread;

import android.os.IBinder;

import android.os.Looper;

import android.os.Message;
@@ -86,7 +87,7 @@ import java.util.Set; 
 * The service class that manages LocationProviders and issues location

 * updates and alerts.

 */

public class LocationManagerService extends ILocationManager.Stub implements Runnable {

public class LocationManagerService extends ILocationManager.Stub {

    private static final String TAG = "LocationManagerService";

    public static final boolean D = false;
@@ -127,7 +128,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 
    // used internally for synchronization

    private final Object mLock = new Object();



    // --- fields below are final after init() ---

    // --- fields below are final after systemReady() ---

    private LocationFudger mLocationFudger;

    private GeofenceManager mGeofenceManager;

    private PowerManager.WakeLock mWakeLock;
@@ -138,6 +139,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 
    private LocationWorkerHandler mLocationHandler;

    private PassiveProvider mPassiveProvider;  // track passive provider for special cases

    private LocationBlacklist mBlacklist;

    private HandlerThread mHandlerThread;



    // --- fields below are protected by mWakeLock ---

    private int mPendingBroadcasts;
@@ -192,36 +194,33 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 
    }



    public void systemReady() {

        Thread thread = new Thread(null, this, THREAD_NAME);

        thread.start();

    }



    @Override

    public void run() {

        Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);

        Looper.prepare();

        mLocationHandler = new LocationWorkerHandler();

        init();

        Looper.loop();

    }

        synchronized (mLock) {

            if (D) Log.d(TAG, "systemReady()");



    private void init() {

        if (D) Log.d(TAG, "init()");

            // fetch package manager

            mPackageManager = mContext.getPackageManager();



        PowerManager powerManager = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);

            // prepare wake lock

            PowerManager powerManager =

                    (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);

            mWakeLock = powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_KEY);

        mPackageManager = mContext.getPackageManager();



            // prepare worker thread

            mHandlerThread = new HandlerThread(THREAD_NAME, Process.THREAD_PRIORITY_BACKGROUND);

            mHandlerThread.start();

            mLocationHandler = new LocationWorkerHandler(mHandlerThread.getLooper());



            // prepare mLocationHandler's dependents

            mLocationFudger = new LocationFudger(mContext, mLocationHandler);

            mBlacklist = new LocationBlacklist(mContext, mLocationHandler);

            mBlacklist.init();

        mLocationFudger = new LocationFudger(mContext, mLocationHandler);

            mGeofenceManager = new GeofenceManager(mContext, mBlacklist);



        synchronized (mLock) {

            // prepare providers

            loadProvidersLocked();

            updateProvidersLocked();

        }



        mGeofenceManager = new GeofenceManager(mContext, mBlacklist);



        // listen for settings changes

        mContext.getContentResolver().registerContentObserver(

                Settings.Secure.getUriFor(Settings.Secure.LOCATION_PROVIDERS_ALLOWED), true,
@@ -233,7 +232,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 
                        }

                    }

                }, UserHandle.USER_ALL);

        mPackageMonitor.register(mContext, Looper.myLooper(), true);

        mPackageMonitor.register(mContext, mLocationHandler.getLooper(), true);



        // listen for user change

        IntentFilter intentFilter = new IntentFilter();
@@ -247,9 +246,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 
                    switchUser(intent.getIntExtra(Intent.EXTRA_USER_HANDLE, 0));

                }

            }

        }, UserHandle.ALL, intentFilter, null, null);



        updateProvidersLocked();

        }, UserHandle.ALL, intentFilter, null, mLocationHandler);

    }



    private void ensureFallbackFusedProviderPresentLocked(ArrayList<String> pkgs) {
@@ -329,7 +326,8 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 


        if (GpsLocationProvider.isSupported()) {

            // Create a gps location provider

            GpsLocationProvider gpsProvider = new GpsLocationProvider(mContext, this);

            GpsLocationProvider gpsProvider = new GpsLocationProvider(mContext, this,

                    mLocationHandler.getLooper());

            mGpsStatusProvider = gpsProvider.getGpsStatusProvider();

            mNetInitiatedListener = gpsProvider.getNetInitiatedListener();

            addProviderLocked(gpsProvider);
@@ -389,7 +387,7 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 


        // bind to geocoder provider

        mGeocodeProvider = GeocoderProxy.createAndBind(mContext, providerPackageNames,

                mCurrentUserId);

                mLocationHandler, mCurrentUserId);

        if (mGeocodeProvider == null) {

            Slog.e(TAG,  "no geocoder provider found");

        }
@@ -1715,6 +1713,10 @@ public class LocationManagerService extends ILocationManager.Stub implements Run 
    }



    private class LocationWorkerHandler extends Handler {

        public LocationWorkerHandler(Looper looper) {

            super(looper, null, true);

        }



        @Override

        public void handleMessage(Message msg) {

            switch (msg.what) {

services/java/com/android/server/location/GeocoderProxy.java

+6 −4
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.content.Context; 
import android.location.Address;

import android.location.GeocoderParams;

import android.location.IGeocodeProvider;

import android.os.Handler;

import android.os.RemoteException;

import android.os.UserHandle;

import android.util.Log;
@@ -39,8 +40,8 @@ public class GeocoderProxy { 
    private final ServiceWatcher mServiceWatcher;



    public static GeocoderProxy createAndBind(Context context,

            List<String> initialPackageNames, int userId) {

        GeocoderProxy proxy = new GeocoderProxy(context, initialPackageNames, userId);

            List<String> initialPackageNames, Handler handler, int userId) {

        GeocoderProxy proxy = new GeocoderProxy(context, initialPackageNames, handler, userId);

        if (proxy.bind()) {

            return proxy;

        } else {
@@ -48,11 +49,12 @@ public class GeocoderProxy { 
        }

    }



    public GeocoderProxy(Context context, List<String> initialPackageNames, int userId) {

    public GeocoderProxy(Context context, List<String> initialPackageNames, Handler handler,

            int userId) {

        mContext = context;



        mServiceWatcher = new ServiceWatcher(mContext, TAG, SERVICE_ACTION, initialPackageNames,

                null, null, userId);

                null, handler, userId);

    }



    private boolean bind () {

services/java/com/android/server/location/GpsLocationProvider.java

+6 −4
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ import android.os.Binder; 
import android.os.Bundle;

import android.os.Handler;

import android.os.IBinder;

import android.os.Looper;

import android.os.Message;

import android.os.PowerManager;

import android.os.RemoteException;
@@ -413,7 +414,8 @@ public class GpsLocationProvider implements LocationProviderInterface { 
        return native_is_supported();

    }



    public GpsLocationProvider(Context context, ILocationManager ilocationManager) {

    public GpsLocationProvider(Context context, ILocationManager ilocationManager,

            Looper looper) {

        mContext = context;

        mNtpTime = NtpTrustedTime.getInstance(context);

        mILocationManager = ilocationManager;
@@ -466,7 +468,7 @@ public class GpsLocationProvider implements LocationProviderInterface { 
        }



        // construct handler, listen for events

        mHandler = new ProviderHandler();

        mHandler = new ProviderHandler(looper);

        listenForBroadcasts();



        // also listen for PASSIVE_PROVIDER updates
@@ -1488,8 +1490,8 @@ public class GpsLocationProvider implements LocationProviderInterface { 
    }



    private final class ProviderHandler extends Handler {

        public ProviderHandler() {

            super(true /*async*/);

        public ProviderHandler(Looper looper) {

            super(looper, null, true /*async*/);

        }



        @Override

services/java/com/android/server/location/LocationProviderProxy.java

+0 −1
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.location.LocationProvider; 
import android.os.Bundle;

import android.os.Handler;

import android.os.RemoteException;

import android.os.UserHandle;

import android.os.WorkSource;

import android.util.Log;