Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9ef3ea11 authored by Hongyi Zhang's avatar Hongyi Zhang
Browse files

Fix RescueParty querying DeviceConfig before SettingsProvider ready

It turns out that PackageWatchdog will invoke
RescuePartyObserver.onBootloop very early in the boot sequence(in startBootstrapServices), thus calling into isDisabled() which calls into DeviceConfig.getBoolean before SettingProvider is initialized(in startOtherServices), thus causing an NPE thrown from SettingsProvider.

Since we registered namespace "configuration" in
SettingsToProperiesMapper.java, DeviceConfig flags from "configuration"
will be synced to corresponding persistent sys props. Thus instead of querying DeviceConfig, we can query the system property which is ready at the time that onBootloop is called.

Note that the race condition won't cause a problem to DeviceConfig reset
on bootloop, since:
   1. executeRescueLevel will swallow the exception when reset is
   attempted before SettingsProvider initialized. Thus won't crashing
   system server.
   2. onSettingsProviderPublished will retry the reset based the
   incremented rescue level set in executeBootLoopMitigation

Bug:152346219
Test: 1. atest RescuePartyTest 2. manually turn on the flag via adb
device_config and observe RescuePary is disable during systemui
crashloop.

Change-Id: Idde46ee5872bc3107cbc8a2ab05b3e1de028d998
parent 75463684
Loading
Loading
Loading
Loading
+3 −4
Original line number Diff line number Diff line
@@ -98,8 +98,8 @@ public class RescueParty {

    private static final String PROP_DISABLE_RESCUE = "persist.sys.disable_rescue";
    private static final String PROP_VIRTUAL_DEVICE = "ro.hardware.virtual_device";

    private static final String DEVICE_CONFIG_DISABLE_FLAG = "disable_rescue_party";
    private static final String PROP_DEVICE_CONFIG_DISABLE_FLAG =
            "persist.device_config.configuration.disable_rescue_party";

    private static final int PERSISTENT_MASK = ApplicationInfo.FLAG_PERSISTENT
            | ApplicationInfo.FLAG_SYSTEM;
@@ -118,8 +118,7 @@ public class RescueParty {

        // We're disabled if the DeviceConfig disable flag is set to true.
        // This is in case that an emergency rollback of the feature is needed.
        if (DeviceConfig.getBoolean(
                DeviceConfig.NAMESPACE_CONFIGURATION, DEVICE_CONFIG_DISABLE_FLAG, false)) {
        if (SystemProperties.getBoolean(PROP_DEVICE_CONFIG_DISABLE_FLAG, false)) {
            Slog.v(TAG, "Disabled because of DeviceConfig flag");
            return true;
        }
+5 −13
Original line number Diff line number Diff line
@@ -79,7 +79,8 @@ public class RescuePartyTest {
    private static final String CALLING_PACKAGE2 = "com.package.name2";
    private static final String NAMESPACE1 = "namespace1";
    private static final String NAMESPACE2 = "namespace2";
    private static final String DISABLE_RESCUE_PARTY_FLAG = "disable_rescue_party";
    private static final String PROP_DEVICE_CONFIG_DISABLE_FLAG =
            "persist.device_config.configuration.disable_rescue_party";

    private MockitoSession mSession;
    private HashMap<String, String> mSystemSettingsMap;
@@ -172,6 +173,7 @@ public class RescuePartyTest {
                Integer.toString(RescueParty.LEVEL_NONE));
        SystemProperties.set(RescueParty.PROP_RESCUE_BOOT_COUNT, Integer.toString(0));
        SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(true));
        SystemProperties.set(PROP_DEVICE_CONFIG_DISABLE_FLAG, Boolean.toString(false));
    }

    @After
@@ -317,13 +319,6 @@ public class RescuePartyTest {

    @Test
    public void testExplicitlyEnablingAndDisablingRescue() {
        // mock the DeviceConfig get call to avoid hitting
        // android.permission.READ_DEVICE_CONFIG when calling real DeviceConfig.
        doReturn(true)
                .when(() -> DeviceConfig.getBoolean(
                    eq(DeviceConfig.NAMESPACE_CONFIGURATION),
                    eq(DISABLE_RESCUE_PARTY_FLAG),
                    eq(false)));
        SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(false));
        SystemProperties.set(PROP_DISABLE_RESCUE, Boolean.toString(true));
        assertEquals(RescuePartyObserver.getInstance(mMockContext).execute(sFailingPackage,
@@ -336,18 +331,15 @@ public class RescuePartyTest {

    @Test
    public void testDisablingRescueByDeviceConfigFlag() {
        doReturn(true)
                .when(() -> DeviceConfig.getBoolean(
                    eq(DeviceConfig.NAMESPACE_CONFIGURATION),
                    eq(DISABLE_RESCUE_PARTY_FLAG),
                    eq(false)));
        SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(false));
        SystemProperties.set(PROP_DEVICE_CONFIG_DISABLE_FLAG, Boolean.toString(true));

        assertEquals(RescuePartyObserver.getInstance(mMockContext).execute(sFailingPackage,
                PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING), false);

        // Restore the property value initalized in SetUp()
        SystemProperties.set(RescueParty.PROP_ENABLE_RESCUE, Boolean.toString(true));
        SystemProperties.set(PROP_DEVICE_CONFIG_DISABLE_FLAG, Boolean.toString(false));
    }

    @Test