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

Commit f6d80374 authored by Joe Bolinger's avatar Joe Bolinger
Browse files

Run ALS callbacks on the HAL service thread instead of main. 

This part of the code was never updated when most of the face/fingerprint methods were moved off the main thread. This may not have been responsible for the bug alone, but other contention for the SensorManager on main recently seems to have made it problematic.

Flag: EXEMPT bugfix
Fix: 409958969
Test: atest BiometricLoggerTest ALSProbeTest BiometricContextProviderTest
Change-Id: I442fb6936335e1a0912451de36ac2693d9145148
parent 6e14ced4

services/core/java/com/android/server/biometrics/log/ALSProbe.java

+9 −10
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ import android.hardware.SensorEvent; 
import android.hardware.SensorEventListener;

import android.hardware.SensorManager;

import android.os.Handler;

import android.os.Looper;

import android.util.Slog;



import com.android.internal.annotations.VisibleForTesting;
@@ -44,7 +43,7 @@ final class ALSProbe implements Probe { 
    @Nullable

    private final Sensor mLightSensor;

    @NonNull

    private final Handler mTimer;

    private final Handler mHandler;

    @DurationMillisLong

    private long mMaxSubscriptionTime = -1;
@@ -71,10 +70,10 @@ final class ALSProbe implements Probe { 
     * Create a probe with a 1-minute max sampling time.

     *

     * @param sensorManager Sensor manager

     * @param handler Handler to use for callbacks events from sensorManager

     */

    ALSProbe(@NonNull SensorManager sensorManager) {

        this(sensorManager, new Handler(Looper.getMainLooper()),

                TimeUnit.MINUTES.toMillis(1));

    ALSProbe(@NonNull SensorManager sensorManager, @NonNull Handler handler) {

        this(sensorManager, handler, TimeUnit.MINUTES.toMillis(1));

    }



    /**
@@ -86,7 +85,7 @@ final class ALSProbe implements Probe { 
     * avoid relying on this timeout to unsubscribe from the sensor when it is not needed.

     *

     * @param sensorManager Sensor manager

     * @param handler Timeout handler

     * @param handler Handler to use for callbacks events from sensorManager

     * @param maxTime The max amount of time to subscribe to events. If this time is exceeded

     *                {@link #disable()} will be called and no sampling will occur until {@link

     *                #enable()} is called again.
@@ -97,7 +96,7 @@ final class ALSProbe implements Probe { 
        mSensorManager = sensorManager;

        mLightSensor = sensorManager != null

                ? sensorManager.getDefaultSensor(Sensor.TYPE_LIGHT) : null;

        mTimer = handler;

        mHandler = handler;

        mMaxSubscriptionTime = maxTime;



        if (mSensorManager == null || mLightSensor == null) {
@@ -194,7 +193,7 @@ final class ALSProbe implements Probe { 
            mEnabled = true;

            mLastAmbientLux = -1;

            mSensorManager.registerListener(mLightSensorListener, mLightSensor,

                    SensorManager.SENSOR_DELAY_NORMAL);

                    SensorManager.SENSOR_DELAY_NORMAL, mHandler);

            Slog.v(TAG, "Enable ALS: " + mLightSensorListener.hashCode());

        }
@@ -215,9 +214,9 @@ final class ALSProbe implements Probe { 
    }



    private void resetTimerLocked(boolean start) {

        mTimer.removeCallbacksAndMessages(this /* token */);

        mHandler.removeCallbacksAndMessages(this /* token */);

        if (start && mMaxSubscriptionTime > 0) {

            mTimer.postDelayed(this::onTimeout, this /* token */, mMaxSubscriptionTime);

            mHandler.postDelayed(this::onTimeout, this /* token */, mMaxSubscriptionTime);

        }

    }

services/core/java/com/android/server/biometrics/log/BiometricContextProvider.java

+42 −17
Original line number Diff line number Diff line
@@ -27,9 +27,11 @@ import android.hardware.biometrics.AuthenticateOptions; 
import android.hardware.biometrics.IBiometricContextListener;

import android.hardware.biometrics.common.OperationContext;

import android.os.Handler;

import android.os.Looper;

import android.os.RemoteException;

import android.os.ServiceManager;

import android.os.ServiceManager.ServiceNotFoundException;

import android.util.Pair;

import android.util.Slog;

import android.view.Display;

import android.view.WindowManager;
@@ -38,6 +40,7 @@ import com.android.internal.annotations.VisibleForTesting; 
import com.android.internal.logging.InstanceId;

import com.android.internal.statusbar.ISessionListener;

import com.android.internal.statusbar.IStatusBarService;

import com.android.server.biometrics.BiometricHandlerProvider;

import com.android.server.biometrics.sensors.AuthSessionCoordinator;



import java.util.Map;
@@ -65,7 +68,8 @@ public final class BiometricContextProvider implements BiometricContext { 
                    sInstance = new BiometricContextProvider(context,

                            (WindowManager) context.getSystemService(Context.WINDOW_SERVICE),

                            IStatusBarService.Stub.asInterface(ServiceManager.getServiceOrThrow(

                                    Context.STATUS_BAR_SERVICE)), null /* handler */,

                                    Context.STATUS_BAR_SERVICE)),

                            BiometricHandlerProvider.getInstance().getBiometricCallbackHandler(),

                            new AuthSessionCoordinator());

                } catch (ServiceNotFoundException e) {

                    throw new IllegalStateException("Failed to find required service", e);
@@ -76,14 +80,13 @@ public final class BiometricContextProvider implements BiometricContext { 
    }



    @NonNull

    private final Map<OperationContextExt, Consumer<OperationContext>> mSubscribers =

            new ConcurrentHashMap<>();

    private final ContextSubscribers mSubscribers = new ContextSubscribers();



    @Nullable

    private final Map<Integer, BiometricContextSessionInfo> mSession = new ConcurrentHashMap<>();

    private final AuthSessionCoordinator mAuthSessionCoordinator;

    private final WindowManager mWindowManager;

    @Nullable private final Handler mHandler;

    @NonNull private final Handler mHandler;

    private int mDockState = Intent.EXTRA_DOCK_STATE_UNDOCKED;

    private int mFoldState = IBiometricContextListener.FoldState.UNKNOWN;

    private int mDisplayState = AuthenticateOptions.DISPLAY_STATE_UNKNOWN;
@@ -101,7 +104,7 @@ public final class BiometricContextProvider implements BiometricContext { 
    @VisibleForTesting

    public BiometricContextProvider(@NonNull Context context,

            @NonNull WindowManager windowManager,

            @NonNull IStatusBarService service, @Nullable Handler handler,

            @NonNull IStatusBarService service, @NonNull Handler handler,

            @NonNull AuthSessionCoordinator authSessionCoordinator) {

        mWindowManager = windowManager;

        mAuthSessionCoordinator = authSessionCoordinator;
@@ -232,7 +235,9 @@ public final class BiometricContextProvider implements BiometricContext { 
            @NonNull Consumer<OperationContext> startHalConsumer,

            @NonNull Consumer<OperationContext> updateContextConsumer,

            @Nullable AuthenticateOptions options) {

        mSubscribers.put(updateContext(context, context.isCrypto()), updateContextConsumer);

        final Handler handler = new Handler(Looper.myLooper());

        mSubscribers.add(updateContext(context, context.isCrypto()),

                updateContextConsumer, handler);

        if (options != null) {

            startHalConsumer.accept(context.toAidlContext(options));

        } else {
@@ -251,17 +256,7 @@ public final class BiometricContextProvider implements BiometricContext { 
    }



    private void notifyChanged() {

        if (mHandler != null) {

            mHandler.post(this::notifySubscribers);

        } else {

            notifySubscribers();

        }

    }



    private void notifySubscribers() {

        mSubscribers.forEach((context, consumer) -> {

            consumer.accept(context.update(this, context.isCrypto()).toAidlContext());

        });

        mHandler.post(() -> mSubscribers.notifySubscribers(this));

    }



    @Override
@@ -276,4 +271,34 @@ public final class BiometricContextProvider implements BiometricContext { 
                + "rotation: " + getCurrentRotation() + ", "

                + "foldState: " + mFoldState + "]";

    }



    /** Helper class for running the subscribers on the HAL threads. */

    private static final class ContextSubscribers {

        private final Map<OperationContextExt, Pair<Consumer<OperationContext>, Handler>> mSubscribers =

                new ConcurrentHashMap<>();



        public void add(@NonNull OperationContextExt context,

                @NonNull Consumer<OperationContext> callback,

                @NonNull Handler handler) {

            mSubscribers.put(context, new Pair<>(callback, handler));

        }



        public void remove(@NonNull OperationContextExt context) {

            mSubscribers.remove(context);

        }



        public void notifySubscribers(@NonNull BiometricContext biometricContext) {

            mSubscribers.forEach((context, consumer) -> {

                final Handler handler = consumer.second;

                final Runnable callback = () -> consumer.first.accept(

                        context.update(biometricContext, context.isCrypto()).toAidlContext());



                if (handler.getLooper().isCurrentThread()) {

                    callback.run();

                } else {

                    handler.post(callback);

                }

            });

        }

    }

}

services/core/java/com/android/server/biometrics/log/BiometricLogger.java

+20 −9
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.hardware.SensorManager; 
import android.hardware.biometrics.BiometricConstants;

import android.hardware.biometrics.BiometricsProtoEnums;

import android.hardware.fingerprint.FingerprintManager;

import android.os.Handler;

import android.util.Slog;



import com.android.internal.annotations.VisibleForTesting;
@@ -42,6 +43,7 @@ public class BiometricLogger { 
    public static final String TAG = "BiometricLogger";

    public static final boolean DEBUG = false;



    private final Handler mHandler;

    private final int mStatsModality;

    private final int mStatsAction;

    private final int mStatsClient;
@@ -52,9 +54,15 @@ public class BiometricLogger { 
    private long mFirstAcquireTimeMs;

    private boolean mShouldLogMetrics = true;



    /** Get a new logger with all unknown fields (for operations that do not require logs). */

    public static BiometricLogger ofUnknown(@NonNull Context context) {

        return new BiometricLogger(context, BiometricsProtoEnums.MODALITY_UNKNOWN,

    /**

     * Get a new logger with all unknown fields (for operations that do not require logs).

     *

     * @param context system_server context

     * @param handler Handler for log events. This is not used for the public "log" methods, but

     *                for internal data collection from SensorManager, etc.

     */

    public static BiometricLogger ofUnknown(@NonNull Context context, @NonNull Handler handler) {

        return new BiometricLogger(context, handler, BiometricsProtoEnums.MODALITY_UNKNOWN,

                BiometricsProtoEnums.ACTION_UNKNOWN, BiometricsProtoEnums.CLIENT_UNKNOWN,

                null /* AuthenticationStatsCollector */);

    }
@@ -66,36 +74,39 @@ public class BiometricLogger { 
     * {@link #swapAction(Context, int)}.

     *

     * @param context system_server context

     * @param handler Handler for log events. This is not used for the public "log" methods, but

     *                for internal data collection from SensorManager, etc.

     * @param statsModality One of {@link BiometricsProtoEnums} MODALITY_* constants.

     * @param statsAction One of {@link BiometricsProtoEnums} ACTION_* constants.

     * @param statsClient One of {@link BiometricsProtoEnums} CLIENT_* constants.

     */

    public BiometricLogger(

            @NonNull Context context, int statsModality, int statsAction, int statsClient,

    public BiometricLogger(@NonNull Context context, @NonNull Handler handler,

            int statsModality, int statsAction, int statsClient,

            @Nullable AuthenticationStatsCollector authenticationStatsCollector) {

        this(statsModality, statsAction, statsClient,

        this(handler, statsModality, statsAction, statsClient,

                BiometricFrameworkStatsLogger.getInstance(),

                authenticationStatsCollector,

                context.getSystemService(SensorManager.class));

    }



    @VisibleForTesting

    BiometricLogger(

    BiometricLogger(@NonNull Handler handler,

            int statsModality, int statsAction, int statsClient,

            BiometricFrameworkStatsLogger logSink,

            @Nullable AuthenticationStatsCollector statsCollector,

            SensorManager sensorManager) {

        mHandler = handler;

        mStatsModality = statsModality;

        mStatsAction = statsAction;

        mStatsClient = statsClient;

        mSink = logSink;

        mAuthenticationStatsCollector = statsCollector;

        mALSProbe = new ALSProbe(sensorManager);

        mALSProbe = new ALSProbe(sensorManager, handler);

    }



    /** Creates a new logger with the action replaced with the new action. */

    public BiometricLogger swapAction(@NonNull Context context, int statsAction) {

        return new BiometricLogger(context, mStatsModality, statsAction, mStatsClient,

        return new BiometricLogger(context, mHandler, mStatsModality, statsAction, mStatsClient,

                null /* AuthenticationStatsCollector */);

    }

services/core/java/com/android/server/biometrics/sensors/face/FaceService.java

+9 −5
Original line number Diff line number Diff line
@@ -753,12 +753,17 @@ public class FaceService extends SystemService { 
    }



    public FaceService(Context context) {

        this(context, null /* faceProviderFunction */, () -> IBiometricService.Stub.asInterface(

                ServiceManager.getService(Context.BIOMETRIC_SERVICE)), null /* faceProvider */,

        this(context,

                BiometricContext.getInstance(context),

                null /* faceProviderFunction */,

                () -> IBiometricService.Stub.asInterface(

                        ServiceManager.getService(Context.BIOMETRIC_SERVICE)),

                null /* faceProvider */,

                () -> getDeclaredInstances());

    }



    @VisibleForTesting FaceService(Context context,

            BiometricContext biometricContext,

            FaceProviderFunction faceProviderFunction,

            Supplier<IBiometricService> biometricServiceSupplier,

            Function<String, FaceProvider> faceProvider,
@@ -789,7 +794,7 @@ public class FaceService extends SystemService { 
                final SensorProps[] props = face.getSensorProps();

                return new FaceProvider(getContext(),

                        mBiometricStateCallback, mAuthenticationStateListeners, props, name,

                        mLockoutResetDispatcher, BiometricContext.getInstance(getContext()),

                        mLockoutResetDispatcher, biometricContext,

                        false /* resetLockoutRequiresChallenge */);

            } catch (RemoteException e) {

                Slog.e(TAG, "Remote exception in getSensorProps: " + fqName);
@@ -803,8 +808,7 @@ public class FaceService extends SystemService { 
                        getContext(), mBiometricStateCallback, mAuthenticationStateListeners,

                        filteredSensorProps.second,

                        filteredSensorProps.first, mLockoutResetDispatcher,

                        BiometricContext.getInstance(getContext()),

                        resetLockoutRequiresChallenge));

                        biometricContext, resetLockoutRequiresChallenge));

    }



    @Override

services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java

+5 −5
Original line number Diff line number Diff line
@@ -413,7 +413,7 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { 
        mHandler.post(() -> {

            final InvalidationRequesterClient<Face> client =

                    new InvalidationRequesterClient<>(mContext, userId, sensorId,

                            BiometricLogger.ofUnknown(mContext),

                            BiometricLogger.ofUnknown(mContext, mHandler),

                            mBiometricContext,

                            mFaceSensors.get(sensorId).getFaceUtilsInstance());

            scheduleForSensor(sensorId, client);
@@ -708,7 +708,7 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { 
                    mFaceSensors.get(sensorId).getLazySession(), token,

                    new ClientMonitorCallbackConverter(receiver), userId,

                    mContext.getOpPackageName(), sensorId,

                    BiometricLogger.ofUnknown(mContext), mBiometricContext,

                    BiometricLogger.ofUnknown(mContext, mHandler), mBiometricContext,

                    feature, enabled, hardwareAuthToken);

            scheduleForSensor(sensorId, client);

        });
@@ -727,8 +727,8 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { 
            }

            final FaceGetFeatureClient client = new FaceGetFeatureClient(mContext,

                    mFaceSensors.get(sensorId).getLazySession(), token, callback, userId,

                    mContext.getOpPackageName(), sensorId, BiometricLogger.ofUnknown(mContext),

                    mBiometricContext, feature);

                    mContext.getOpPackageName(), sensorId,

                    BiometricLogger.ofUnknown(mContext, mHandler), mBiometricContext, feature);

            scheduleForSensor(sensorId, client);

        });

    }
@@ -771,7 +771,7 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { 


    private BiometricLogger createLogger(int statsAction, int statsClient,

            AuthenticationStatsCollector authenticationStatsCollector) {

        return new BiometricLogger(mContext, BiometricsProtoEnums.MODALITY_FACE,

        return new BiometricLogger(mContext, mHandler, BiometricsProtoEnums.MODALITY_FACE,

                statsAction, statsClient, authenticationStatsCollector);

    }