From 67a598b050d619e4d2200a736197a4b5ec1e31e4 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Tue, 29 Jun 2021 02:01:41 -0700 Subject: [PATCH] Skip resetting the shouldNotFreeze flag while re-evaluating oom adj Otherwise this flag could be reset to false unexpectedly if there's a cycle between the processes. Also pass the shouldNotFreeze flag down the bindings. BYPASS_INCLUSIVE_LANGUAGE_REASON=Legacy API Bug: 192317641 Test: atest FrameworksServicesTests:ActivityManagerTest Change-Id: I775c3d5de0676cb106c54c098d3e06d6571da514 --- .../android/server/am/CachedAppOptimizer.java | 21 +++++ .../com/android/server/am/OomAdjuster.java | 21 +++-- .../android/server/am/ProcessStateRecord.java | 17 ---- .../server/am/ActivityManagerTest.java | 78 +++++++++++++++++- .../simpleservicetestapp/SimpleService.java | 79 ++++++++++++++++++- 5 files changed, 187 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/am/CachedAppOptimizer.java b/services/core/java/com/android/server/am/CachedAppOptimizer.java index 317b7757c2395..8db7eeaaa89a0 100644 --- a/services/core/java/com/android/server/am/CachedAppOptimizer.java +++ b/services/core/java/com/android/server/am/CachedAppOptimizer.java @@ -212,6 +212,23 @@ public final class CachedAppOptimizer { } }; + private final OnPropertiesChangedListener mOnNativeBootFlagsChangedListener = + new OnPropertiesChangedListener() { + @Override + public void onPropertiesChanged(Properties properties) { + synchronized (mPhenotypeFlagLock) { + for (String name : properties.getKeyset()) { + if (KEY_FREEZER_DEBOUNCE_TIMEOUT.equals(name)) { + updateFreezerDebounceTimeout(); + } + } + } + if (mTestCallback != null) { + mTestCallback.onPropertyChanged(); + } + } + }; + private final class SettingsContentObserver extends ContentObserver { SettingsContentObserver() { super(mAm.mHandler); @@ -328,6 +345,10 @@ public final class CachedAppOptimizer { // TODO: initialize flags to default and only update them if values are set in DeviceConfig DeviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, ActivityThread.currentApplication().getMainExecutor(), mOnFlagsChangedListener); + DeviceConfig.addOnPropertiesChangedListener( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER_NATIVE_BOOT, + ActivityThread.currentApplication().getMainExecutor(), + mOnNativeBootFlagsChangedListener); mAm.mContext.getContentResolver().registerContentObserver( CACHED_APP_FREEZER_ENABLED_URI, false, mSettingsObserver); synchronized (mPhenotypeFlagLock) { diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 6cebf47ec8c22..5cb6fd9a52797 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -1530,9 +1530,11 @@ public class OomAdjuster { state.setAdjTarget(null); state.setEmpty(false); state.setCached(false); - state.setNoKillOnForcedAppStandbyAndIdle(false); state.resetAllowStartFgsState(); - app.mOptRecord.setShouldNotFreeze(false); + if (!cycleReEval) { + // Don't reset this flag when doing cycles re-evaluation. + app.mOptRecord.setShouldNotFreeze(false); + } final int appUid = app.info.uid; final int logUid = mService.mCurOomAdjUid; @@ -1983,6 +1985,11 @@ public class OomAdjuster { final boolean clientIsSystem = clientProcState < PROCESS_STATE_TOP; + if (client.mOptRecord.shouldNotFreeze()) { + // Propagate the shouldNotFreeze flag down the bindings. + app.mOptRecord.setShouldNotFreeze(true); + } + if ((cr.flags & Context.BIND_WAIVE_PRIORITY) == 0) { if (shouldSkipDueToCycle(app, cstate, procState, adj, cycleReEval)) { continue; @@ -2019,9 +2026,6 @@ public class OomAdjuster { // Similar to BIND_WAIVE_PRIORITY, keep it unfrozen. if (clientAdj < ProcessList.CACHED_APP_MIN_ADJ) { app.mOptRecord.setShouldNotFreeze(true); - // Similarly, we shouldn't kill it when it's in forced-app-standby - // mode and cached & idle state. - app.mState.setNoKillOnForcedAppStandbyAndIdle(true); } // Not doing bind OOM management, so treat // this guy more like a started service. @@ -2226,9 +2230,6 @@ public class OomAdjuster { // unfrozen. if (clientAdj < ProcessList.CACHED_APP_MIN_ADJ) { app.mOptRecord.setShouldNotFreeze(true); - // Similarly, we shouldn't kill it when it's in forced-app-standby - // mode and cached & idle state. - app.mState.setNoKillOnForcedAppStandbyAndIdle(true); } } if ((cr.flags&Context.BIND_TREAT_LIKE_ACTIVITY) != 0) { @@ -2302,6 +2303,10 @@ public class OomAdjuster { // we are going to consider it empty. clientProcState = PROCESS_STATE_CACHED_EMPTY; } + if (client.mOptRecord.shouldNotFreeze()) { + // Propagate the shouldNotFreeze flag down the bindings. + app.mOptRecord.setShouldNotFreeze(true); + } String adjType = null; if (adj > clientAdj) { if (state.hasShownUi() && !state.getCachedIsHomeProcess() diff --git a/services/core/java/com/android/server/am/ProcessStateRecord.java b/services/core/java/com/android/server/am/ProcessStateRecord.java index c113bc2ecb4f4..7520d88fcd164 100644 --- a/services/core/java/com/android/server/am/ProcessStateRecord.java +++ b/services/core/java/com/android/server/am/ProcessStateRecord.java @@ -357,13 +357,6 @@ final class ProcessStateRecord { @ElapsedRealtimeLong private long mLastInvisibleTime; - /** - * Whether or not this process could be killed when it's in forced-app-standby mode - * and cached & idle state. - */ - @GuardedBy("mService") - private boolean mNoKillOnForcedAppStandbyAndIdle; - // Below are the cached task info for OomAdjuster only private static final int VALUE_INVALID = -1; private static final int VALUE_FALSE = 0; @@ -1134,16 +1127,6 @@ final class ProcessStateRecord { return mLastInvisibleTime; } - @GuardedBy("mService") - void setNoKillOnForcedAppStandbyAndIdle(boolean shouldNotKill) { - mNoKillOnForcedAppStandbyAndIdle = shouldNotKill; - } - - @GuardedBy("mService") - boolean shouldNotKillOnForcedAppStandbyAndIdle() { - return mNoKillOnForcedAppStandbyAndIdle; - } - @GuardedBy({"mService", "mProcLock"}) void dump(PrintWriter pw, String prefix, long nowUptime) { if (mReportedInteraction || mFgInteractionTime != 0) { diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerTest.java index 4b030d18f7c39..857726bdb4b5f 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerTest.java @@ -94,6 +94,17 @@ public class ActivityManagerTest { "com.android.servicestests.apps.simpleservicetestapp.ACTION_FGS_STATS_TEST"; private static final String EXTRA_MESSENGER = "extra_messenger"; + private static final String EXTRA_CALLBACK = "callback"; + private static final String EXTRA_COMMAND = "command"; + private static final String EXTRA_FLAGS = "flags"; + private static final String EXTRA_TARGET_PACKAGE = "target_package"; + + private static final int COMMAND_INVALID = 0; + private static final int COMMAND_EMPTY = 1; + private static final int COMMAND_BIND_SERVICE = 2; + private static final int COMMAND_UNBIND_SERVICE = 3; + private static final int COMMAND_STOP_SELF = 4; + private IActivityManager mService; private IRemoteCallback mCallback; private Context mContext; @@ -333,6 +344,12 @@ public class ActivityManagerTest { SettingsSession amConstantsSettings = null; DeviceConfigSession freezerDebounceTimeout = null; MyServiceConnection autoConnection = null; + final ActivityManager am = mContext.getSystemService(ActivityManager.class); + final PackageManager pm = mContext.getPackageManager(); + final int uid1 = pm.getPackageUid(TEST_APP1, 0); + final int uid2 = pm.getPackageUid(TEST_APP2, 0); + final MyUidImportanceListener uid1Listener = new MyUidImportanceListener(uid1); + final MyUidImportanceListener uid2Listener = new MyUidImportanceListener(uid2); try { if (!freezerWasEnabled) { freezerEnabled = new SettingsSession<>( @@ -346,7 +363,7 @@ public class ActivityManagerTest { } } freezerDebounceTimeout = new DeviceConfigSession<>( - DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER_NATIVE_BOOT, CachedAppOptimizer.KEY_FREEZER_DEBOUNCE_TIMEOUT, DeviceConfig::getLong, CachedAppOptimizer.DEFAULT_FREEZER_DEBOUNCE_TIMEOUT); freezerDebounceTimeout.set(waitFor); @@ -359,13 +376,20 @@ public class ActivityManagerTest { amConstantsSettings.set( ActivityManagerConstants.KEY_MAX_SERVICE_INACTIVITY + "=" + waitFor); + runShellCommand("cmd deviceidle whitelist +" + TEST_APP1); + runShellCommand("cmd deviceidle whitelist +" + TEST_APP2); + + am.addOnUidImportanceListener(uid1Listener, + RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE); + am.addOnUidImportanceListener(uid2Listener, RunningAppProcessInfo.IMPORTANCE_CACHED); + final Intent intent = new Intent(); intent.setClassName(TEST_APP1, TEST_CLASS); - final CountDownLatch latch = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(1); autoConnection = new MyServiceConnection(latch); mContext.bindService(intent, autoConnection, - Context.BIND_AUTO_CREATE | Context.BIND_ALLOW_OOM_MANAGEMENT); + Context.BIND_AUTO_CREATE | Context.BIND_WAIVE_PRIORITY); try { assertTrue("Timeout to bind to service " + intent.getComponent(), latch.await(AWAIT_TIMEOUT, TimeUnit.MILLISECONDS)); @@ -383,6 +407,37 @@ public class ActivityManagerTest { // It still shouldn't be frozen, although it's been in cached state. assertFalse(TEST_APP1 + " shouldn't be frozen now.", isAppFrozen(TEST_APP1)); + + final CountDownLatch[] latchHolder = new CountDownLatch[1]; + final IRemoteCallback callback = new IRemoteCallback.Stub() { + @Override + public void sendResult(Bundle bundle) { + if (bundle != null) { + latchHolder[0].countDown(); + } + } + }; + + // Bind from app1 to app2 without BIND_WAIVE_PRIORITY. + final Bundle extras = new Bundle(); + extras.putBinder(EXTRA_CALLBACK, callback.asBinder()); + latchHolder[0] = new CountDownLatch(1); + sendCommand(COMMAND_BIND_SERVICE, TEST_APP1, TEST_APP2, extras); + assertTrue("Timed out to bind to " + TEST_APP2, latchHolder[0].await( + waitFor, TimeUnit.MILLISECONDS)); + + // Stop service in app1 + extras.clear(); + sendCommand(COMMAND_STOP_SELF, TEST_APP1, TEST_APP1, extras); + + assertTrue(TEST_APP2 + " should be in cached", uid2Listener.waitFor( + RunningAppProcessInfo.IMPORTANCE_CACHED, waitFor)); + + // Wait for the freezer kick in if there is any. + Thread.sleep(waitFor * 4); + + // It still shouldn't be frozen, although it's been in cached state. + assertFalse(TEST_APP2 + " shouldn't be frozen now.", isAppFrozen(TEST_APP2)); } finally { toggleScreenOn(true); if (amConstantsSettings != null) { @@ -397,9 +452,26 @@ public class ActivityManagerTest { if (autoConnection != null) { mContext.unbindService(autoConnection); } + am.removeOnUidImportanceListener(uid1Listener); + am.removeOnUidImportanceListener(uid2Listener); + sendCommand(COMMAND_UNBIND_SERVICE, TEST_APP1, TEST_APP2, null); + sendCommand(COMMAND_UNBIND_SERVICE, TEST_APP2, TEST_APP1, null); + runShellCommand("cmd deviceidle whitelist -" + TEST_APP1); + runShellCommand("cmd deviceidle whitelist -" + TEST_APP2); } } + private void sendCommand(int command, String sourcePkg, String targetPkg, Bundle extras) { + final Intent intent = new Intent(); + intent.setClassName(sourcePkg, TEST_CLASS); + intent.putExtra(EXTRA_COMMAND, command); + intent.putExtra(EXTRA_TARGET_PACKAGE, targetPkg); + if (extras != null) { + intent.putExtras(extras); + } + mContext.startService(intent); + } + private boolean isFreezerEnabled() throws Exception { final String output = runShellCommand("dumpsys activity settings"); final Matcher matcher = Pattern.compile("\\b" + CachedAppOptimizer.KEY_USE_FREEZER diff --git a/services/tests/servicestests/test-apps/SimpleServiceTestApp/src/com/android/servicestests/apps/simpleservicetestapp/SimpleService.java b/services/tests/servicestests/test-apps/SimpleServiceTestApp/src/com/android/servicestests/apps/simpleservicetestapp/SimpleService.java index 674ce8aade4c7..4e981b22cd320 100644 --- a/services/tests/servicestests/test-apps/SimpleServiceTestApp/src/com/android/servicestests/apps/simpleservicetestapp/SimpleService.java +++ b/services/tests/servicestests/test-apps/SimpleServiceTestApp/src/com/android/servicestests/apps/simpleservicetestapp/SimpleService.java @@ -16,29 +16,106 @@ package com.android.servicestests.apps.simpleservicetestapp; import android.app.Service; +import android.content.ComponentName; import android.content.Intent; +import android.content.ServiceConnection; import android.os.Bundle; import android.os.IBinder; import android.os.IRemoteCallback; import android.os.Process; +import android.os.RemoteException; +import android.util.ArrayMap; import android.util.Log; public class SimpleService extends Service { private static final String TAG = "SimpleService"; + private static final String TEST_CLASS = + "com.android.servicestests.apps.simpleservicetestapp.SimpleService"; + + private static final String EXTRA_CALLBACK = "callback"; + private static final String EXTRA_COMMAND = "command"; + private static final String EXTRA_FLAGS = "flags"; + private static final String EXTRA_TARGET_PACKAGE = "target_package"; + + private static final int COMMAND_INVALID = 0; + private static final int COMMAND_EMPTY = 1; + private static final int COMMAND_BIND_SERVICE = 2; + private static final int COMMAND_UNBIND_SERVICE = 3; + private static final int COMMAND_STOP_SELF = 4; + + private ArrayMap mServiceConnections = new ArrayMap<>(); + private final IRemoteCallback.Stub mBinder = new IRemoteCallback.Stub() { @Override public void sendResult(Bundle bundle) { - Process.killProcess(Process.myPid()); + if (bundle == null) { + Process.killProcess(Process.myPid()); + } else { + // No-op for now. + } } }; @Override public int onStartCommand(Intent intent, int flags, int startId) { Log.i(TAG, "onStartCommand"); + int command = intent.getIntExtra(EXTRA_COMMAND, COMMAND_INVALID); + if (command != COMMAND_INVALID) { + final String targetPkg = intent.getStringExtra(EXTRA_TARGET_PACKAGE); + Log.i(TAG, "Received command " + command + " targetPkg=" + targetPkg); + switch (command) { + case COMMAND_BIND_SERVICE: + final Bundle extras = intent.getExtras(); + bindToService(targetPkg, intent.getIntExtra(EXTRA_FLAGS, 0), + IRemoteCallback.Stub.asInterface(extras.getBinder(EXTRA_CALLBACK))); + break; + case COMMAND_UNBIND_SERVICE: + unbindService(targetPkg); + break; + case COMMAND_STOP_SELF: + stopSelf(); + return START_NOT_STICKY; + } + } return START_STICKY; } + private void bindToService(String targetPkg, int flags, IRemoteCallback callback) { + Intent intent = new Intent(); + intent.setClassName(targetPkg, TEST_CLASS); + final ServiceConnection conn = new ServiceConnection() { + @Override + public void onServiceConnected(ComponentName name, IBinder service) { + if (callback != null) { + try { + callback.sendResult(new Bundle()); + } catch (RemoteException e) { + } + } + } + + @Override + public void onServiceDisconnected(ComponentName name) { + } + }; + if (getApplicationContext().bindService(intent, conn, BIND_AUTO_CREATE | flags)) { + mServiceConnections.put(targetPkg, conn); + } else if (callback != null) { + try { + callback.sendResult(null); + } catch (RemoteException e) { + } + } + } + + private void unbindService(String targetPkg) { + final ServiceConnection conn = mServiceConnections.remove(targetPkg); + if (conn != null) { + getApplicationContext().unbindService(conn); + } + } + @Override public IBinder onBind(Intent intent) { return mBinder;