From 12f3e8d6d4d5858bed9f8dd4b0106573151450d3 Mon Sep 17 00:00:00 2001 From: Marco Ballesio Date: Wed, 3 Jun 2020 08:56:46 -0700 Subject: [PATCH] freezer: reentrant enable/disable method the app freezer state can be toggled in multiple situations when a debugging tool like heapdump is invoked, or when system_server dumps its binder connections. All these uses are potentially concurrent and a robust method to handle reentrancy is necessary to avoid leaving the freezer in a state incompatible with a specific operation. This patch moves freezer enable and disable operations from Process to CachedAppOptimizer (ActivityManager), introduces a new ActivityManager API to centralize all freezer state changes to ActivityManager and modifies client code accordingly. Bug: 151225245 Test: manually verified that no regression are introduced on freezer behavior, verified that concurrent behavior is handled properly Change-Id: I7d588cc6e0499012dce64ed4e42ff2adb336062d Merged-In: I7d588cc6e0499012dce64ed4e42ff2adb336062d --- core/java/android/app/IActivityManager.aidl | 10 +++ core/java/android/os/BinderProxy.java | 14 ++++- core/jni/android_util_Process.cpp | 17 ----- .../server/am/ActivityManagerService.java | 48 +++++++------- .../android/server/am/CachedAppOptimizer.java | 63 ++++++++++++++++++- services/core/jni/Android.bp | 1 + ...m_android_server_am_CachedAppOptimizer.cpp | 18 ++++++ 7 files changed, 124 insertions(+), 47 deletions(-) diff --git a/core/java/android/app/IActivityManager.aidl b/core/java/android/app/IActivityManager.aidl index 2abe9cf9fce57..f98e26338063a 100644 --- a/core/java/android/app/IActivityManager.aidl +++ b/core/java/android/app/IActivityManager.aidl @@ -684,4 +684,14 @@ interface IActivityManager { * Kills uid with the reason of permission change. */ void killUidForPermissionChange(int appId, int userId, String reason); + + /** + * Control the app freezer state. Returns true in case of success, false if the operation + * didn't succeed (for example, when the app freezer isn't supported). + * Handling the freezer state via this method is reentrant, that is it can be + * disabled and re-enabled multiple times in parallel. As long as there's a 1:1 disable to + * enable match, the freezer is re-enabled at last enable only. + * @param enable set it to true to enable the app freezer, false to disable it. + */ + boolean enableAppFreezer(in boolean enable); } diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java index 683993f762c05..0185ba444ca4d 100644 --- a/core/java/android/os/BinderProxy.java +++ b/core/java/android/os/BinderProxy.java @@ -18,6 +18,7 @@ package android.os; import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.ActivityManager; import android.app.AppOpsManager; import android.util.Log; import android.util.SparseIntArray; @@ -255,7 +256,12 @@ public final class BinderProxy implements IBinder { // out of system_server to all processes hosting binder objects it holds a reference to; // since some of those processes might be frozen, we don't want to block here // forever. Disable the freezer. - Process.enableFreezer(false); + try { + ActivityManager.getService().enableAppFreezer(false); + } catch (RemoteException e) { + Log.e(Binder.TAG, "RemoteException while disabling app freezer"); + } + for (WeakReference weakRef : proxiesToQuery) { BinderProxy bp = weakRef.get(); String key; @@ -278,7 +284,11 @@ public final class BinderProxy implements IBinder { counts.put(key, i + 1); } } - Process.enableFreezer(true); + try { + ActivityManager.getService().enableAppFreezer(true); + } catch (RemoteException e) { + Log.e(Binder.TAG, "RemoteException while re-enabling app freezer"); + } Map.Entry[] sorted = counts.entrySet().toArray( new Map.Entry[counts.size()]); diff --git a/core/jni/android_util_Process.cpp b/core/jni/android_util_Process.cpp index 3486a8574ba03..6becb07d02a4d 100644 --- a/core/jni/android_util_Process.cpp +++ b/core/jni/android_util_Process.cpp @@ -346,22 +346,6 @@ void android_os_Process_setProcessFrozen( } } -void android_os_Process_enableFreezer( - JNIEnv *env, jobject clazz, jboolean enable) -{ - bool success = true; - - if (enable) { - success = SetTaskProfiles(0, {"FreezerEnabled"}, true); - } else { - success = SetTaskProfiles(0, {"FreezerDisabled"}, true); - } - - if (!success) { - jniThrowException(env, "java/lang/RuntimeException", "Unknown error"); - } -} - jint android_os_Process_getProcessGroup(JNIEnv* env, jobject clazz, jint pid) { SchedPolicy sp; @@ -1360,7 +1344,6 @@ static const JNINativeMethod methods[] = { {"sendSignal", "(II)V", (void*)android_os_Process_sendSignal}, {"sendSignalQuiet", "(II)V", (void*)android_os_Process_sendSignalQuiet}, {"setProcessFrozen", "(IIZ)V", (void*)android_os_Process_setProcessFrozen}, - {"enableFreezer", "(Z)V", (void*)android_os_Process_enableFreezer}, {"getFreeMemory", "()J", (void*)android_os_Process_getFreeMemory}, {"getTotalMemory", "()J", (void*)android_os_Process_getTotalMemory}, {"readProcLines", "(Ljava/lang/String;[Ljava/lang/String;[J)V", diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 44e3bbf915655..491579a5a2946 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -2218,17 +2218,13 @@ public class ActivityManagerService extends IActivityManager.Stub @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { try { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(false); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(false); if (!DumpUtils.checkDumpAndUsageStatsPermission(mActivityManagerService.mContext, "meminfo", pw)) return; PriorityDump.dump(mPriorityDumper, fd, pw, args); } finally { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(true); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(true); } } } @@ -2242,17 +2238,13 @@ public class ActivityManagerService extends IActivityManager.Stub @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { try { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(false); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(false); if (!DumpUtils.checkDumpAndUsageStatsPermission(mActivityManagerService.mContext, "gfxinfo", pw)) return; mActivityManagerService.dumpGraphicsHardwareUsage(fd, pw, args); } finally { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(true); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(true); } } } @@ -2266,17 +2258,13 @@ public class ActivityManagerService extends IActivityManager.Stub @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { try { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(false); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(false); if (!DumpUtils.checkDumpAndUsageStatsPermission(mActivityManagerService.mContext, "dbinfo", pw)) return; mActivityManagerService.dumpDbInfo(fd, pw, args); } finally { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(true); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(true); } } } @@ -2322,9 +2310,7 @@ public class ActivityManagerService extends IActivityManager.Stub @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { try { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(false); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(false); if (!DumpUtils.checkDumpAndUsageStatsPermission(mActivityManagerService.mContext, "cacheinfo", pw)) { @@ -2333,9 +2319,7 @@ public class ActivityManagerService extends IActivityManager.Stub mActivityManagerService.dumpBinderCacheContents(fd, pw, args); } finally { - if (mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.useFreezer()) { - Process.enableFreezer(true); - } + mActivityManagerService.mOomAdjuster.mCachedAppOptimizer.enableFreezer(true); } } } @@ -18630,14 +18614,14 @@ public class ActivityManagerService extends IActivityManager.Stub } } - Process.enableFreezer(false); + mOomAdjuster.mCachedAppOptimizer.enableFreezer(false); final RemoteCallback intermediateCallback = new RemoteCallback( new RemoteCallback.OnResultListener() { @Override public void onResult(Bundle result) { finishCallback.sendResult(result); - Process.enableFreezer(true); + mOomAdjuster.mCachedAppOptimizer.enableFreezer(true); } }, null); @@ -20399,4 +20383,16 @@ public class ActivityManagerService extends IActivityManager.Stub Binder.restoreCallingIdentity(token); } } + + @Override + public boolean enableAppFreezer(boolean enable) { + int callerUid = Binder.getCallingUid(); + + // Only system can toggle the freezer state + if (callerUid == SYSTEM_UID) { + return mOomAdjuster.mCachedAppOptimizer.enableFreezer(enable); + } else { + throw new SecurityException("Caller uid " + callerUid + " cannot set freezer state "); + } + } } diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index ce7c73a5d8a31..c5047e5eed03e 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -208,6 +208,8 @@ public final class CachedAppOptimizer { @GuardedBy("mPhenotypeFlagLock") private volatile boolean mUseCompaction = DEFAULT_USE_COMPACTION; private volatile boolean mUseFreezer = DEFAULT_USE_FREEZER; + @GuardedBy("this") + private int mFreezerDisableCount = 1; // Freezer is initially disabled, until enabled private final Random mRandom = new Random(); @GuardedBy("mPhenotypeFlagLock") @VisibleForTesting volatile float mCompactStatsdSampleRate = DEFAULT_STATSD_SAMPLE_RATE; @@ -419,6 +421,63 @@ public final class CachedAppOptimizer { } } + /** + * Enables or disabled the app freezer. + * @param enable Enables the freezer if true, disables it if false. + * @return true if the operation completed successfully, false otherwise. + */ + public synchronized boolean enableFreezer(boolean enable) { + if (!mUseFreezer) { + return false; + } + + if (enable) { + mFreezerDisableCount--; + + if (mFreezerDisableCount > 0) { + return true; + } else if (mFreezerDisableCount < 0) { + Slog.e(TAG_AM, "unbalanced call to enableFreezer, ignoring"); + mFreezerDisableCount = 0; + return false; + } + } else { + mFreezerDisableCount++; + + if (mFreezerDisableCount > 1) { + return true; + } + } + + try { + enableFreezerInternal(enable); + return true; + } catch (java.lang.RuntimeException e) { + if (enable) { + mFreezerDisableCount = 0; + } else { + mFreezerDisableCount = 1; + } + + Slog.e(TAG_AM, "Exception handling freezer state (enable: " + enable + "): " + + e.toString()); + } + + return false; + } + + /** + * Enable or disable the freezer. When enable == false all frozen processes are unfrozen, + * but aren't removed from the freezer. While in this state, processes can be added or removed + * by using Process.setProcessFrozen(), but they wouldn't be actually frozen until the freezer + * is enabled. If enable == true all processes in the freezer are frozen. + * + * @param enable Specify whether to enable (true) or disable (false) the freezer. + * + * @hide + */ + private static native void enableFreezerInternal(boolean enable); + /** * Determines whether the freezer is supported by this system */ @@ -471,7 +530,7 @@ public final class CachedAppOptimizer { if (mUseFreezer && mFreezeHandler == null) { Slog.d(TAG_AM, "Freezer enabled"); - Process.enableFreezer(true); + enableFreezer(true); if (!mCachedAppOptimizerThread.isAlive()) { mCachedAppOptimizerThread.start(); @@ -482,7 +541,7 @@ public final class CachedAppOptimizer { Process.setThreadGroupAndCpuset(mCachedAppOptimizerThread.getThreadId(), Process.THREAD_GROUP_SYSTEM); } else { - Process.enableFreezer(false); + enableFreezer(false); } } diff --git a/services/core/jni/Android.bp b/services/core/jni/Android.bp index 925ad0f57f191..460842e56764e 100644 --- a/services/core/jni/Android.bp +++ b/services/core/jni/Android.bp @@ -106,6 +106,7 @@ cc_defaults { "libkeystore_binder", "libmtp", "libnativehelper", + "libprocessgroup", "libutils", "libui", "libinput", diff --git a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp index 6a6da0e2b3953..7e9e11d209a69 100644 --- a/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp +++ b/services/core/jni/com_android_server_am_CachedAppOptimizer.cpp @@ -30,6 +30,7 @@ #include #include #include +#include using android::base::StringPrintf; using android::base::WriteStringToFile; @@ -74,9 +75,26 @@ static void com_android_server_am_CachedAppOptimizer_compactSystem(JNIEnv *, job } } +static void com_android_server_am_CachedAppOptimizer_enableFreezerInternal( + JNIEnv *env, jobject clazz, jboolean enable) { + bool success = true; + + if (enable) { + success = SetTaskProfiles(0, {"FreezerEnabled"}, true); + } else { + success = SetTaskProfiles(0, {"FreezerDisabled"}, true); + } + + if (!success) { + jniThrowException(env, "java/lang/RuntimeException", "Unknown error"); + } +} + static const JNINativeMethod sMethods[] = { /* name, signature, funcPtr */ {"compactSystem", "()V", (void*)com_android_server_am_CachedAppOptimizer_compactSystem}, + {"enableFreezerInternal", "(Z)V", + (void*)com_android_server_am_CachedAppOptimizer_enableFreezerInternal}, }; int register_android_server_am_CachedAppOptimizer(JNIEnv* env)