Merge "Fix deadlock in RebootEscrowManager" am: 13b99b7549

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2469529

Change-Id: Idd320aad5a0b62df21e01432514b9bf1d076a6fb
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

services/core/java/com/android/server/locksettings/LockSettingsService.java

@@ -416,6 +416,8 @@ public class LockSettingsService extends ILockSettings.Stub {
     static class Injector {
 
         protected Context mContext;
+        private ServiceThread mHandlerThread;
+        private Handler mHandler;
 
         public Injector(Context context) {
             mContext = context;
@@ -426,14 +428,20 @@ public class LockSettingsService extends ILockSettings.Stub {
         }
 
         public ServiceThread getServiceThread() {
-            ServiceThread handlerThread = new ServiceThread(TAG, Process.THREAD_PRIORITY_BACKGROUND,
+            if (mHandlerThread == null) {
-                    true /*allowIo*/);
+                mHandlerThread = new ServiceThread(TAG,
-            handlerThread.start();
+                        Process.THREAD_PRIORITY_BACKGROUND,
-            return handlerThread;
+                        true /*allowIo*/);
+                mHandlerThread.start();
+            }
+            return mHandlerThread;
         }
 
         public Handler getHandler(ServiceThread handlerThread) {
-            return new Handler(handlerThread.getLooper());
+            if (mHandler == null) {
+                mHandler = new Handler(handlerThread.getLooper());
+            }
+            return mHandler;
         }
 
         public LockSettingsStorage getStorage() {
@@ -514,7 +522,8 @@ public class LockSettingsService extends ILockSettings.Stub {
 
         public RebootEscrowManager getRebootEscrowManager(RebootEscrowManager.Callbacks callbacks,
                 LockSettingsStorage storage) {
-            return new RebootEscrowManager(mContext, callbacks, storage);
+            return new RebootEscrowManager(mContext, callbacks, storage,
+                    getHandler(getServiceThread()));
         }
 
         public boolean hasEnrolledBiometrics(int userId) {

services/core/java/com/android/server/locksettings/RebootEscrowManager.java

@@ -205,6 +205,8 @@ class RebootEscrowManager {
 
     private final RebootEscrowKeyStoreManager mKeyStoreManager;
 
+    private final Handler mHandler;
+
     PowerManager.WakeLock mWakeLock;
 
     private ConnectivityManager.NetworkCallback mNetworkCallback;
@@ -399,19 +401,21 @@ class RebootEscrowManager {
         }
     }
 
-    RebootEscrowManager(Context context, Callbacks callbacks, LockSettingsStorage storage) {
+    RebootEscrowManager(Context context, Callbacks callbacks, LockSettingsStorage storage,
-        this(new Injector(context, storage), callbacks, storage);
+            Handler handler) {
+        this(new Injector(context, storage), callbacks, storage, handler);
     }
 
     @VisibleForTesting
     RebootEscrowManager(Injector injector, Callbacks callbacks,
-            LockSettingsStorage storage) {
+            LockSettingsStorage storage, Handler handler) {
         mInjector = injector;
         mCallbacks = callbacks;
         mStorage = storage;
         mUserManager = injector.getUserManager();
         mEventLog = injector.getEventLog();
         mKeyStoreManager = injector.getKeyStoreManager();
+        mHandler = handler;
     }
 
     /** Wrapper function to set error code serialized through handler, */
@@ -937,7 +941,7 @@ class RebootEscrowManager {
 
     private void setRebootEscrowReady(boolean ready) {
         if (mRebootEscrowReady != ready) {
-            mRebootEscrowListener.onPreparedForReboot(ready);
+            mHandler.post(() -> mRebootEscrowListener.onPreparedForReboot(ready));
         }
         mRebootEscrowReady = ready;
     }

services/tests/servicestests/src/com/android/server/locksettings/RebootEscrowManagerTests.java

@@ -74,6 +74,8 @@ import org.mockito.ArgumentCaptor;
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 
 import javax.crypto.SecretKey;
@@ -327,16 +329,30 @@ public class RebootEscrowManagerTests {
         mInjected = mock(MockableRebootEscrowInjected.class);
         mMockInjector = new MockInjector(mContext, mUserManager, mRebootEscrow,
                 mKeyStoreManager, mStorage, mInjected);
-        mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage);
         HandlerThread thread = new HandlerThread("RebootEscrowManagerTest");
         thread.start();
         mHandler = new Handler(thread.getLooper());
+        mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage, mHandler);
+
     }
 
     private void setServerBasedRebootEscrowProvider() throws Exception {
         mMockInjector = new MockInjector(mContext, mUserManager, mServiceConnection,
                 mKeyStoreManager, mStorage, mInjected);
-        mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage);
+        mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage, mHandler);
+    }
+
+    private void waitForHandler() throws InterruptedException {
+        // Wait for handler to complete processing.
+        CountDownLatch latch = new CountDownLatch(1);
+        mHandler.post(latch::countDown);
+        assertTrue(latch.await(5, TimeUnit.SECONDS));
+
+    }
+
+    private void callToRebootEscrowIfNeededAndWait(int userId) throws InterruptedException {
+        mService.callToRebootEscrowIfNeeded(userId, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        waitForHandler();
     }
 
     @Test
@@ -346,7 +362,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mRebootEscrow, never()).storeKey(any());
     }
@@ -358,8 +374,7 @@ public class RebootEscrowManagerTests {
         mService.setRebootEscrowListener(mockListener);
         mService.prepareRebootEscrow();
 
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
-        verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
         assertFalse(mStorage.hasRebootEscrowServerBlob());
     }
@@ -369,7 +384,7 @@ public class RebootEscrowManagerTests {
         RebootEscrowListener mockListener = mock(RebootEscrowListener.class);
         mService.setRebootEscrowListener(mockListener);
         mService.prepareRebootEscrow();
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
 
         clearInvocations(mRebootEscrow);
@@ -393,7 +408,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mRebootEscrow, never()).storeKey(any());
 
@@ -417,7 +432,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -438,7 +453,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mRebootEscrow, never()).storeKey(any());
 
@@ -456,10 +471,9 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
-        mService.callToRebootEscrowIfNeeded(SECURE_SECONDARY_USER_ID, FAKE_SP_VERSION,
+        callToRebootEscrowIfNeededAndWait(SECURE_SECONDARY_USER_ID);
-                FAKE_AUTH_TOKEN);
         verify(mRebootEscrow, never()).storeKey(any());
 
         assertTrue(mStorage.hasRebootEscrow(PRIMARY_USER_ID));
@@ -491,7 +505,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mRebootEscrow, never()).storeKey(any());
 
@@ -514,7 +528,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
 
         verify(mRebootEscrow, never()).storeKey(any());
@@ -557,7 +571,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -601,7 +615,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -646,7 +660,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -692,7 +706,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -741,7 +755,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -794,7 +808,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -849,7 +863,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -896,7 +910,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -952,7 +966,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -1011,7 +1025,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -1071,7 +1085,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -1127,7 +1141,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mServiceConnection);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
 
@@ -1179,7 +1193,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
 
         verify(mRebootEscrow, never()).storeKey(any());
@@ -1210,7 +1224,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
 
         verify(mRebootEscrow, never()).storeKey(any());
@@ -1238,7 +1252,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
 
         verify(mRebootEscrow, never()).storeKey(any());
@@ -1277,7 +1291,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
 
         verify(mRebootEscrow, never()).storeKey(any());
@@ -1312,7 +1326,7 @@ public class RebootEscrowManagerTests {
         mService.prepareRebootEscrow();
 
         clearInvocations(mRebootEscrow);
-        mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
+        callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
         verify(mockListener).onPreparedForReboot(eq(true));
         assertTrue(mStorage.hasRebootEscrow(PRIMARY_USER_ID));
         verify(mRebootEscrow, never()).storeKey(any());