From a256d31e7234df92be6c55355674297b7a134ec8 Mon Sep 17 00:00:00 2001 From: Jeongsik Mun Date: Mon, 3 May 2021 15:56:16 +0900 Subject: [PATCH] Introduce a new thread in system_server for the PermisisonController FgThread could be blocked by lock contention for a while especially during a new user creation. During this time, PermisisonPolicyService synchronizes permissions and app ops in FgThread. Meanwhile, PermissionController does async things for initialization. Both use PackageManager a lot to get package info, which could result in lock contention and blocking FgThread for a very short duration for each call to PackageManager. It could delay the user creation or switching. This CL adds a new thread for PermisisonPolicyService and PermisisonController to avoid blocking FgThread by lock contention. Bug: 165030092 Test: cts-tradefed run cts -m CtsDevicePolicyManagerTestCases -t \ com.android.cts.devicepolicy.MixedManagedProfileOwnerTest#testPermissionGrant Change-Id: I0b92f681b6fffee47d41473012a6187cd261c32a --- .../com/android/server/PermissionThread.java | 84 +++++++++++++++++++ .../OneTimePermissionUserManager.java | 4 +- .../permission/PermissionManagerService.java | 8 +- .../policy/PermissionPolicyService.java | 17 ++-- 4 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 services/core/java/com/android/server/PermissionThread.java diff --git a/services/core/java/com/android/server/PermissionThread.java b/services/core/java/com/android/server/PermissionThread.java new file mode 100644 index 0000000000000..cf444fac4d035 --- /dev/null +++ b/services/core/java/com/android/server/PermissionThread.java @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server; + +import android.os.Handler; +import android.os.HandlerExecutor; +import android.os.Looper; +import android.os.Trace; + +import com.android.internal.annotations.GuardedBy; +import com.android.server.ServiceThread; + +import java.util.concurrent.Executor; + +/** + * Shared singleton thread for the system. This is a thread for handling + * calls to and from the PermissionController and handling synchronization + * between permissions and appops states. + */ +public final class PermissionThread extends ServiceThread { + private static final long SLOW_DISPATCH_THRESHOLD_MS = 100; + private static final long SLOW_DELIVERY_THRESHOLD_MS = 200; + + private static final Object sLock = new Object(); + + @GuardedBy("sLock") + private static PermissionThread sInstance; + private static Handler sHandler; + private static HandlerExecutor sHandlerExecutor; + + private PermissionThread() { + super("android.perm", android.os.Process.THREAD_PRIORITY_DEFAULT, /* allowIo= */ true); + } + + private static void ensureThreadLocked() { + if (sInstance != null) { + return; + } + + sInstance = new PermissionThread(); + sInstance.start(); + final Looper looper = sInstance.getLooper(); + looper.setTraceTag(Trace.TRACE_TAG_SYSTEM_SERVER); + looper.setSlowLogThresholdMs( + SLOW_DISPATCH_THRESHOLD_MS, SLOW_DELIVERY_THRESHOLD_MS); + sHandler = new Handler(sInstance.getLooper()); + sHandlerExecutor = new HandlerExecutor(sHandler); + } + + public static PermissionThread get() { + synchronized (sLock) { + ensureThreadLocked(); + return sInstance; + } + } + + public static Handler getHandler() { + synchronized (sLock) { + ensureThreadLocked(); + return sHandler; + } + } + + public static Executor getExecutor() { + synchronized (sLock) { + ensureThreadLocked(); + return sHandlerExecutor; + } + } +} \ No newline at end of file diff --git a/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java b/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java index 4bbe3733719e5..dd50b0c099919 100644 --- a/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java +++ b/services/core/java/com/android/server/pm/permission/OneTimePermissionUserManager.java @@ -33,6 +33,7 @@ import android.util.Log; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; +import com.android.server.PermissionThread; /** * Class that handles one-time permissions for a user @@ -79,7 +80,8 @@ public class OneTimePermissionUserManager { mContext = context; mActivityManager = context.getSystemService(ActivityManager.class); mAlarmManager = context.getSystemService(AlarmManager.class); - mPermissionControllerManager = context.getSystemService(PermissionControllerManager.class); + mPermissionControllerManager = new PermissionControllerManager( + mContext, PermissionThread.getHandler()); mHandler = context.getMainThreadHandler(); } diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index fbe14ce5b9657..b619a9d914717 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -145,6 +145,7 @@ import com.android.internal.util.function.TriFunction; import com.android.internal.util.function.pooled.PooledLambda; import com.android.server.FgThread; import com.android.server.LocalServices; +import com.android.server.PermissionThread; import com.android.server.ServiceThread; import com.android.server.SystemConfig; import com.android.server.Watchdog; @@ -2056,7 +2057,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { private byte[] backupRuntimePermissions(@UserIdInt int userId) { CompletableFuture backup = new CompletableFuture<>(); mPermissionControllerManager.getRuntimePermissionBackup(UserHandle.of(userId), - mContext.getMainExecutor(), backup::complete); + PermissionThread.getExecutor(), backup::complete); try { return backup.get(BACKUP_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); @@ -2101,7 +2102,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } mPermissionControllerManager.applyStagedRuntimePermissionBackup(packageName, - UserHandle.of(userId), mContext.getMainExecutor(), (hasMoreBackup) -> { + UserHandle.of(userId), PermissionThread.getExecutor(), (hasMoreBackup) -> { if (hasMoreBackup) { return; } @@ -4548,7 +4549,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - mPermissionControllerManager = mContext.getSystemService(PermissionControllerManager.class); + mPermissionControllerManager = new PermissionControllerManager( + mContext, PermissionThread.getHandler()); mPermissionPolicyInternal = LocalServices.getService(PermissionPolicyInternal.class); } diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java index ad435146f5795..2709d4fe13009 100644 --- a/services/core/java/com/android/server/policy/PermissionPolicyService.java +++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java @@ -64,8 +64,8 @@ import com.android.internal.app.IAppOpsService; import com.android.internal.infra.AndroidFuture; import com.android.internal.util.IntPair; import com.android.internal.util.function.pooled.PooledLambda; -import com.android.server.FgThread; import com.android.server.LocalServices; +import com.android.server.PermissionThread; import com.android.server.SystemService; import com.android.server.pm.UserManagerInternal; import com.android.server.pm.parsing.pkg.AndroidPackage; @@ -279,7 +279,7 @@ public final class PermissionPolicyService extends SystemService { PermissionControllerManager manager = mPermControllerManagers.get(user); if (manager == null) { manager = new PermissionControllerManager( - getUserContext(getContext(), user), FgThread.getHandler()); + getUserContext(getContext(), user), PermissionThread.getHandler()); mPermControllerManagers.put(user, manager); } manager.updateUserSensitiveForApp(uid); @@ -287,8 +287,9 @@ public final class PermissionPolicyService extends SystemService { }, UserHandle.ALL, intentFilter, null, null); PermissionControllerManager manager = new PermissionControllerManager( - getUserContext(getContext(), Process.myUserHandle()), FgThread.getHandler()); - FgThread.getHandler().postDelayed(manager::updateUserSensitive, + getUserContext(getContext(), Process.myUserHandle()), + PermissionThread.getHandler()); + PermissionThread.getHandler().postDelayed(manager::updateUserSensitive, USER_SENSITIVE_UPDATE_DELAY_MS); } @@ -315,7 +316,7 @@ public final class PermissionPolicyService extends SystemService { if (isStarted(changedUserId)) { synchronized (mLock) { if (mIsPackageSyncsScheduled.add(new Pair<>(packageName, changedUserId))) { - FgThread.getHandler().sendMessage(PooledLambda.obtainMessage( + PermissionThread.getHandler().sendMessage(PooledLambda.obtainMessage( PermissionPolicyService ::synchronizePackagePermissionsAndAppOpsForUser, this, packageName, changedUserId)); @@ -421,9 +422,9 @@ public final class PermissionPolicyService extends SystemService { final PermissionControllerManager permissionControllerManager = new PermissionControllerManager( getUserContext(getContext(), UserHandle.of(userId)), - FgThread.getHandler()); + PermissionThread.getHandler()); permissionControllerManager.grantOrUpgradeDefaultRuntimePermissions( - FgThread.getExecutor(), successful -> { + PermissionThread.getExecutor(), successful -> { if (successful) { future.complete(null); } else { @@ -527,7 +528,7 @@ public final class PermissionPolicyService extends SystemService { synchronized (mLock) { if (!mIsUidSyncScheduled.get(uid)) { mIsUidSyncScheduled.put(uid, true); - FgThread.getHandler().sendMessage(PooledLambda.obtainMessage( + PermissionThread.getHandler().sendMessage(PooledLambda.obtainMessage( PermissionPolicyService::resetAppOpPermissionsIfNotRequestedForUid, this, uid)); }