diff options
author | Veena Arvind <aveena@google.com> | 2023-03-14 01:21:52 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2023-03-14 01:21:52 +0000 |
commit | 13b99b754924f76464d43d974879b97588a90259 (patch) | |
tree | 4fad2c6734e764deefc86116b6c2f1c2a84f155a | |
parent | 5f43355071aee112f269ec6b101bda22a5502c97 (diff) | |
parent | f9f0a8757bb88d0d4c08567df5eb0c3dbace7ddb (diff) | |
download | base-13b99b754924f76464d43d974879b97588a90259.tar.gz |
Merge "Fix deadlock in RebootEscrowManager"
3 files changed, 68 insertions, 41 deletions
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 78cffa6f4f79..1d9fed767991 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/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, - true /*allowIo*/); - handlerThread.start(); - return handlerThread; + if (mHandlerThread == null) { + mHandlerThread = new ServiceThread(TAG, + Process.THREAD_PRIORITY_BACKGROUND, + 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) { diff --git a/services/core/java/com/android/server/locksettings/RebootEscrowManager.java b/services/core/java/com/android/server/locksettings/RebootEscrowManager.java index 9b42cfca2e68..e1cd2c585146 100644 --- a/services/core/java/com/android/server/locksettings/RebootEscrowManager.java +++ b/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) { - this(new Injector(context, storage), callbacks, storage); + RebootEscrowManager(Context context, Callbacks callbacks, LockSettingsStorage 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; } diff --git a/services/tests/servicestests/src/com/android/server/locksettings/RebootEscrowManagerTests.java b/services/tests/servicestests/src/com/android/server/locksettings/RebootEscrowManagerTests.java index ce6bd6ccbe51..bb12d699a15c 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/RebootEscrowManagerTests.java +++ b/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); - verify(mockListener).onPreparedForReboot(eq(true)); + callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID); 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, - FAKE_AUTH_TOKEN); + callToRebootEscrowIfNeededAndWait(SECURE_SECONDARY_USER_ID); 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()); |