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

Commit 77f9710f authored by Joe Bolinger's avatar Joe Bolinger
Browse files

Always invoke start next operation from start & stop user clients. 

There's not an easy way to reproduce this, but it can happen if the HAL crashes while a start/stop user operation is pending.

Fix: 223070875
Test: atest UserAwareBiometricSchedulerTest
Change-Id: I8d1453efd4639ac100858c3c79df0d9ff904130f
parent 547a4d65

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

+6 −11
Original line number Diff line number Diff line
@@ -57,30 +57,25 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { 
    @Nullable private StopUserClient<?> mStopUserClient;



    private class ClientFinishedCallback implements ClientMonitorCallback {

        private final BaseClientMonitor mOwner;

        @NonNull private final BaseClientMonitor mOwner;



        ClientFinishedCallback(BaseClientMonitor owner) {

        ClientFinishedCallback(@NonNull BaseClientMonitor owner) {

            mOwner = owner;

        }



        @Override

        public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) {

            mHandler.post(() -> {

                if (mOwner != clientMonitor) {

                    Slog.e(getTag(), "[Wrong client finished], actual: "

                            + clientMonitor + ", expected: " + mOwner);

                    return;

                }



                Slog.d(getTag(), "[Client finished] " + clientMonitor + ", success: " + success);

                if (mCurrentOperation != null && mCurrentOperation.isFor(mOwner)) {

                    mCurrentOperation = null;

                    startNextOperationIfIdle();

                } else {

                    // can usually be ignored (hal died, etc.)

                    Slog.d(getTag(), "operation is already null or different (reset?): "

                    // can happen if the hal dies and is usually okay

                    // do not unset the current operation that may be newer

                    Slog.w(getTag(), "operation is already null or different (reset?): "

                            + mCurrentOperation);

                }

                startNextOperationIfIdle();

            });

        }

    }

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

+59 −23
Original line number Diff line number Diff line
@@ -18,9 +18,8 @@ package com.android.server.biometrics.sensors; 


import static android.testing.TestableLooper.RunWithLooper;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertSame;

import static com.google.common.truth.Truth.assertThat;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;
@@ -45,11 +44,15 @@ import com.android.server.biometrics.log.BiometricContext; 
import com.android.server.biometrics.log.BiometricLogger;



import org.junit.Before;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.Mock;

import org.mockito.MockitoAnnotations;

import org.mockito.junit.MockitoJUnit;

import org.mockito.junit.MockitoRule;



import java.util.ArrayList;

import java.util.List;

import java.util.function.Supplier;



@Presubmit
@@ -61,9 +64,12 @@ public class UserAwareBiometricSchedulerTest { 
    private static final String TAG = "UserAwareBiometricSchedulerTest";

    private static final int TEST_SENSOR_ID = 0;



    @Rule

    public final MockitoRule mockito = MockitoJUnit.rule();



    private Handler mHandler;

    private UserAwareBiometricScheduler mScheduler;

    private IBinder mToken = new Binder();

    private final IBinder mToken = new Binder();



    @Mock

    private Context mContext;
@@ -74,15 +80,14 @@ public class UserAwareBiometricSchedulerTest { 
    @Mock

    private BiometricContext mBiometricContext;



    private TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback();

    private TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback();

    private final TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback();

    private final TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback();

    private int mCurrentUserId = UserHandle.USER_NULL;

    private boolean mStartOperationsFinish = true;

    private int mStartUserClientCount = 0;



    @Before

    public void setUp() {

        MockitoAnnotations.initMocks(this);

        mHandler = new Handler(TestableLooper.get(this).getLooper());

        mScheduler = new UserAwareBiometricScheduler(TAG,

                mHandler,
@@ -121,8 +126,8 @@ public class UserAwareBiometricSchedulerTest { 
        mScheduler.scheduleClientMonitor(nextClient);

        waitForIdle();



        assertEquals(0, mUserStoppedCallback.numInvocations);

        assertEquals(1, mUserStartedCallback.numInvocations);

        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0);

        assertThat(mUserStartedCallback.mStartedUsers).containsExactly(0);

        verify(nextClient).start(any());

    }
@@ -142,9 +147,9 @@ public class UserAwareBiometricSchedulerTest { 
            waitForIdle();

        }



        assertEquals(0, mUserStoppedCallback.numInvocations);

        assertEquals(0, mUserStartedCallback.numInvocations);

        assertEquals(1, mStartUserClientCount);

        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0);

        assertThat(mUserStartedCallback.mStartedUsers).isEmpty();

        assertThat(mStartUserClientCount).isEqualTo(1);

        for (BaseClientMonitor client : nextClients) {

            verify(client, never()).start(any());

        }
@@ -163,13 +168,13 @@ public class UserAwareBiometricSchedulerTest { 
        final TestStartUserClient startUserClient =

                (TestStartUserClient) mScheduler.mCurrentOperation.getClientMonitor();

        mScheduler.reset();

        assertNull(mScheduler.mCurrentOperation);

        assertThat(mScheduler.mCurrentOperation).isNull();



        final BiometricSchedulerOperation fakeOperation = new BiometricSchedulerOperation(

                mock(BaseClientMonitor.class), new ClientMonitorCallback() {});

        mScheduler.mCurrentOperation = fakeOperation;

        startUserClient.mCallback.onClientFinished(startUserClient, true);

        assertSame(fakeOperation, mScheduler.mCurrentOperation);

        assertThat(fakeOperation).isSameInstanceAs(mScheduler.mCurrentOperation);

    }



    @Test
@@ -184,8 +189,8 @@ public class UserAwareBiometricSchedulerTest { 
        waitForIdle();



        verify(nextClient).start(any());

        assertEquals(0, mUserStoppedCallback.numInvocations);

        assertEquals(0, mUserStartedCallback.numInvocations);

        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0);

        assertThat(mUserStartedCallback.mStartedUsers).isEmpty();

    }



    @Test
@@ -199,36 +204,67 @@ public class UserAwareBiometricSchedulerTest { 
        mScheduler.scheduleClientMonitor(nextClient);



        waitForIdle();

        assertEquals(1, mUserStoppedCallback.numInvocations);

        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1);



        waitForIdle();

        assertEquals(1, mUserStartedCallback.numInvocations);

        assertThat(mUserStartedCallback.mStartedUsers).containsExactly(nextUserId);



        waitForIdle();

        verify(nextClient).start(any());

    }



    @Test

    public void testStartUser_alwaysStartsNextOperation() {

        BaseClientMonitor nextClient = mock(BaseClientMonitor.class);

        when(nextClient.getTargetUserId()).thenReturn(10);



        mScheduler.scheduleClientMonitor(nextClient);



        waitForIdle();

        verify(nextClient).start(any());



        // finish first operation

        mScheduler.getInternalCallback().onClientFinished(nextClient, true /* success */);

        waitForIdle();



        // schedule second operation but swap out the current operation

        // before it runs so that it's not current when it's completion callback runs

        nextClient = mock(BaseClientMonitor.class);

        when(nextClient.getTargetUserId()).thenReturn(11);

        mUserStartedCallback.mAfterStart = () -> mScheduler.mCurrentOperation = null;

        mScheduler.scheduleClientMonitor(nextClient);



        waitForIdle();

        verify(nextClient).start(any());

        assertThat(mUserStartedCallback.mStartedUsers).containsExactly(10, 11).inOrder();

        assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1);

    }



    private void waitForIdle() {

        TestableLooper.get(this).processAllMessages();

    }



    private class TestUserStoppedCallback implements StopUserClient.UserStoppedCallback {

        int numInvocations;

        int mNumInvocations;



        @Override

        public void onUserStopped() {

            numInvocations++;

            mNumInvocations++;

            mCurrentUserId = UserHandle.USER_NULL;

        }

    }



    private class TestUserStartedCallback implements StartUserClient.UserStartedCallback<Object> {

        int numInvocations;

        final List<Integer> mStartedUsers = new ArrayList<>();

        Runnable mAfterStart = null;



        @Override

        public void onUserStarted(int newUserId, Object newObject, int halInterfaceVersion) {

            numInvocations++;

            mStartedUsers.add(newUserId);

            mCurrentUserId = newUserId;

            if (mAfterStart != null) {

                mAfterStart.run();

            }

        }

    }