diff options
2 files changed, 211 insertions, 22 deletions
diff --git a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java index 588e865112c2..ce2d3405ddc6 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java @@ -28,6 +28,7 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.util.Slog; +import com.android.internal.annotations.VisibleForTesting; import com.android.server.biometrics.sensors.fingerprint.GestureAvailabilityDispatcher; import java.io.PrintWriter; @@ -37,9 +38,9 @@ import java.text.SimpleDateFormat; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Date; +import java.util.Deque; import java.util.List; import java.util.Locale; -import java.util.Queue; /** * A scheduler for biometric HAL operations. Maintains a queue of {@link ClientMonitor} operations, @@ -53,7 +54,8 @@ public class BiometricScheduler { /** * Contains all the necessary information for a HAL operation. */ - private static final class Operation { + @VisibleForTesting + static final class Operation { /** * The operation is added to the list of pending operations and waiting for its turn. @@ -176,8 +178,8 @@ public class BiometricScheduler { @NonNull private final IBiometricService mBiometricService; @NonNull private final Handler mHandler = new Handler(Looper.getMainLooper()); @NonNull private final InternalCallback mInternalCallback; - @NonNull private final Queue<Operation> mPendingOperations; - @Nullable private Operation mCurrentOperation; + @VisibleForTesting @NonNull final Deque<Operation> mPendingOperations; + @VisibleForTesting @Nullable Operation mCurrentOperation; @NonNull private final ArrayDeque<CrashState> mCrashStates; // Internal callback, notified when an operation is complete. Notifies the requester @@ -226,6 +228,18 @@ public class BiometricScheduler { } } + @VisibleForTesting + BiometricScheduler(@NonNull String tag, + @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher, + @NonNull IBiometricService biometricService) { + mBiometricTag = tag; + mInternalCallback = new InternalCallback(); + mGestureAvailabilityDispatcher = gestureAvailabilityDispatcher; + mPendingOperations = new ArrayDeque<>(); + mBiometricService = biometricService; + mCrashStates = new ArrayDeque<>(); + } + /** * Creates a new scheduler. * @param tag for the specific instance of the scheduler. Should be unique. @@ -234,13 +248,8 @@ public class BiometricScheduler { */ public BiometricScheduler(@NonNull String tag, @Nullable GestureAvailabilityDispatcher gestureAvailabilityDispatcher) { - mBiometricTag = tag; - mInternalCallback = new InternalCallback(); - mGestureAvailabilityDispatcher = gestureAvailabilityDispatcher; - mPendingOperations = new ArrayDeque<>(); - mBiometricService = IBiometricService.Stub.asInterface( - ServiceManager.getService(Context.BIOMETRIC_SERVICE)); - mCrashStates = new ArrayDeque<>(); + this(tag, gestureAvailabilityDispatcher, IBiometricService.Stub.asInterface( + ServiceManager.getService(Context.BIOMETRIC_SERVICE))); } /** @@ -295,9 +304,50 @@ public class BiometricScheduler { // to arrive at the head of the queue, before pinging it to start. final boolean shouldStartNow = currentClient.getCookie() == 0; if (shouldStartNow) { - Slog.d(getTag(), "[Starting] " + mCurrentOperation); - currentClient.start(getInternalCallback()); - mCurrentOperation.state = Operation.STATE_STARTED; + if (mCurrentOperation.clientMonitor.getFreshDaemon() == null) { + // Note down current length of queue + final int pendingOperationsLength = mPendingOperations.size(); + final Operation lastOperation = mPendingOperations.peekLast(); + Slog.e(getTag(), "[Unable To Start] " + mCurrentOperation + + ". Last pending operation: " + lastOperation); + + // For current operations, 1) unableToStart, which notifies the caller-side, then + // 2) notify operation's callback, to notify applicable system service that the + // operation failed. + mCurrentOperation.clientMonitor.unableToStart(); + if (mCurrentOperation.mClientCallback != null) { + mCurrentOperation.mClientCallback + .onClientFinished(mCurrentOperation.clientMonitor, false /* success */); + } + + // Then for each operation currently in the pending queue at the time of this + // failure, do the same as above. Otherwise, it's possible that something like + // setActiveUser fails, but then authenticate (for the wrong user) is invoked. + for (int i = 0; i < pendingOperationsLength; i++) { + final Operation operation = mPendingOperations.pollFirst(); + if (operation == null) { + Slog.e(getTag(), "Null operation, index: " + i + + ", expected length: " + pendingOperationsLength); + break; + } + operation.clientMonitor.unableToStart(); + if (operation.mClientCallback != null) { + operation.mClientCallback.onClientFinished(operation.clientMonitor, + false /* success */); + } + Slog.w(getTag(), "[Aborted Operation] " + operation); + } + + // It's possible that during cleanup a new set of operations came in. We can try to + // run these. A single request from the manager layer to the service layer may + // actually be multiple operations (i.e. updateActiveUser + authenticate). + mCurrentOperation = null; + startNextOperationIfIdle(); + } else { + Slog.d(getTag(), "[Starting] " + mCurrentOperation); + currentClient.start(getInternalCallback()); + mCurrentOperation.state = Operation.STATE_STARTED; + } } else { try { mBiometricService.onReadyForAuthentication(currentClient.getCookie()); @@ -338,9 +388,21 @@ public class BiometricScheduler { return; } - Slog.d(getTag(), "[Starting] Prepared client: " + mCurrentOperation); - mCurrentOperation.state = Operation.STATE_STARTED; - mCurrentOperation.clientMonitor.start(getInternalCallback()); + if (mCurrentOperation.clientMonitor.getFreshDaemon() == null) { + Slog.e(getTag(), "[Unable To Start] Prepared client: " + mCurrentOperation); + // This is BiometricPrompt trying to auth but something's wrong with the HAL. + mCurrentOperation.clientMonitor.unableToStart(); + if (mCurrentOperation.mClientCallback != null) { + mCurrentOperation.mClientCallback.onClientFinished(mCurrentOperation.clientMonitor, + false /* success */); + } + mCurrentOperation = null; + startNextOperationIfIdle(); + } else { + Slog.d(getTag(), "[Starting] Prepared client: " + mCurrentOperation); + mCurrentOperation.state = Operation.STATE_STARTED; + mCurrentOperation.clientMonitor.start(getInternalCallback()); + } } /** diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java index 36c55cce076b..c890c52f2181 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java @@ -16,11 +16,21 @@ package com.android.server.biometrics.sensors; +import static junit.framework.Assert.assertTrue; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + import android.content.Context; +import android.hardware.biometrics.IBiometricService; import android.platform.test.annotations.Presubmit; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; @@ -40,18 +50,21 @@ public class BiometricSchedulerTest { @Mock private Context mContext; @Mock - private ClientMonitor.LazyDaemon<Object> mLazyDaemon; + private IBiometricService mBiometricService; @Before public void setUp() { MockitoAnnotations.initMocks(this); - mScheduler = new BiometricScheduler(TAG, null /* gestureAvailabilityTracker */); + mScheduler = new BiometricScheduler(TAG, null /* gestureAvailabilityTracker */, + mBiometricService); } @Test public void testClientDuplicateFinish_ignoredBySchedulerAndDoesNotCrash() { - final ClientMonitor<Object> client1 = new TestClientMonitor(mContext, mLazyDaemon); - final ClientMonitor<Object> client2 = new TestClientMonitor(mContext, mLazyDaemon); + final ClientMonitor.LazyDaemon<Object> nonNullDaemon = () -> mock(Object.class); + + final ClientMonitor<Object> client1 = new TestClientMonitor(mContext, nonNullDaemon); + final ClientMonitor<Object> client2 = new TestClientMonitor(mContext, nonNullDaemon); mScheduler.scheduleClientMonitor(client1); mScheduler.scheduleClientMonitor(client2); @@ -59,7 +72,93 @@ public class BiometricSchedulerTest { client1.mCallback.onClientFinished(client1, true /* success */); } + @Test + public void testRemovesPendingOperations_whenNullHal_andNotBiometricPrompt() { + // Even if second client has a non-null daemon, it needs to be canceled. + Object daemon2 = mock(Object.class); + + final ClientMonitor.LazyDaemon<Object> lazyDaemon1 = () -> null; + final ClientMonitor.LazyDaemon<Object> lazyDaemon2 = () -> daemon2; + + final TestClientMonitor client1 = new TestClientMonitor(mContext, lazyDaemon1); + final TestClientMonitor client2 = new TestClientMonitor(mContext, lazyDaemon2); + + final ClientMonitor.Callback callback1 = mock(ClientMonitor.Callback.class); + final ClientMonitor.Callback callback2 = mock(ClientMonitor.Callback.class); + + // Pretend the scheduler is busy so the first operation doesn't start right away. We want + // to pretend like there are two operations in the queue before kicking things off + mScheduler.mCurrentOperation = new BiometricScheduler.Operation( + mock(ClientMonitor.class), mock(ClientMonitor.Callback.class)); + + mScheduler.scheduleClientMonitor(client1, callback1); + assertEquals(1, mScheduler.mPendingOperations.size()); + // client1 is pending. Allow the scheduler to start once second client is added. + mScheduler.mCurrentOperation = null; + mScheduler.scheduleClientMonitor(client2, callback2); + waitForIdle(); + + assertTrue(client1.wasUnableToStart()); + verify(callback1).onClientFinished(eq(client1), eq(false) /* success */); + verify(callback1, never()).onClientStarted(any()); + + assertTrue(client2.wasUnableToStart()); + verify(callback2).onClientFinished(eq(client2), eq(false) /* success */); + verify(callback2, never()).onClientStarted(any()); + + assertTrue(mScheduler.mPendingOperations.isEmpty()); + } + + @Test + public void testRemovesOnlyBiometricPromptOperation_whenNullHal() { + // Second non-BiometricPrompt client has a valid daemon + final Object daemon2 = mock(Object.class); + + final ClientMonitor.LazyDaemon<Object> lazyDaemon1 = () -> null; + final ClientMonitor.LazyDaemon<Object> lazyDaemon2 = () -> daemon2; + + final TestClientMonitor client1 = + new TestBiometricPromptClientMonitor(mContext, lazyDaemon1); + final TestClientMonitor client2 = new TestClientMonitor(mContext, lazyDaemon2); + + final ClientMonitor.Callback callback1 = mock(ClientMonitor.Callback.class); + final ClientMonitor.Callback callback2 = mock(ClientMonitor.Callback.class); + + // Pretend the scheduler is busy so the first operation doesn't start right away. We want + // to pretend like there are two operations in the queue before kicking things off + mScheduler.mCurrentOperation = new BiometricScheduler.Operation( + mock(ClientMonitor.class), mock(ClientMonitor.Callback.class)); + + mScheduler.scheduleClientMonitor(client1, callback1); + assertEquals(1, mScheduler.mPendingOperations.size()); + // client1 is pending. Allow the scheduler to start once second client is added. + mScheduler.mCurrentOperation = null; + mScheduler.scheduleClientMonitor(client2, callback2); + waitForIdle(); + + // Simulate that the BiometricPrompt client's sensor is ready + mScheduler.startPreparedClient(client1.getCookie()); + + assertTrue(client1.wasUnableToStart()); + verify(callback1).onClientFinished(eq(client1), eq(false) /* success */); + verify(callback1, never()).onClientStarted(any()); + + // Client 2 was able to start + assertFalse(client2.wasUnableToStart()); + assertTrue(client2.hasStarted()); + verify(callback2).onClientStarted(eq(client2)); + } + + private static class TestBiometricPromptClientMonitor extends TestClientMonitor { + public TestBiometricPromptClientMonitor(@NonNull Context context, + @NonNull LazyDaemon<Object> lazyDaemon) { + super(context, lazyDaemon, 1 /* cookie */); + } + } + private static class TestClientMonitor extends ClientMonitor<Object> { + private boolean mUnableToStart; + private boolean mStarted; public TestClientMonitor(@NonNull Context context, @NonNull LazyDaemon<Object> lazyDaemon) { super(context, lazyDaemon, null /* token */, null /* listener */, 0 /* userId */, @@ -67,14 +166,42 @@ public class BiometricSchedulerTest { 0 /* statsAction */, 0 /* statsClient */); } + public TestClientMonitor(@NonNull Context context, @NonNull LazyDaemon<Object> lazyDaemon, + int cookie) { + super(context, lazyDaemon, null /* token */, null /* listener */, 0 /* userId */, + TAG, cookie, 0 /* sensorId */, 0 /* statsModality */, + 0 /* statsAction */, 0 /* statsClient */); + } + + @Override public void unableToStart() { + assertFalse(mUnableToStart); + mUnableToStart = true; + } + @Override + public void start(@NonNull Callback callback) { + super.start(callback); + assertFalse(mStarted); + mStarted = true; } @Override protected void startHalOperation() { } + + public boolean wasUnableToStart() { + return mUnableToStart; + } + + public boolean hasStarted() { + return mStarted; + } + } + + private static void waitForIdle() { + InstrumentationRegistry.getInstrumentation().waitForIdleSync(); } } |