From 9b7603b58c81d0069dba9126e3a8d9827cae66c6 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Wed, 27 Jan 2021 16:07:49 +0800 Subject: [PATCH] Remove WM lock from the path of service operations The method WPC#areBackgroundFgsStartsAllowed may be called frequently from ActiveServices when checking if foreground service is allowed to start. Because it usually has no window related operation, there should not have lock contention from window manager. This only changes to use fine grained lock and volatile fields. The logic is the same as original. Bug: 177268056 Test: BackgroundActivityLaunchTest ActivityRecordTests Change-Id: Ia3ef5b436ff09ea0968776fa7c8ea86112e356d2 --- .../server/wm/ActivityTaskManagerService.java | 6 +- .../wm/BackgroundActivityStartCallback.java | 6 +- .../wm/BackgroundLaunchProcessController.java | 264 ++++++++++++++++++ .../server/wm/WindowProcessController.java | 193 ++----------- 4 files changed, 300 insertions(+), 169 deletions(-) create mode 100644 services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 56105737bb822..31a04282f15bd 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -486,13 +486,13 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { * Whether normal application switches are allowed; a call to {@link #stopAppSwitches() * disables this. */ - private boolean mAppSwitchesAllowed = true; + private volatile boolean mAppSwitchesAllowed = true; /** * Last stop app switches time, apps finished before this time cannot start background activity * even if they are in grace period. */ - private long mLastStopAppSwitchesTime; + private volatile long mLastStopAppSwitchesTime; IActivityController mController = null; boolean mControllerIsAMonkey = false; @@ -698,6 +698,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { int OOM_ADJUSTMENT = 1; int LRU_UPDATE = 2; int PROCESS_CHANGE = 3; + int START_SERVICE = 4; int caller() default NONE; } @@ -4808,6 +4809,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { } /** A uid is considered to be foreground if it has a visible non-toast window. */ + @HotPath(caller = HotPath.START_SERVICE) boolean hasActiveVisibleWindow(int uid) { if (mVisibleActivityProcessTracker.hasVisibleActivity(uid)) { return true; diff --git a/services/core/java/com/android/server/wm/BackgroundActivityStartCallback.java b/services/core/java/com/android/server/wm/BackgroundActivityStartCallback.java index 200f207fe50aa..eec648dae24a5 100644 --- a/services/core/java/com/android/server/wm/BackgroundActivityStartCallback.java +++ b/services/core/java/com/android/server/wm/BackgroundActivityStartCallback.java @@ -31,7 +31,8 @@ public interface BackgroundActivityStartCallback { * Note that if the start was allowed due to a mechanism other than tokens (eg. permission), * this won't be called. * - * This will be called holding the WM lock, don't do anything costly here. + * This will be called holding the WM and local lock, don't do anything costly or invoke AM/WM + * methods here directly. */ boolean isActivityStartAllowed(Collection tokens, int uid, String packageName); @@ -40,7 +41,8 @@ public interface BackgroundActivityStartCallback { * #ACTION_CLOSE_SYSTEM_DIALOGS}, presumably to start activities, based on the originating * tokens {@code tokens} currently associated with potential activity starts. * - * This will be called holding the AM and WM lock, don't do anything costly here. + * This will be called holding the AM and local lock, don't do anything costly or invoke AM/WM + * methods here directly. */ boolean canCloseSystemDialogs(Collection tokens, int uid); } diff --git a/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java b/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java new file mode 100644 index 0000000000000..ab1ed67d4cf94 --- /dev/null +++ b/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java @@ -0,0 +1,264 @@ +/* + * 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.wm; + +import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_ACTIVITY_STARTS; +import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_ATM; +import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_WITH_CLASS_NAME; +import static com.android.server.wm.ActivityTaskManagerService.ACTIVITY_BG_START_GRACE_PERIOD_MS; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.os.Binder; +import android.os.IBinder; +import android.os.SystemClock; +import android.util.ArrayMap; +import android.util.ArraySet; +import android.util.IntArray; +import android.util.Slog; + +import com.android.internal.annotations.GuardedBy; + +import java.io.PrintWriter; +import java.util.Arrays; +import java.util.function.IntPredicate; + +/** + * A per-process controller to decide whether the process can start activity or foreground service + * (especially from background). All methods of this class must be thread safe. The caller does not + * need to hold WM lock, e.g. lock contention of WM lock shouldn't happen when starting service. + */ +class BackgroundLaunchProcessController { + private static final String TAG = + TAG_WITH_CLASS_NAME ? "BackgroundLaunchProcessController" : TAG_ATM; + + /** It is {@link ActivityTaskManagerService#hasActiveVisibleWindow(int)}. */ + private final IntPredicate mUidHasActiveVisibleWindowPredicate; + + private final @Nullable BackgroundActivityStartCallback mBackgroundActivityStartCallback; + + /** + * A set of tokens that currently contribute to this process being temporarily allowed + * to start activities even if it's not in the foreground. The values of this map are optional + * (can be null) and are used to trace back the grant to the notification token mechanism. + */ + @GuardedBy("this") + private @Nullable ArrayMap mBackgroundActivityStartTokens; + + /** Set of UIDs of clients currently bound to this process. */ + @GuardedBy("this") + private @Nullable IntArray mBoundClientUids; + + BackgroundLaunchProcessController(@NonNull IntPredicate uidHasActiveVisibleWindowPredicate, + @Nullable BackgroundActivityStartCallback callback) { + mUidHasActiveVisibleWindowPredicate = uidHasActiveVisibleWindowPredicate; + mBackgroundActivityStartCallback = callback; + } + + boolean areBackgroundActivityStartsAllowed(int pid, int uid, String packageName, + boolean appSwitchAllowed, boolean isCheckingForFgsStart, boolean hasVisibleActivities, + boolean hasBackgroundActivityStartPrivileges, long lastStopAppSwitchesTime, + long lastActivityLaunchTime, long lastActivityFinishTime) { + // If app switching is not allowed, we ignore all the start activity grace period + // exception so apps cannot start itself in onPause() after pressing home button. + if (appSwitchAllowed) { + // Allow if any activity in the caller has either started or finished very recently, and + // it must be started or finished after last stop app switches time. + final long now = SystemClock.uptimeMillis(); + if (now - lastActivityLaunchTime < ACTIVITY_BG_START_GRACE_PERIOD_MS + || now - lastActivityFinishTime < ACTIVITY_BG_START_GRACE_PERIOD_MS) { + // If activity is started and finished before stop app switch time, we should not + // let app to be able to start background activity even it's in grace period. + if (lastActivityLaunchTime > lastStopAppSwitchesTime + || lastActivityFinishTime > lastStopAppSwitchesTime) { + if (DEBUG_ACTIVITY_STARTS) { + Slog.d(TAG, "[Process(" + pid + + ")] Activity start allowed: within " + + ACTIVITY_BG_START_GRACE_PERIOD_MS + "ms grace period"); + } + return true; + } + if (DEBUG_ACTIVITY_STARTS) { + Slog.d(TAG, "[Process(" + pid + ")] Activity start within " + + ACTIVITY_BG_START_GRACE_PERIOD_MS + + "ms grace period but also within stop app switch window"); + } + + } + } + // Allow if the proc is instrumenting with background activity starts privs. + if (hasBackgroundActivityStartPrivileges) { + if (DEBUG_ACTIVITY_STARTS) { + Slog.d(TAG, "[Process(" + pid + + ")] Activity start allowed: process instrumenting with background " + + "activity starts privileges"); + } + return true; + } + // Allow if the caller has an activity in any foreground task. + if (appSwitchAllowed && hasVisibleActivities) { + if (DEBUG_ACTIVITY_STARTS) { + Slog.d(TAG, "[Process(" + pid + + ")] Activity start allowed: process has activity in foreground task"); + } + return true; + } + // Allow if the caller is bound by a UID that's currently foreground. + if (isBoundByForegroundUid()) { + if (DEBUG_ACTIVITY_STARTS) { + Slog.d(TAG, "[Process(" + pid + + ")] Activity start allowed: process bound by foreground uid"); + } + return true; + } + // Allow if the flag was explicitly set. + if (isBackgroundStartAllowedByToken(uid, packageName, isCheckingForFgsStart)) { + if (DEBUG_ACTIVITY_STARTS) { + Slog.d(TAG, "[Process(" + pid + + ")] Activity start allowed: process allowed by token"); + } + return true; + } + return false; + } + + /** + * If there are no tokens, we don't allow *by token*. If there are tokens and + * isCheckingForFgsStart is false, we ask the callback if the start is allowed for these tokens, + * otherwise if there is no callback we allow. + */ + private boolean isBackgroundStartAllowedByToken(int uid, String packageName, + boolean isCheckingForFgsStart) { + synchronized (this) { + if (mBackgroundActivityStartTokens == null + || mBackgroundActivityStartTokens.isEmpty()) { + return false; + } + if (isCheckingForFgsStart) { + // BG-FGS-start only checks if there is a token. + return true; + } + + if (mBackgroundActivityStartCallback == null) { + // We have tokens but no callback to decide => allow. + return true; + } + // The callback will decide. + return mBackgroundActivityStartCallback.isActivityStartAllowed( + mBackgroundActivityStartTokens.values(), uid, packageName); + } + } + + private boolean isBoundByForegroundUid() { + synchronized (this) { + if (mBoundClientUids != null) { + for (int i = mBoundClientUids.size() - 1; i >= 0; i--) { + if (mUidHasActiveVisibleWindowPredicate.test(mBoundClientUids.get(i))) { + return true; + } + } + } + } + return false; + } + + void setBoundClientUids(ArraySet boundClientUids) { + synchronized (this) { + if (boundClientUids == null || boundClientUids.isEmpty()) { + mBoundClientUids = null; + return; + } + if (mBoundClientUids == null) { + mBoundClientUids = new IntArray(); + } else { + mBoundClientUids.clear(); + } + for (int i = boundClientUids.size() - 1; i >= 0; i--) { + mBoundClientUids.add(boundClientUids.valueAt(i)); + } + } + } + + /** + * Allows background activity starts using token {@code entity}. Optionally, you can provide + * {@code originatingToken} if you have one such originating token, this is useful for tracing + * back the grant in the case of the notification token. + * + * If {@code entity} is already added, this method will update its {@code originatingToken}. + */ + void addOrUpdateAllowBackgroundActivityStartsToken(Binder entity, + @Nullable IBinder originatingToken) { + synchronized (this) { + if (mBackgroundActivityStartTokens == null) { + mBackgroundActivityStartTokens = new ArrayMap<>(); + } + mBackgroundActivityStartTokens.put(entity, originatingToken); + } + } + + /** + * Removes token {@code entity} that allowed background activity starts added via {@link + * #addOrUpdateAllowBackgroundActivityStartsToken(Binder, IBinder)}. + */ + void removeAllowBackgroundActivityStartsToken(Binder entity) { + synchronized (this) { + if (mBackgroundActivityStartTokens != null) { + mBackgroundActivityStartTokens.remove(entity); + } + } + } + + /** + * Returns whether this process is allowed to close system dialogs via a background activity + * start token that allows the close system dialogs operation (eg. notification). + */ + boolean canCloseSystemDialogsByToken(int uid) { + if (mBackgroundActivityStartCallback == null) { + return false; + } + synchronized (this) { + if (mBackgroundActivityStartTokens == null + || mBackgroundActivityStartTokens.isEmpty()) { + return false; + } + return mBackgroundActivityStartCallback.canCloseSystemDialogs( + mBackgroundActivityStartTokens.values(), uid); + } + } + + void dump(PrintWriter pw, String prefix) { + synchronized (this) { + if (mBackgroundActivityStartTokens != null + && !mBackgroundActivityStartTokens.isEmpty()) { + pw.print(prefix); + pw.println("Background activity start tokens (token: originating token):"); + for (int i = mBackgroundActivityStartTokens.size() - 1; i >= 0; i--) { + pw.print(prefix); + pw.print(" - "); + pw.print(mBackgroundActivityStartTokens.keyAt(i)); + pw.print(": "); + pw.println(mBackgroundActivityStartTokens.valueAt(i)); + } + } + if (mBoundClientUids != null && mBoundClientUids.size() > 0) { + pw.print(prefix); + pw.print("BoundClientUids:"); + pw.println(Arrays.toString(mBoundClientUids.toArray())); + } + } + } +} diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index 5676909d021c6..8b4d415efcb5b 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -24,13 +24,11 @@ import static android.os.IInputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS; import static com.android.internal.protolog.ProtoLogGroup.WM_DEBUG_CONFIGURATION; import static com.android.internal.util.Preconditions.checkArgument; import static com.android.server.am.ActivityManagerService.MY_PID; -import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_ACTIVITY_STARTS; import static com.android.server.wm.ActivityTaskManagerDebugConfig.DEBUG_RELEASE; import static com.android.server.wm.ActivityTaskManagerDebugConfig.POSTFIX_CONFIGURATION; import static com.android.server.wm.ActivityTaskManagerDebugConfig.POSTFIX_RELEASE; import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_ATM; import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_WITH_CLASS_NAME; -import static com.android.server.wm.ActivityTaskManagerService.ACTIVITY_BG_START_GRACE_PERIOD_MS; import static com.android.server.wm.ActivityTaskManagerService.INSTRUMENTATION_KEY_DISPATCHING_TIMEOUT_MILLIS; import static com.android.server.wm.ActivityTaskManagerService.RELAUNCH_REASON_NONE; import static com.android.server.wm.Task.ActivityState.DESTROYED; @@ -61,8 +59,6 @@ import android.os.IBinder; import android.os.Message; import android.os.Process; import android.os.RemoteException; -import android.os.SystemClock; -import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; import android.util.Slog; @@ -103,10 +99,6 @@ public class WindowProcessController extends ConfigurationContainer mBackgroundActivityStartTokens = new ArrayMap<>(); // The process of this application; 0 if none private volatile int mPid; // user of process. @@ -118,6 +110,7 @@ public class WindowProcessController extends ConfigurationContainer mPkgList = new ArraySet<>(); private final WindowProcessListener mListener; private final ActivityTaskManagerService mAtm; + private final BackgroundLaunchProcessController mBgLaunchController; // The actual proc... may be null only if 'persistent' is true (in which case we are in the // process of launching the app) private IApplicationThread mThread; @@ -169,8 +162,6 @@ public class WindowProcessController extends ConfigurationContainer mBoundClientUids = new ArraySet(); // Thread currently set for VR scheduling int mVrThreadTid; @@ -191,10 +182,10 @@ public class WindowProcessController extends ConfigurationContainer mAtm.getLastStopAppSwitchesTime() - || mLastActivityFinishTime > mAtm.getLastStopAppSwitchesTime()) { - if (DEBUG_ACTIVITY_STARTS) { - Slog.d(TAG, "[WindowProcessController(" + mPid - + ")] Activity start allowed: within " - + ACTIVITY_BG_START_GRACE_PERIOD_MS + "ms grace period"); - } - return true; - } - if (DEBUG_ACTIVITY_STARTS) { - Slog.d(TAG, "[WindowProcessController(" + mPid + ")] Activity start within " - + ACTIVITY_BG_START_GRACE_PERIOD_MS - + "ms grace period but also within stop app switch window"); - } - - } - } - // allow if the proc is instrumenting with background activity starts privs - if (mInstrumentingWithBackgroundActivityStartPrivileges) { - if (DEBUG_ACTIVITY_STARTS) { - Slog.d(TAG, "[WindowProcessController(" + mPid - + ")] Activity start allowed: process instrumenting with background " - + "activity starts privileges"); - } - return true; - } - // allow if the caller has an activity in any foreground task - if (appSwitchAllowed && hasActivityInVisibleTask()) { - if (DEBUG_ACTIVITY_STARTS) { - Slog.d(TAG, "[WindowProcessController(" + mPid - + ")] Activity start allowed: process has activity in foreground task"); - } - return true; - } - // allow if the caller is bound by a UID that's currently foreground - if (isBoundByForegroundUid()) { - if (DEBUG_ACTIVITY_STARTS) { - Slog.d(TAG, "[WindowProcessController(" + mPid - + ")] Activity start allowed: process bound by foreground uid"); - } - return true; - } - // allow if the flag was explicitly set - if (isBackgroundStartAllowedByToken(isCheckingForFgsStart)) { - if (DEBUG_ACTIVITY_STARTS) { - Slog.d(TAG, "[WindowProcessController(" + mPid - + ")] Activity start allowed: process allowed by token"); - } - return true; - } - return false; - } - - /** - * If there are no tokens, we don't allow *by token*. If there are tokens and - * isCheckingForFgsStart is false, we ask the callback if the start is allowed for these tokens, - * otherwise if there is no callback we allow. - */ - private boolean isBackgroundStartAllowedByToken(boolean isCheckingForFgsStart) { - if (mBackgroundActivityStartTokens.isEmpty()) { - return false; - } - - if (isCheckingForFgsStart) { - /// The checking is for BG-FGS-start. - return true; - } - - if (mBackgroundActivityStartCallback == null) { - // We have tokens but no callback to decide => allow - return true; - } - // The callback will decide - return mBackgroundActivityStartCallback.isActivityStartAllowed( - mBackgroundActivityStartTokens.values(), mInfo.uid, mInfo.packageName); + return mBgLaunchController.areBackgroundActivityStartsAllowed(mPid, mUid, mInfo.packageName, + appSwitchAllowed, isCheckingForFgsStart, hasVisibleActivities(), + mInstrumentingWithBackgroundActivityStartPrivileges, + mAtm.getLastStopAppSwitchesTime(), + mLastActivityLaunchTime, mLastActivityFinishTime); } /** * Returns whether this process is allowed to close system dialogs via a background activity * start token that allows the close system dialogs operation (eg. notification). */ - public boolean canCloseSystemDialogsByToken() { - synchronized (mAtm.mGlobalLock) { - return !mBackgroundActivityStartTokens.isEmpty() - && mBackgroundActivityStartCallback != null - && mBackgroundActivityStartCallback.canCloseSystemDialogs( - mBackgroundActivityStartTokens.values(), mInfo.uid); - } - } - - private boolean isBoundByForegroundUid() { - for (int i = mBoundClientUids.size() - 1; i >= 0; --i) { - if (mAtm.hasActiveVisibleWindow(mBoundClientUids.valueAt(i))) { - return true; - } - } - return false; + boolean canCloseSystemDialogsByToken() { + return mBgLaunchController.canCloseSystemDialogsByToken(mUid); } public void setBoundClientUids(ArraySet boundClientUids) { - mBoundClientUids = boundClientUids; + mBgLaunchController.setBoundClientUids(boundClientUids); } /** @@ -793,20 +679,6 @@ public class WindowProcessController extends ConfigurationContainer= 0; --i) { - Task task = mActivities.get(i).getTask(); - if (task == null) { - continue; - } - ActivityRecord topActivity = task.getTopNonFinishingActivity(); - if (topActivity != null && topActivity.mVisibleRequested) { - return true; - } - } - return false; - } - /** * Update the top resuming activity in process for pre-Q apps, only the top-most visible * activities are allowed to be resumed per process. @@ -1701,17 +1573,8 @@ public class WindowProcessController extends ConfigurationContainer