From 7056a06d698b7e20bc0f6538d2f76f3ae600ffe2 Mon Sep 17 00:00:00 2001 From: Wale Ogunwale Date: Thu, 18 Oct 2018 15:02:50 -0700 Subject: [PATCH] Have some shell start activity commands go thorugh AMS instead of ATMS (28/n) Some shell start activity commands set debug flags that requires us to synchronously call into the process logic in AMS in the middle of starting an acitivty in ATMS. To avoid dealock in this situation, we have the shell commands go throuhg AMS to start activity which will in turn acquire the AMS lock before calling into ATMS. Bug: 80414790 Test: Existing tests pass Change-Id: I407e0c6573cb903b9c2d2f635fd4d99ef833b026 --- .../android/app/ActivityManagerInternal.java | 4 ++ .../server/am/ActivityManagerService.java | 55 ++++++++++++++++++- .../am/ActivityManagerShellCommand.java | 4 +- .../server/am/ActivityStackSupervisor.java | 29 +++++----- 4 files changed, 73 insertions(+), 19 deletions(-) diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java index 50e618a577a26..069effd3ef193 100644 --- a/core/java/android/app/ActivityManagerInternal.java +++ b/core/java/android/app/ActivityManagerInternal.java @@ -276,4 +276,8 @@ public abstract class ActivityManagerInternal { /** Starts a given process. */ public abstract void startProcess(String processName, ApplicationInfo info, boolean knownToBeDead, String hostingType, ComponentName hostingName); + + /** Starts up the starting activity process for debugging if needed. */ + public abstract void setDebugFlagsForStartingActivity(ActivityInfo aInfo, int startFlags, + ProfilerInfo profilerInfo); } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 6a25cf13c6cfa..3253d58cf7da5 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -193,6 +193,7 @@ import android.app.NotificationManager; import android.app.PendingIntent; import android.app.ProcessMemoryState; import android.app.ProfilerInfo; +import android.app.WaitResult; import android.app.WindowConfiguration.ActivityType; import android.app.WindowConfiguration.WindowingMode; import android.app.backup.IBackupManager; @@ -3992,9 +3993,35 @@ public class ActivityManagerService extends IActivityManager.Stub public final int startActivityAsUser(IApplicationThread caller, String callingPackage, Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode, int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, int userId) { - return mActivityTaskManager.startActivityAsUser(caller, callingPackage, intent, - resolvedType, resultTo, resultWho, requestCode, startFlags, profilerInfo, bOptions, - userId); + synchronized (this) { + /** + * Flags like {@link android.app.ActivityManager#START_FLAG_DEBUG} maybe be set on this + * call when called/invoked from the shell command. To avoid deadlock, we go ahead and + * acquire the AMS lock now since ATMS will need to synchronously call back into AMS + * later to modify process settings due to the flags. + * TODO(b/80414790): Investigate a better way of untangling this. + */ + return mActivityTaskManager.startActivityAsUser(caller, callingPackage, intent, + resolvedType, resultTo, resultWho, requestCode, startFlags, profilerInfo, + bOptions, userId); + } + } + + WaitResult startActivityAndWait(IApplicationThread caller, String callingPackage, + Intent intent, String resolvedType, IBinder resultTo, String resultWho, int requestCode, + int startFlags, ProfilerInfo profilerInfo, Bundle bOptions, int userId) { + synchronized (this) { + /** + * Flags like {@link android.app.ActivityManager#START_FLAG_DEBUG} maybe be set on this + * call when called/invoked from the shell command. To avoid deadlock, we go ahead and + * acquire the AMS lock now since ATMS will need to synchronously call back into AMS + * later to modify process settings due to the flags. + * TODO(b/80414790): Investigate a better way of untangling this. + */ + return mActivityTaskManager.startActivityAndWait(caller, callingPackage, intent, + resolvedType, resultTo, resultWho, requestCode, startFlags, profilerInfo, + bOptions, userId); + } } @Override @@ -20420,6 +20447,28 @@ public class ActivityManagerService extends IActivityManager.Stub false /* isolated */, true /* keepIfLarge */); } } + + @Override + public void setDebugFlagsForStartingActivity(ActivityInfo aInfo, int startFlags, + ProfilerInfo profilerInfo) { + synchronized (ActivityManagerService.this) { + if ((startFlags & ActivityManager.START_FLAG_DEBUG) != 0) { + setDebugApp(aInfo.processName, true, false); + } + + if ((startFlags & ActivityManager.START_FLAG_NATIVE_DEBUGGING) != 0) { + setNativeDebuggingAppLocked(aInfo.applicationInfo, aInfo.processName); + } + + if ((startFlags & ActivityManager.START_FLAG_TRACK_ALLOCATION) != 0) { + setTrackAllocationApp(aInfo.applicationInfo, aInfo.processName); + } + + if (profilerInfo != null) { + setProfileApp(aInfo.applicationInfo, aInfo.processName, profilerInfo); + } + } + } } long inputDispatchingTimedOut(int pid, final boolean aboveSystem, String reason) { diff --git a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java index 9f768a84d2237..692b2d433dbc1 100644 --- a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java +++ b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java @@ -481,12 +481,12 @@ final class ActivityManagerShellCommand extends ShellCommand { options.setLockTaskEnabled(true); } if (mWaitOption) { - result = mTaskInterface.startActivityAndWait(null, null, intent, mimeType, + result = mInternal.startActivityAndWait(null, null, intent, mimeType, null, null, 0, mStartFlags, profilerInfo, options != null ? options.toBundle() : null, mUserId); res = result.result; } else { - res = mTaskInterface.startActivityAsUser(null, null, intent, mimeType, + res = mInternal.startActivityAsUser(null, null, intent, mimeType, null, null, 0, mStartFlags, profilerInfo, options != null ? options.toBundle() : null, mUserId); } diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java index 2e36fc9385bad..90e2f5bcd1b48 100644 --- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java @@ -21,6 +21,9 @@ import static android.Manifest.permission.INTERNAL_SYSTEM_WINDOW; import static android.Manifest.permission.START_ANY_ACTIVITY; import static android.app.ActivityManager.LOCK_TASK_MODE_LOCKED; import static android.app.ActivityManager.START_DELIVERED_TO_TOP; +import static android.app.ActivityManager.START_FLAG_DEBUG; +import static android.app.ActivityManager.START_FLAG_NATIVE_DEBUGGING; +import static android.app.ActivityManager.START_FLAG_TRACK_ALLOCATION; import static android.app.ActivityManager.START_TASK_TO_FRONT; import static android.app.ActivityTaskManager.INVALID_STACK_ID; import static android.app.ActivityTaskManager.INVALID_TASK_ID; @@ -113,6 +116,7 @@ import android.app.ActivityManager.StackInfo; import android.app.ActivityManagerInternal; import android.app.ActivityOptions; import android.app.AppOpsManager; +import android.app.IApplicationThread; import android.app.ProfilerInfo; import android.app.ResultInfo; import android.app.WaitResult; @@ -1300,20 +1304,17 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D // Don't debug things in the system process if (!aInfo.processName.equals("system")) { - if ((startFlags & ActivityManager.START_FLAG_DEBUG) != 0) { - mService.mAm.setDebugApp(aInfo.processName, true, false); - } - - if ((startFlags & ActivityManager.START_FLAG_NATIVE_DEBUGGING) != 0) { - mService.mAm.setNativeDebuggingAppLocked(aInfo.applicationInfo, aInfo.processName); - } - - if ((startFlags & ActivityManager.START_FLAG_TRACK_ALLOCATION) != 0) { - mService.mAm.setTrackAllocationApp(aInfo.applicationInfo, aInfo.processName); - } - - if (profilerInfo != null) { - mService.mAm.setProfileApp(aInfo.applicationInfo, aInfo.processName, profilerInfo); + if ((startFlags & (START_FLAG_DEBUG | START_FLAG_NATIVE_DEBUGGING + | START_FLAG_TRACK_ALLOCATION)) != 0 || profilerInfo != null) { + /** + * Assume safe to call into AMS synchronously because the call that set these + * flags should have originated from AMS which will already have its lock held. + * @see ActivityManagerService#startActivityAndWait(IApplicationThread, String, + * Intent, String, IBinder, String, int, int, ProfilerInfo, Bundle, int) + * TODO(b/80414790): Investigate a better way of untangling this. + */ + mService.mAmInternal.setDebugFlagsForStartingActivity( + aInfo, startFlags, profilerInfo); } } final String intentLaunchToken = intent.getLaunchToken();