Destroy clients that do not start.

Fix: 207282536
Test: atest BiometricSchedulerTest
Change-Id: Ia7121236bbbbfe92ddd4641fac44ffbb1aaf2242

services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java

@@ -251,7 +251,7 @@ public abstract class AuthenticationClient<T> extends AcquisitionClient<T>
                         "Successful background authentication!");
             }
 
-            mAlreadyDone = true;
+            markAlreadyDone();
 
             if (mTaskStackListener != null) {
                 mActivityTaskManager.unregisterTaskStackListener(mTaskStackListener);
@@ -327,7 +327,7 @@ public abstract class AuthenticationClient<T> extends AcquisitionClient<T>
             final @LockoutTracker.LockoutMode int lockoutMode =
                     handleFailedAttempt(getTargetUserId());
             if (lockoutMode != LockoutTracker.LOCKOUT_NONE) {
-                mAlreadyDone = true;
+                markAlreadyDone();
             }
 
             final CoexCoordinator coordinator = CoexCoordinator.getInstance();

services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java

@@ -114,7 +114,7 @@ public abstract class BaseClientMonitor extends LoggableMonitor
     // Currently only used for authentication client. The cookie generated by BiometricService
     // is never 0.
     private final int mCookie;
-    boolean mAlreadyDone;
+    private boolean mAlreadyDone = false;
 
     // Use an empty callback by default since delayed operations can receive events
     // before they are started and cause NPE in subclasses that access this field directly.
@@ -202,11 +202,9 @@ public abstract class BaseClientMonitor extends LoggableMonitor
         return callback;
     }
 
-    public boolean isAlreadyDone() {
-        return mAlreadyDone;
-    }
-
-    public void destroy() {
+    /** Signals this operation has completed its lifecycle and should no longer be used. */
+    void destroy() {
+        mAlreadyDone = true;
         if (mToken != null) {
             try {
                 mToken.unlinkToDeath(this, 0);
@@ -218,6 +216,20 @@ public abstract class BaseClientMonitor extends LoggableMonitor
         }
     }
 
+    /**
+     * Call while the operation is still active, but nearly done, to prevent any action
+     * upon client death (only needed for authentication clients).
+     */
+    void markAlreadyDone() {
+        Slog.d(TAG, "marking operation as done: " + this);
+        mAlreadyDone = true;
+    }
+
+    /** If this operation has been marked as completely done (or cancelled). */
+    public boolean isAlreadyDone() {
+        return mAlreadyDone;
+    }
+
     @Override
     public void binderDied() {
         binderDiedInternal(true /* clearListener */);
@@ -225,10 +237,9 @@ public abstract class BaseClientMonitor extends LoggableMonitor
 
     // TODO(b/157790417): Move this to the scheduler
     void binderDiedInternal(boolean clearListener) {
-        Slog.e(TAG, "Binder died, owner: " + getOwnerString()
-                + ", operation: " + this.getClass().getName());
+        Slog.e(TAG, "Binder died, operation: " + this);
 
-        if (isAlreadyDone()) {
+        if (mAlreadyDone) {
             Slog.w(TAG, "Binder died but client is finished, ignoring");
             return;
         }
@@ -299,7 +310,7 @@ public abstract class BaseClientMonitor extends LoggableMonitor
     @Override
     public String toString() {
         return "{[" + mSequentialId + "] "
-                + this.getClass().getSimpleName()
+                + this.getClass().getName()
                 + ", proto=" + getProtoEnum()
                 + ", owner=" + getOwnerString()
                 + ", cookie=" + getCookie()

services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java

@@ -605,6 +605,9 @@ public class BiometricScheduler {
         if (operation.mState == Operation.STATE_WAITING_FOR_COOKIE) {
             Slog.w(getTag(), "Skipping cancellation for non-started operation: " + operation);
             // We can set it to null immediately, since the HAL was never notified to start.
+            if (mCurrentOperation != null) {
+                mCurrentOperation.mClientMonitor.destroy();
+            }
             mCurrentOperation = null;
             startNextOperationIfIdle();
             return;

services/tests/servicestests/src/com/android/server/biometrics/sensors/BiometricSchedulerTest.java

@@ -143,8 +143,8 @@ public class BiometricSchedulerTest {
 
         final ClientMonitorCallbackConverter listener1 = mock(ClientMonitorCallbackConverter.class);
 
-        final BiometricPromptClientMonitor client1 =
-                new BiometricPromptClientMonitor(mContext, mToken, lazyDaemon1, listener1);
+        final TestAuthenticationClient client1 =
+                new TestAuthenticationClient(mContext, lazyDaemon1, mToken, listener1);
         final TestClientMonitor client2 = new TestClientMonitor(mContext, mToken, lazyDaemon2);
 
         final BaseClientMonitor.Callback callback1 = mock(BaseClientMonitor.Callback.class);
@@ -180,8 +180,8 @@ public class BiometricSchedulerTest {
     @Test
     public void testCancelNotInvoked_whenOperationWaitingForCookie() {
         final HalClientMonitor.LazyDaemon<Object> lazyDaemon1 = () -> mock(Object.class);
-        final BiometricPromptClientMonitor client1 = new BiometricPromptClientMonitor(mContext,
-                mToken, lazyDaemon1, mock(ClientMonitorCallbackConverter.class));
+        final TestAuthenticationClient client1 = new TestAuthenticationClient(mContext,
+                lazyDaemon1, mToken, mock(ClientMonitorCallbackConverter.class));
         final BaseClientMonitor.Callback callback1 = mock(BaseClientMonitor.Callback.class);
 
         // Schedule a BiometricPrompt authentication request
@@ -195,6 +195,8 @@ public class BiometricSchedulerTest {
         // should go back to idle, since in this case the framework has not even requested the HAL
         // to authenticate yet.
         mScheduler.cancelAuthenticationOrDetection(mToken, 1 /* requestId */);
+        assertTrue(client1.isAlreadyDone());
+        assertTrue(client1.mDestroyed);
         assertNull(mScheduler.mCurrentOperation);
     }
 
@@ -316,6 +318,10 @@ public class BiometricSchedulerTest {
                 eq(BiometricConstants.BIOMETRIC_ERROR_CANCELED),
                 eq(0) /* vendorCode */);
         assertNull(mScheduler.getCurrentClient());
+        assertTrue(client1.isAlreadyDone());
+        assertTrue(client1.mDestroyed);
+        assertTrue(client2.isAlreadyDone());
+        assertTrue(client2.mDestroyed);
     }
 
     @Test
@@ -465,39 +471,9 @@ public class BiometricSchedulerTest {
         return BiometricSchedulerProto.parseFrom(mScheduler.dumpProtoState(clearSchedulerBuffer));
     }
 
-    private static class BiometricPromptClientMonitor extends AuthenticationClient<Object> {
-
-        public BiometricPromptClientMonitor(@NonNull Context context, @NonNull IBinder token,
-                @NonNull LazyDaemon<Object> lazyDaemon, ClientMonitorCallbackConverter listener) {
-            super(context, lazyDaemon, token, listener, 0 /* targetUserId */, 0 /* operationId */,
-                    false /* restricted */, TAG, 1 /* cookie */, false /* requireConfirmation */,
-                    TEST_SENSOR_ID, true /* isStrongBiometric */, 0 /* statsModality */,
-                    0 /* statsClient */, null /* taskStackListener */, mock(LockoutTracker.class),
-                    false /* isKeyguard */, true /* shouldVibrate */,
-                    false /* isKeyguardBypassEnabled */);
-        }
-
-        @Override
-        protected void stopHalOperation() {
-        }
-
-        @Override
-        protected void startHalOperation() {
-        }
-
-        @Override
-        protected void handleLifecycleAfterAuth(boolean authenticated) {
-
-        }
-
-        @Override
-        public boolean wasUserDetected() {
-            return false;
-        }
-    }
-
     private static class TestAuthenticationClient extends AuthenticationClient<Object> {
         int mNumCancels = 0;
+        boolean mDestroyed = false;
 
         public TestAuthenticationClient(@NonNull Context context,
                 @NonNull LazyDaemon<Object> lazyDaemon, @NonNull IBinder token,
@@ -530,6 +506,13 @@ public class BiometricSchedulerTest {
             return false;
         }
 
+        @Override
+        public void destroy() {
+            mDestroyed = true;
+            super.destroy();
+        }
+
+        @Override
         public void cancel() {
             mNumCancels++;
             super.cancel();
@@ -595,6 +578,7 @@ public class BiometricSchedulerTest {
 
         @Override
         public void destroy() {
+            super.destroy();
             mDestroyed = true;
         }