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

Commit 6dc9f05f authored by Yu-Han Yang's avatar Yu-Han Yang
Browse files

Not to use handler thread in GnssGeofenceProvider 

- Synchronize the calls to native methods with a lock.
- When native calls come back to GnssLocationProvider, make sure to post
tasks on the background thread so the lock is released.

Bug: 116788068
Change-Id: I613c9bb7190ce19100b2bc154e3cda92bf44e2a7
Fixes: 116788068
Test: atest GnssGeofenceProviderTest
parent 71988345

services/core/java/com/android/server/location/GnssGeofenceProvider.java

+19 −34
Original line number Diff line number Diff line 
package com.android.server.location;



import android.location.IGpsGeofenceHardware;

import android.os.Handler;

import android.os.IBinder;

import android.os.Looper;

import android.util.Log;

import android.util.SparseArray;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;



import java.util.concurrent.Callable;

import java.util.concurrent.ExecutionException;

import java.util.concurrent.FutureTask;



/**

 * Manages GNSS Geofence operations.

 */
@@ -34,26 +28,26 @@ class GnssGeofenceProvider extends IGpsGeofenceHardware.Stub { 
        public boolean paused;

    }



    private final Object mLock = new Object();

    @GuardedBy("mLock")

    private final GnssGeofenceProviderNative mNative;

    @GuardedBy("mLock")

    private final SparseArray<GeofenceEntry> mGeofenceEntries = new SparseArray<>();

    private final Handler mHandler;



    GnssGeofenceProvider(Looper looper) {

        this(looper, new GnssGeofenceProviderNative());

    GnssGeofenceProvider() {

        this(new GnssGeofenceProviderNative());

    }



    @VisibleForTesting

    GnssGeofenceProvider(Looper looper, GnssGeofenceProviderNative gnssGeofenceProviderNative) {

        mHandler = new Handler(looper);

    GnssGeofenceProvider(GnssGeofenceProviderNative gnssGeofenceProviderNative) {

        mNative = gnssGeofenceProviderNative;

    }



    // TODO(b/37460011): use this method in HAL death recovery.

    void resumeIfStarted() {

        if (DEBUG) {

            Log.d(TAG, "resumeIfStarted");

        }

        mHandler.post(() -> {

        synchronized (mLock) {

            for (int i = 0; i < mGeofenceEntries.size(); i++) {

                GeofenceEntry entry = mGeofenceEntries.valueAt(i);

                boolean added = mNative.addGeofence(entry.geofenceId, entry.latitude,
@@ -65,30 +59,21 @@ class GnssGeofenceProvider extends IGpsGeofenceHardware.Stub { 
                    mNative.pauseGeofence(entry.geofenceId);

                }

            }

        });

    }



    private boolean runOnHandlerThread(Callable<Boolean> callable) {

        FutureTask<Boolean> futureTask = new FutureTask<>(callable);

        mHandler.post(futureTask);

        try {

            return futureTask.get();

        } catch (InterruptedException | ExecutionException e) {

            Log.e(TAG, "Failed running callable.", e);

        }

        return false;

    }



    @Override

    public boolean isHardwareGeofenceSupported() {

        return runOnHandlerThread(mNative::isGeofenceSupported);

        synchronized (mLock) {

            return mNative.isGeofenceSupported();

        }

    }



    @Override

    public boolean addCircularHardwareGeofence(int geofenceId, double latitude,

            double longitude, double radius, int lastTransition, int monitorTransitions,

            int notificationResponsiveness, int unknownTimer) {

        return runOnHandlerThread(() -> {

        synchronized (mLock) {

            boolean added = mNative.addGeofence(geofenceId, latitude, longitude, radius,

                    lastTransition, monitorTransitions, notificationResponsiveness,

                    unknownTimer);
@@ -105,23 +90,23 @@ class GnssGeofenceProvider extends IGpsGeofenceHardware.Stub { 
                mGeofenceEntries.put(geofenceId, entry);

            }

            return added;

        });

        }

    }



    @Override

    public boolean removeHardwareGeofence(int geofenceId) {

        return runOnHandlerThread(() -> {

        synchronized (mLock) {

            boolean removed = mNative.removeGeofence(geofenceId);

            if (removed) {

                mGeofenceEntries.remove(geofenceId);

            }

            return removed;

        });

        }

    }



    @Override

    public boolean pauseHardwareGeofence(int geofenceId) {

        return runOnHandlerThread(() -> {

        synchronized (mLock) {

            boolean paused = mNative.pauseGeofence(geofenceId);

            if (paused) {

                GeofenceEntry entry = mGeofenceEntries.get(geofenceId);
@@ -130,12 +115,12 @@ class GnssGeofenceProvider extends IGpsGeofenceHardware.Stub { 
                }

            }

            return paused;

        });

        }

    }



    @Override

    public boolean resumeHardwareGeofence(int geofenceId, int monitorTransitions) {

        return runOnHandlerThread(() -> {

        synchronized (mLock) {

            boolean resumed = mNative.resumeGeofence(geofenceId, monitorTransitions);

            if (resumed) {

                GeofenceEntry entry = mGeofenceEntries.get(geofenceId);
@@ -145,7 +130,7 @@ class GnssGeofenceProvider extends IGpsGeofenceHardware.Stub { 
                }

            }

            return resumed;

        });

        }

    }



    @VisibleForTesting

services/core/java/com/android/server/location/GnssLocationProvider.java

+52 −40
Original line number Diff line number Diff line
@@ -762,7 +762,7 @@ public class GnssLocationProvider extends LocationProviderInterface 
                looper, this);

        mHandler.post(mGnssSatelliteBlacklistHelper::updateSatelliteBlacklist);

        mGnssBatchingProvider = new GnssBatchingProvider();

        mGnssGeofenceProvider = new GnssGeofenceProvider(looper);

        mGnssGeofenceProvider = new GnssGeofenceProvider();

    }



    /**
@@ -1824,7 +1824,7 @@ public class GnssLocationProvider extends LocationProviderInterface 
    /**

     * Converts the GPS HAL status to the internal Geofence Hardware status.

     */

    private int getGeofenceStatus(int status) {

    private static int getGeofenceStatus(int status) {

        switch (status) {

            case GPS_GEOFENCE_OPERATION_SUCCESS:

                return GeofenceHardware.GEOFENCE_SUCCESS;
@@ -1849,6 +1849,7 @@ public class GnssLocationProvider extends LocationProviderInterface 
     */

    private void reportGeofenceTransition(int geofenceId, Location location, int transition,

            long transitionTimestamp) {

        mHandler.post(() -> {

            if (mGeofenceHardwareImpl == null) {

                mGeofenceHardwareImpl = GeofenceHardwareImpl.getInstance(mContext);

            }
@@ -1860,12 +1861,14 @@ public class GnssLocationProvider extends LocationProviderInterface 
                    transitionTimestamp,

                    GeofenceHardware.MONITORING_TYPE_GPS_HARDWARE,

                    FusedBatchOptions.SourceTechnologies.GNSS);

        });

    }



    /**

     * called from native code to report GPS status change.

     */

    private void reportGeofenceStatus(int status, Location location) {

        mHandler.post(() -> {

            if (mGeofenceHardwareImpl == null) {

                mGeofenceHardwareImpl = GeofenceHardwareImpl.getInstance(mContext);

            }
@@ -1878,46 +1881,55 @@ public class GnssLocationProvider extends LocationProviderInterface 
                    monitorStatus,

                    location,

                    FusedBatchOptions.SourceTechnologies.GNSS);

        });

    }



    /**

     * called from native code - Geofence Add callback

     */

    private void reportGeofenceAddStatus(int geofenceId, int status) {

        mHandler.post(() -> {

            if (mGeofenceHardwareImpl == null) {

                mGeofenceHardwareImpl = GeofenceHardwareImpl.getInstance(mContext);

            }

            mGeofenceHardwareImpl.reportGeofenceAddStatus(geofenceId, getGeofenceStatus(status));

        });

    }



    /**

     * called from native code - Geofence Remove callback

     */

    private void reportGeofenceRemoveStatus(int geofenceId, int status) {

        mHandler.post(() -> {

            if (mGeofenceHardwareImpl == null) {

                mGeofenceHardwareImpl = GeofenceHardwareImpl.getInstance(mContext);

            }

            mGeofenceHardwareImpl.reportGeofenceRemoveStatus(geofenceId, getGeofenceStatus(status));

        });

    }



    /**

     * called from native code - Geofence Pause callback

     */

    private void reportGeofencePauseStatus(int geofenceId, int status) {

        mHandler.post(() -> {

            if (mGeofenceHardwareImpl == null) {

                mGeofenceHardwareImpl = GeofenceHardwareImpl.getInstance(mContext);

            }

            mGeofenceHardwareImpl.reportGeofencePauseStatus(geofenceId, getGeofenceStatus(status));

        });

    }



    /**

     * called from native code - Geofence Resume callback

     */

    private void reportGeofenceResumeStatus(int geofenceId, int status) {

        mHandler.post(() -> {

            if (mGeofenceHardwareImpl == null) {

                mGeofenceHardwareImpl = GeofenceHardwareImpl.getInstance(mContext);

            }

            mGeofenceHardwareImpl.reportGeofenceResumeStatus(geofenceId, getGeofenceStatus(status));

        });

    }



    //=============================================================

services/robotests/src/com/android/server/location/GnssGeofenceProviderTest.java

+1 −2
Original line number Diff line number Diff line
@@ -7,7 +7,6 @@ import static org.mockito.Mockito.times; 
import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.os.Looper;

import android.os.RemoteException;

import android.platform.test.annotations.Presubmit;
@@ -44,7 +43,7 @@ public class GnssGeofenceProviderTest { 
        when(mMockNative.pauseGeofence(anyInt())).thenReturn(true);

        when(mMockNative.removeGeofence(anyInt())).thenReturn(true);

        when(mMockNative.resumeGeofence(anyInt(), anyInt())).thenReturn(true);

        mTestProvider = new GnssGeofenceProvider(Looper.myLooper(), mMockNative);

        mTestProvider = new GnssGeofenceProvider(mMockNative);

        mTestProvider.addCircularHardwareGeofence(GEOFENCE_ID, LATITUDE,

                LONGITUDE, RADIUS, LAST_TRANSITION, MONITOR_TRANSITIONS,

                NOTIFICATION_RESPONSIVENESS,