From 907dcf8fb867b6463e1cb8328e196dbfb0eb60c1 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Wed, 20 Jan 2021 04:27:15 -0800 Subject: [PATCH] Fix granting default roles timeout. The RoleControllerManager takes a Handler for service connection callbacks, and that Handler has to be a thread other than the main thread inside system server because we need to block the main thread before necessary permissions are granted via role. However, since a recent change that exposed RoleControllerManager APIs on RoleManager instead to hide this implementation detail from API, we are creating the RoleControllerManager instance upon RoleManager creation and implicitly initializing it with the main thread Handler. This may result in the RoleControllerManager getting the main thread handler instead of the desired foreground thread handler if someone else in the system server created RoleManager first, and that would create a deadlock when we block the main thread. So revert to the original timing for RoleControllerManager creation by creating it lazily, so that it gets the correct handler as the only user of it inside system server is RoleManagerService, and other services who only need the RoleManager won't affect RoleControllerManager. Fixes: 177251364 Test: manual Change-Id: I649a23fa3b88fbd9120b3de408eee8a590d50a17 --- core/java/android/app/role/RoleManager.java | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/core/java/android/app/role/RoleManager.java b/core/java/android/app/role/RoleManager.java index 0fcf44d2dda63..3788257bcf171 100644 --- a/core/java/android/app/role/RoleManager.java +++ b/core/java/android/app/role/RoleManager.java @@ -174,8 +174,10 @@ public final class RoleManager { @NonNull private final Object mListenersLock = new Object(); - @NonNull - private final RoleControllerManager mRoleControllerManager; + @GuardedBy("mRoleControllerManagerLock") + @Nullable + private RoleControllerManager mRoleControllerManager; + private final Object mRoleControllerManagerLock = new Object(); /** * @hide @@ -184,7 +186,6 @@ public final class RoleManager { mContext = context; mService = IRoleManager.Stub.asInterface(ServiceManager.getServiceOrThrow( Context.ROLE_SERVICE)); - mRoleControllerManager = new RoleControllerManager(context); } /** @@ -693,7 +694,7 @@ public final class RoleManager { @TestApi public void isRoleVisible(@NonNull String roleName, @NonNull @CallbackExecutor Executor executor, @NonNull Consumer callback) { - mRoleControllerManager.isRoleVisible(roleName, executor, callback); + getRoleControllerManager().isRoleVisible(roleName, executor, callback); } /** @@ -714,10 +715,20 @@ public final class RoleManager { @TestApi public void isApplicationVisibleForRole(@NonNull String roleName, @NonNull String packageName, @NonNull @CallbackExecutor Executor executor, @NonNull Consumer callback) { - mRoleControllerManager.isApplicationVisibleForRole(roleName, packageName, executor, + getRoleControllerManager().isApplicationVisibleForRole(roleName, packageName, executor, callback); } + @NonNull + private RoleControllerManager getRoleControllerManager() { + synchronized (mRoleControllerManagerLock) { + if (mRoleControllerManager == null) { + mRoleControllerManager = new RoleControllerManager(mContext); + } + return mRoleControllerManager; + } + } + private static class OnRoleHoldersChangedListenerDelegate extends IOnRoleHoldersChangedListener.Stub {