diff --git a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java index 61b8ded60db72..7341e744dea3a 100644 --- a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java @@ -251,7 +251,7 @@ public abstract class AuthenticationClient extends AcquisitionClient "Successful background authentication!"); } - mAlreadyDone = true; + markAlreadyDone(); if (mTaskStackListener != null) { mActivityTaskManager.unregisterTaskStackListener(mTaskStackListener); @@ -327,7 +327,7 @@ public abstract class AuthenticationClient extends AcquisitionClient final @LockoutTracker.LockoutMode int lockoutMode = handleFailedAttempt(getTargetUserId()); if (lockoutMode != LockoutTracker.LOCKOUT_NONE) { - mAlreadyDone = true; + markAlreadyDone(); } final CoexCoordinator coordinator = CoexCoordinator.getInstance(); diff --git a/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java b/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java index 9764a167fbbff..b73e91173a432 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BaseClientMonitor.java +++ b/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() 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 361ec40f28779..a358bc2bad55e 100644 --- a/services/core/java/com/android/server/biometrics/sensors/BiometricScheduler.java +++ b/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; 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 e3e3900c47e08..d192697827f6b 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 @@ -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 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 { - - public BiometricPromptClientMonitor(@NonNull Context context, @NonNull IBinder token, - @NonNull LazyDaemon 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 { int mNumCancels = 0; + boolean mDestroyed = false; public TestAuthenticationClient(@NonNull Context context, @NonNull LazyDaemon 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; }