diff options
6 files changed, 121 insertions, 62 deletions
diff --git a/packages/CrashRecovery/services/java/com/android/server/PackageWatchdog.java b/packages/CrashRecovery/services/java/com/android/server/PackageWatchdog.java index d256aead97e8..b5cf011c32a6 100644 --- a/packages/CrashRecovery/services/java/com/android/server/PackageWatchdog.java +++ b/packages/CrashRecovery/services/java/com/android/server/PackageWatchdog.java @@ -33,12 +33,12 @@ import android.os.Looper; import android.os.Process; import android.os.SystemProperties; import android.provider.DeviceConfig; +import android.sysprop.CrashRecoveryProperties; import android.text.TextUtils; import android.util.ArrayMap; import android.util.ArraySet; import android.util.AtomicFile; import android.util.LongArrayQueue; -import android.util.MathUtils; import android.util.Slog; import android.util.Xml; @@ -128,18 +128,9 @@ public class PackageWatchdog { @VisibleForTesting static final int DEFAULT_BOOT_LOOP_TRIGGER_COUNT = 5; + @VisibleForTesting static final long DEFAULT_BOOT_LOOP_TRIGGER_WINDOW_MS = TimeUnit.MINUTES.toMillis(10); - // These properties track individual system server boot events, and are reset once the boot - // threshold is met, or the boot loop trigger window is exceeded between boot events. - private static final String PROP_RESCUE_BOOT_COUNT = "sys.rescue_boot_count"; - private static final String PROP_RESCUE_BOOT_START = "sys.rescue_boot_start"; - - // These properties track multiple calls made to observers tracking boot loops. They are reset - // when the de-escalation window is exceeded between boot events. - private static final String PROP_BOOT_MITIGATION_WINDOW_START = "sys.boot_mitigation_start"; - private static final String PROP_BOOT_MITIGATION_COUNT = "sys.boot_mitigation_count"; - private long mNumberOfNativeCrashPollsRemaining; private static final int DB_VERSION = 1; @@ -1702,42 +1693,45 @@ public class PackageWatchdog { setCount(0); } - private int getCount() { - return SystemProperties.getInt(PROP_RESCUE_BOOT_COUNT, 0); + protected int getCount() { + return CrashRecoveryProperties.rescueBootCount().orElse(0); } - private void setCount(int count) { - SystemProperties.set(PROP_RESCUE_BOOT_COUNT, Integer.toString(count)); + protected void setCount(int count) { + CrashRecoveryProperties.rescueBootCount(count); } public long getStart() { - return SystemProperties.getLong(PROP_RESCUE_BOOT_START, 0); + return CrashRecoveryProperties.rescueBootStart().orElse(0L); } public int getMitigationCount() { - return SystemProperties.getInt(PROP_BOOT_MITIGATION_COUNT, 0); + return CrashRecoveryProperties.bootMitigationCount().orElse(0); } public void setStart(long start) { - setPropertyStart(PROP_RESCUE_BOOT_START, start); + CrashRecoveryProperties.rescueBootStart(getStartTime(start)); } public void setMitigationStart(long start) { - setPropertyStart(PROP_BOOT_MITIGATION_WINDOW_START, start); + CrashRecoveryProperties.bootMitigationStart(getStartTime(start)); } public long getMitigationStart() { - return SystemProperties.getLong(PROP_BOOT_MITIGATION_WINDOW_START, 0); + return CrashRecoveryProperties.bootMitigationStart().orElse(0L); } public void setMitigationCount(int count) { - SystemProperties.set(PROP_BOOT_MITIGATION_COUNT, Integer.toString(count)); + CrashRecoveryProperties.bootMitigationCount(count); + } + + private static long constrain(long amount, long low, long high) { + return amount < low ? low : (amount > high ? high : amount); } - public void setPropertyStart(String property, long start) { + public long getStartTime(long start) { final long now = mSystemClock.uptimeMillis(); - final long newStart = MathUtils.constrain(start, 0, now); - SystemProperties.set(property, Long.toString(newStart)); + return constrain(start, 0, now); } public void saveMitigationCountToMetadata() { diff --git a/packages/CrashRecovery/services/java/com/android/server/RescueParty.java b/packages/CrashRecovery/services/java/com/android/server/RescueParty.java index 5d03ef578995..6766afc5e45a 100644 --- a/packages/CrashRecovery/services/java/com/android/server/RescueParty.java +++ b/packages/CrashRecovery/services/java/com/android/server/RescueParty.java @@ -37,6 +37,7 @@ import android.os.SystemProperties; import android.os.UserHandle; import android.provider.DeviceConfig; import android.provider.Settings; +import android.sysprop.CrashRecoveryProperties; import android.text.TextUtils; import android.util.ArraySet; import android.util.ExceptionUtils; @@ -75,12 +76,6 @@ import java.util.concurrent.TimeUnit; */ public class RescueParty { @VisibleForTesting - static final String PROP_ENABLE_RESCUE = "persist.sys.enable_rescue"; - static final String PROP_ATTEMPTING_FACTORY_RESET = "sys.attempting_factory_reset"; - static final String PROP_ATTEMPTING_REBOOT = "sys.attempting_reboot"; - static final String PROP_MAX_RESCUE_LEVEL_ATTEMPTED = "sys.max_rescue_level_attempted"; - static final String PROP_LAST_FACTORY_RESET_TIME_MS = "persist.sys.last_factory_reset"; - @VisibleForTesting static final int LEVEL_NONE = 0; @VisibleForTesting static final int LEVEL_RESET_SETTINGS_UNTRUSTED_DEFAULTS = 1; @@ -93,8 +88,6 @@ public class RescueParty { @VisibleForTesting static final int LEVEL_FACTORY_RESET = 5; @VisibleForTesting - static final String PROP_RESCUE_BOOT_COUNT = "sys.rescue_boot_count"; - @VisibleForTesting static final String TAG = "RescueParty"; @VisibleForTesting static final long DEFAULT_OBSERVING_DURATION_MS = TimeUnit.DAYS.toMillis(2); @@ -131,7 +124,7 @@ public class RescueParty { private static boolean isDisabled() { // Check if we're explicitly enabled for testing - if (SystemProperties.getBoolean(PROP_ENABLE_RESCUE, false)) { + if (CrashRecoveryProperties.enableRescueParty().orElse(false)) { return false; } @@ -177,11 +170,11 @@ public class RescueParty { } static boolean isFactoryResetPropertySet() { - return SystemProperties.getBoolean(PROP_ATTEMPTING_FACTORY_RESET, false); + return CrashRecoveryProperties.attemptingFactoryReset().orElse(false); } static boolean isRebootPropertySet() { - return SystemProperties.getBoolean(PROP_ATTEMPTING_REBOOT, false); + return CrashRecoveryProperties.attemptingReboot().orElse(false); } /** @@ -440,7 +433,7 @@ public class RescueParty { case LEVEL_WARM_REBOOT: // Request the reboot from a separate thread to avoid deadlock on PackageWatchdog // when device shutting down. - SystemProperties.set(PROP_ATTEMPTING_REBOOT, "true"); + CrashRecoveryProperties.attemptingReboot(true); runnable = () -> { try { PowerManager pm = context.getSystemService(PowerManager.class); @@ -462,9 +455,9 @@ public class RescueParty { if (isRebootPropertySet()) { break; } - SystemProperties.set(PROP_ATTEMPTING_FACTORY_RESET, "true"); + CrashRecoveryProperties.attemptingFactoryReset(true); long now = System.currentTimeMillis(); - SystemProperties.set(PROP_LAST_FACTORY_RESET_TIME_MS, Long.toString(now)); + CrashRecoveryProperties.lastFactoryResetTimeMs(now); runnable = new Runnable() { @Override public void run() { @@ -514,10 +507,10 @@ public class RescueParty { private static void resetAllSettingsIfNecessary(Context context, int mode, int level) throws Exception { // No need to reset Settings again if they are already reset in the current level once. - if (SystemProperties.getInt(PROP_MAX_RESCUE_LEVEL_ATTEMPTED, LEVEL_NONE) >= level) { + if (CrashRecoveryProperties.maxRescueLevelAttempted().orElse(LEVEL_NONE) >= level) { return; } - SystemProperties.set(PROP_MAX_RESCUE_LEVEL_ATTEMPTED, Integer.toString(level)); + CrashRecoveryProperties.maxRescueLevelAttempted(level); // Try our best to reset all settings possible, and once finished // rethrow any exception that we encountered Exception res = null; @@ -732,7 +725,7 @@ public class RescueParty { * Will return {@code false} if a factory reset was already offered recently. */ private boolean shouldThrottleReboot() { - Long lastResetTime = SystemProperties.getLong(PROP_LAST_FACTORY_RESET_TIME_MS, 0); + Long lastResetTime = CrashRecoveryProperties.lastFactoryResetTimeMs().orElse(0L); long now = System.currentTimeMillis(); long throttleDurationMin = SystemProperties.getLong(PROP_THROTTLE_DURATION_MIN_FLAG, DEFAULT_FACTORY_RESET_THROTTLE_DURATION_MIN); diff --git a/packages/CrashRecovery/services/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/packages/CrashRecovery/services/java/com/android/server/rollback/RollbackPackageHealthObserver.java index 50322f09640f..dd74a2a978b2 100644 --- a/packages/CrashRecovery/services/java/com/android/server/rollback/RollbackPackageHealthObserver.java +++ b/packages/CrashRecovery/services/java/com/android/server/rollback/RollbackPackageHealthObserver.java @@ -34,6 +34,7 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.PowerManager; import android.os.SystemProperties; +import android.sysprop.CrashRecoveryProperties; import android.util.ArraySet; import android.util.Slog; import android.util.SparseArray; @@ -70,7 +71,6 @@ import java.util.function.Consumer; final class RollbackPackageHealthObserver implements PackageHealthObserver { private static final String TAG = "RollbackPackageHealthObserver"; private static final String NAME = "rollback-observer"; - private static final String PROP_ATTEMPTING_REBOOT = "sys.attempting_reboot"; private static final int PERSISTENT_MASK = ApplicationInfo.FLAG_PERSISTENT | ApplicationInfo.FLAG_SYSTEM; @@ -450,7 +450,7 @@ final class RollbackPackageHealthObserver implements PackageHealthObserver { markStagedSessionHandled(rollback.getRollbackId()); // Wait for all pending staged sessions to get handled before rebooting. if (isPendingStagedSessionsEmpty()) { - SystemProperties.set(PROP_ATTEMPTING_REBOOT, "true"); + CrashRecoveryProperties.attemptingReboot(true); mContext.getSystemService(PowerManager.class).reboot("Rollback staged install"); } } diff --git a/services/tests/mockingservicestests/Android.bp b/services/tests/mockingservicestests/Android.bp index 321d945e9c15..2dd4b62ae7f4 100644 --- a/services/tests/mockingservicestests/Android.bp +++ b/services/tests/mockingservicestests/Android.bp @@ -55,6 +55,7 @@ android_test { "mockito-target-extended-minus-junit4", "platform-compat-test-rules", "platform-test-annotations", + "PlatformProperties", "service-blobstore", "service-jobscheduler", "service-permission.impl", diff --git a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java index 7b771aff0055..2d065e263a6f 100644 --- a/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/RescuePartyTest.java @@ -45,6 +45,7 @@ import android.os.SystemProperties; import android.os.UserHandle; import android.provider.DeviceConfig; import android.provider.Settings; +import android.sysprop.CrashRecoveryProperties; import android.util.ArraySet; import com.android.dx.mockito.inline.extended.ExtendedMockito; @@ -95,7 +96,6 @@ public class RescuePartyTest { "persist.device_config.configuration.disable_rescue_party"; private static final String PROP_DISABLE_FACTORY_RESET_FLAG = "persist.device_config.configuration.disable_rescue_party_factory_reset"; - private static final String PROP_LAST_FACTORY_RESET_TIME_MS = "persist.sys.last_factory_reset"; private static final int THROTTLING_DURATION_MIN = 10; @@ -211,8 +211,8 @@ public class RescuePartyTest { doReturn(CURRENT_NETWORK_TIME_MILLIS).when(() -> RescueParty.getElapsedRealtime()); - SystemProperties.set(RescueParty.PROP_RESCUE_BOOT_COUNT, Integer.toString(0)); - SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(true)); + CrashRecoveryProperties.rescueBootCount(0); + CrashRecoveryProperties.enableRescueParty(true); SystemProperties.set(PROP_DEVICE_CONFIG_DISABLE_FLAG, Boolean.toString(false)); } @@ -255,7 +255,7 @@ public class RescuePartyTest { noteBoot(4); assertTrue(RescueParty.isRebootPropertySet()); - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); noteBoot(5); assertTrue(RescueParty.isFactoryResetPropertySet()); } @@ -280,7 +280,7 @@ public class RescuePartyTest { noteAppCrash(4, true); assertTrue(RescueParty.isRebootPropertySet()); - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); noteAppCrash(5, true); assertTrue(RescueParty.isFactoryResetPropertySet()); } @@ -438,7 +438,7 @@ public class RescuePartyTest { noteBoot(i + 1); } assertFalse(RescueParty.isFactoryResetPropertySet()); - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); noteBoot(LEVEL_FACTORY_RESET + 1); assertTrue(RescueParty.isAttemptingFactoryReset()); assertTrue(RescueParty.isFactoryResetPropertySet()); @@ -456,7 +456,7 @@ public class RescuePartyTest { noteBoot(mitigationCount++); assertFalse(RescueParty.isFactoryResetPropertySet()); noteBoot(mitigationCount++); - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); noteBoot(mitigationCount + 1); assertTrue(RescueParty.isAttemptingFactoryReset()); assertTrue(RescueParty.isFactoryResetPropertySet()); @@ -464,10 +464,10 @@ public class RescuePartyTest { @Test public void testThrottlingOnBootFailures() { - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); long now = System.currentTimeMillis(); long beforeTimeout = now - TimeUnit.MINUTES.toMillis(THROTTLING_DURATION_MIN - 1); - SystemProperties.set(PROP_LAST_FACTORY_RESET_TIME_MS, Long.toString(beforeTimeout)); + CrashRecoveryProperties.lastFactoryResetTimeMs(beforeTimeout); for (int i = 1; i <= LEVEL_FACTORY_RESET; i++) { noteBoot(i); } @@ -476,10 +476,10 @@ public class RescuePartyTest { @Test public void testThrottlingOnAppCrash() { - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); long now = System.currentTimeMillis(); long beforeTimeout = now - TimeUnit.MINUTES.toMillis(THROTTLING_DURATION_MIN - 1); - SystemProperties.set(PROP_LAST_FACTORY_RESET_TIME_MS, Long.toString(beforeTimeout)); + CrashRecoveryProperties.lastFactoryResetTimeMs(beforeTimeout); for (int i = 0; i <= LEVEL_FACTORY_RESET; i++) { noteAppCrash(i + 1, true); } @@ -488,10 +488,10 @@ public class RescuePartyTest { @Test public void testNotThrottlingAfterTimeoutOnBootFailures() { - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); long now = System.currentTimeMillis(); long afterTimeout = now - TimeUnit.MINUTES.toMillis(THROTTLING_DURATION_MIN + 1); - SystemProperties.set(PROP_LAST_FACTORY_RESET_TIME_MS, Long.toString(afterTimeout)); + CrashRecoveryProperties.lastFactoryResetTimeMs(afterTimeout); for (int i = 1; i <= LEVEL_FACTORY_RESET; i++) { noteBoot(i); } @@ -499,10 +499,10 @@ public class RescuePartyTest { } @Test public void testNotThrottlingAfterTimeoutOnAppCrash() { - SystemProperties.set(RescueParty.PROP_ATTEMPTING_REBOOT, Boolean.toString(false)); + CrashRecoveryProperties.attemptingReboot(false); long now = System.currentTimeMillis(); long afterTimeout = now - TimeUnit.MINUTES.toMillis(THROTTLING_DURATION_MIN + 1); - SystemProperties.set(PROP_LAST_FACTORY_RESET_TIME_MS, Long.toString(afterTimeout)); + CrashRecoveryProperties.lastFactoryResetTimeMs(afterTimeout); for (int i = 0; i <= LEVEL_FACTORY_RESET; i++) { noteAppCrash(i + 1, true); } @@ -525,26 +525,26 @@ public class RescuePartyTest { @Test public void testExplicitlyEnablingAndDisablingRescue() { - SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(false)); + CrashRecoveryProperties.enableRescueParty(false); SystemProperties.set(PROP_DISABLE_RESCUE, Boolean.toString(true)); assertEquals(RescuePartyObserver.getInstance(mMockContext).execute(sFailingPackage, PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 1), false); - SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(true)); + CrashRecoveryProperties.enableRescueParty(true); assertTrue(RescuePartyObserver.getInstance(mMockContext).execute(sFailingPackage, PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 1)); } @Test public void testDisablingRescueByDeviceConfigFlag() { - SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(false)); + CrashRecoveryProperties.enableRescueParty(false); SystemProperties.set(PROP_DEVICE_CONFIG_DISABLE_FLAG, Boolean.toString(true)); assertEquals(RescuePartyObserver.getInstance(mMockContext).execute(sFailingPackage, PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING, 1), false); // Restore the property value initialized in SetUp() - SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(true)); + CrashRecoveryProperties.enableRescueParty(true); SystemProperties.set(PROP_DEVICE_CONFIG_DISABLE_FLAG, Boolean.toString(false)); } diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java index d7fa124623ce..755636aef7ed 100644 --- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java +++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java @@ -70,6 +70,7 @@ import org.mockito.stubbing.Answer; import java.io.File; import java.io.FileOutputStream; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -107,10 +108,13 @@ public class PackageWatchdogTest { private ConnectivityModuleConnector mConnectivityModuleConnector; @Mock private PackageManager mMockPackageManager; + // Mock only sysprop apis + private PackageWatchdog.BootThreshold mSpyBootThreshold; @Captor private ArgumentCaptor<ConnectivityModuleHealthListener> mConnectivityModuleCallbackCaptor; private MockitoSession mSession; private HashMap<String, String> mSystemSettingsMap; + private HashMap<String, String> mCrashRecoveryPropertiesMap; private boolean retry(Supplier<Boolean> supplier) throws Exception { for (int i = 0; i < RETRY_MAX_COUNT; ++i) { @@ -1416,6 +1420,8 @@ public class PackageWatchdogTest { PackageWatchdog watchdog = new PackageWatchdog(mSpyContext, policyFile, handler, handler, controller, mConnectivityModuleConnector, mTestClock); + mockCrashRecoveryProperties(watchdog); + // Verify controller is not automatically started assertThat(controller.mIsEnabled).isFalse(); if (withPackagesReady) { @@ -1432,6 +1438,71 @@ public class PackageWatchdogTest { return watchdog; } + // Mock CrashRecoveryProperties as they cannot be accessed due to SEPolicy restrictions + private void mockCrashRecoveryProperties(PackageWatchdog watchdog) { + try { + mSpyBootThreshold = spy(watchdog.new BootThreshold( + PackageWatchdog.DEFAULT_BOOT_LOOP_TRIGGER_COUNT, + PackageWatchdog.DEFAULT_BOOT_LOOP_TRIGGER_WINDOW_MS)); + mCrashRecoveryPropertiesMap = new HashMap<>(); + + doAnswer((Answer<Integer>) invocationOnMock -> { + String storedValue = mCrashRecoveryPropertiesMap + .getOrDefault("crashrecovery.rescue_boot_count", "0"); + return Integer.parseInt(storedValue); + }).when(mSpyBootThreshold).getCount(); + doAnswer((Answer<Void>) invocationOnMock -> { + int count = invocationOnMock.getArgument(0); + mCrashRecoveryPropertiesMap.put("crashrecovery.rescue_boot_count", + Integer.toString(count)); + return null; + }).when(mSpyBootThreshold).setCount(anyInt()); + + doAnswer((Answer<Integer>) invocationOnMock -> { + String storedValue = mCrashRecoveryPropertiesMap + .getOrDefault("crashrecovery.boot_mitigation_count", "0"); + return Integer.parseInt(storedValue); + }).when(mSpyBootThreshold).getMitigationCount(); + doAnswer((Answer<Void>) invocationOnMock -> { + int count = invocationOnMock.getArgument(0); + mCrashRecoveryPropertiesMap.put("crashrecovery.boot_mitigation_count", + Integer.toString(count)); + return null; + }).when(mSpyBootThreshold).setMitigationCount(anyInt()); + + doAnswer((Answer<Long>) invocationOnMock -> { + String storedValue = mCrashRecoveryPropertiesMap + .getOrDefault("crashrecovery.rescue_boot_start", "0"); + return Long.parseLong(storedValue); + }).when(mSpyBootThreshold).getStart(); + doAnswer((Answer<Void>) invocationOnMock -> { + long count = invocationOnMock.getArgument(0); + mCrashRecoveryPropertiesMap.put("crashrecovery.rescue_boot_start", + Long.toString(count)); + return null; + }).when(mSpyBootThreshold).setStart(anyLong()); + + doAnswer((Answer<Long>) invocationOnMock -> { + String storedValue = mCrashRecoveryPropertiesMap + .getOrDefault("crashrecovery.boot_mitigation_start", "0"); + return Long.parseLong(storedValue); + }).when(mSpyBootThreshold).getMitigationStart(); + doAnswer((Answer<Void>) invocationOnMock -> { + long count = invocationOnMock.getArgument(0); + mCrashRecoveryPropertiesMap.put("crashrecovery.boot_mitigation_start", + Long.toString(count)); + return null; + }).when(mSpyBootThreshold).setMitigationStart(anyLong()); + + Field mBootThresholdField = watchdog.getClass().getDeclaredField("mBootThreshold"); + mBootThresholdField.setAccessible(true); + mBootThresholdField.set(watchdog, mSpyBootThreshold); + } catch (Exception e) { + // tests will fail, just printing the error + System.out.println("Error detected while spying BootThreshold" + e.getMessage()); + } + } + private static class TestObserver implements PackageHealthObserver { private final String mName; private int mImpact; |