From cdc5524f79ecbd6e7052e0526aa2ea2d7a086c58 Mon Sep 17 00:00:00 2001 From: Gavin Corkery Date: Wed, 27 Nov 2019 15:46:29 +0000 Subject: [PATCH] Add package failure flags to Package Watchdog This is a prerequisite for adding additional logging of the Watchdog-triggered rollback reason. Add flags which indicate the failure observed (native, crash, ANR, explicit health check). These will be used in the future by RollbackPackageHealthObserver to map the failure type to the (new) set of available logging metrics. Test: atest PackageWatchdogTest Bug: 146415463 Change-Id: I7e7c5e5399011e2761dada2b989a95c2013307e9 Merged-In: I7e7c5e5399011e2761dada2b989a95c2013307e9 (cherry picked from commit f305f4def072363087595a610043852127c399e2) --- .../com/android/server/PackageWatchdog.java | 31 ++++++-- .../java/com/android/server/am/AppErrors.java | 6 +- .../RollbackPackageHealthObserver.java | 5 +- .../android/server/PackageWatchdogTest.java | 73 ++++++++++++++++--- 4 files changed, 95 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java index dee89e5d5c4d5..501de7f63ebc5 100644 --- a/services/core/java/com/android/server/PackageWatchdog.java +++ b/services/core/java/com/android/server/PackageWatchdog.java @@ -56,6 +56,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; @@ -80,6 +81,22 @@ public class PackageWatchdog { static final String PROPERTY_WATCHDOG_EXPLICIT_HEALTH_CHECK_ENABLED = "watchdog_explicit_health_check_enabled"; + public static final int FAILURE_REASON_UNKNOWN = 0; + public static final int FAILURE_REASON_NATIVE_CRASH = 1; + public static final int FAILURE_REASON_EXPLICIT_HEALTH_CHECK = 2; + public static final int FAILURE_REASON_APP_CRASH = 3; + public static final int FAILURE_REASON_APP_NOT_RESPONDING = 4; + + @IntDef(prefix = { "FAILURE_REASON_" }, value = { + FAILURE_REASON_UNKNOWN, + FAILURE_REASON_NATIVE_CRASH, + FAILURE_REASON_EXPLICIT_HEALTH_CHECK, + FAILURE_REASON_APP_CRASH, + FAILURE_REASON_APP_NOT_RESPONDING + }) + @Retention(RetentionPolicy.SOURCE) + public @interface FailureReasons {} + // Duration to count package failures before it resets to 0 private static final int DEFAULT_TRIGGER_FAILURE_DURATION_MS = (int) TimeUnit.MINUTES.toMillis(1); @@ -295,14 +312,15 @@ public class PackageWatchdog { } /** - * Called when a process fails either due to a crash or ANR. + * Called when a process fails due to a crash, ANR or explicit health check. * *

For each package contained in the process, one registered observer with the least user * impact will be notified for mitigation. * *

This method could be called frequently if there is a severe problem on the device. */ - public void onPackageFailure(List packages) { + public void onPackageFailure(List packages, + @FailureReasons int failureReason) { mLongTaskHandler.post(() -> { synchronized (mLock) { if (mAllObservers.isEmpty()) { @@ -333,7 +351,7 @@ public class PackageWatchdog { // Execute action with least user impact if (currentObserverToNotify != null) { - currentObserverToNotify.execute(versionedPackage); + currentObserverToNotify.execute(versionedPackage, failureReason); } } } @@ -404,7 +422,7 @@ public class PackageWatchdog { * * @return {@code true} if action was executed successfully, {@code false} otherwise */ - boolean execute(VersionedPackage versionedPackage); + boolean execute(VersionedPackage versionedPackage, @FailureReasons int failureReason); // TODO(b/120598832): Ensure uniqueness? /** @@ -648,7 +666,8 @@ public class PackageWatchdog { // the tests don't install any packages versionedPkg = new VersionedPackage(failedPackage, 0L); } - registeredObserver.execute(versionedPkg); + registeredObserver.execute(versionedPkg, + PackageWatchdog.FAILURE_REASON_EXPLICIT_HEALTH_CHECK); } } } @@ -759,7 +778,7 @@ public class PackageWatchdog { final List pkgList = Collections.singletonList(pkg); final long failureCount = getTriggerFailureCount(); for (int i = 0; i < failureCount; i++) { - onPackageFailure(pkgList); + onPackageFailure(pkgList, FAILURE_REASON_EXPLICIT_HEALTH_CHECK); } }); } diff --git a/services/core/java/com/android/server/am/AppErrors.java b/services/core/java/com/android/server/am/AppErrors.java index 1ff6f4dac724f..a4c695067139f 100644 --- a/services/core/java/com/android/server/am/AppErrors.java +++ b/services/core/java/com/android/server/am/AppErrors.java @@ -430,7 +430,8 @@ class AppErrors { RescueParty.noteAppCrash(mContext, r.uid); } - mPackageWatchdog.onPackageFailure(r.getPackageListWithVersionCode()); + mPackageWatchdog.onPackageFailure(r.getPackageListWithVersionCode(), + PackageWatchdog.FAILURE_REASON_APP_CRASH); } final int relaunchReason = r != null @@ -884,7 +885,8 @@ class AppErrors { } // Notify PackageWatchdog without the lock held if (packageList != null) { - mPackageWatchdog.onPackageFailure(packageList); + mPackageWatchdog.onPackageFailure(packageList, + PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING); } } diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java index 9c19aeccd59a3..3673b8834a80b 100644 --- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java +++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java @@ -41,6 +41,7 @@ import android.util.StatsLog; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; import com.android.server.PackageWatchdog; +import com.android.server.PackageWatchdog.FailureReasons; import com.android.server.PackageWatchdog.PackageHealthObserver; import com.android.server.PackageWatchdog.PackageHealthObserverImpact; @@ -106,7 +107,7 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve } @Override - public boolean execute(VersionedPackage failedPackage) { + public boolean execute(VersionedPackage failedPackage, @FailureReasons int rollbackReason) { RollbackManager rollbackManager = mContext.getSystemService(RollbackManager.class); VersionedPackage moduleMetadataPackage = getModuleMetadataPackage(); RollbackInfo rollback = getAvailableRollback(rollbackManager, failedPackage); @@ -371,7 +372,7 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve mNumberOfNativeCrashPollsRemaining--; // Check if native watchdog reported a crash if ("1".equals(SystemProperties.get("ro.init.updatable_crashing"))) { - execute(getModuleMetadataPackage()); + execute(getModuleMetadataPackage(), PackageWatchdog.FAILURE_REASON_NATIVE_CRASH); // we stop polling after an attempt to execute rollback, regardless of whether the // attempt succeeds or not } else { diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java index 232b5cb170233..31c6996217f24 100644 --- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java +++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java @@ -269,7 +269,8 @@ public class PackageWatchdogTest { // Then fail APP_A below the threshold for (int i = 0; i < watchdog.getTriggerFailureCount() - 1; i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers @@ -296,7 +297,8 @@ public class PackageWatchdogTest { // Then fail APP_C (not observed) above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_C, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_C, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers @@ -331,7 +333,8 @@ public class PackageWatchdogTest { // Then fail APP_A (different version) above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { watchdog.onPackageFailure(Arrays.asList( - new VersionedPackage(APP_A, differentVersionCode))); + new VersionedPackage(APP_A, differentVersionCode)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers @@ -372,7 +375,8 @@ public class PackageWatchdogTest { watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE), new VersionedPackage(APP_B, VERSION_CODE), new VersionedPackage(APP_C, VERSION_CODE), - new VersionedPackage(APP_D, VERSION_CODE))); + new VersionedPackage(APP_D, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers @@ -422,7 +426,8 @@ public class PackageWatchdogTest { // Then fail APP_A above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers mTestLooper.dispatchAll(); @@ -439,7 +444,8 @@ public class PackageWatchdogTest { // Then fail APP_A again above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers mTestLooper.dispatchAll(); @@ -456,7 +462,8 @@ public class PackageWatchdogTest { // Then fail APP_A again above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers mTestLooper.dispatchAll(); @@ -473,7 +480,8 @@ public class PackageWatchdogTest { // Then fail APP_A again above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers mTestLooper.dispatchAll(); @@ -500,7 +508,8 @@ public class PackageWatchdogTest { // Then fail APP_A above the threshold for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { - watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE))); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_UNKNOWN); } // Run handler so package failures are dispatched to observers @@ -746,6 +755,44 @@ public class PackageWatchdogTest { assertEquals(APP_A, observer.mFailedPackages.get(0)); } + /** Test that observers execute correctly for different failure reasons */ + @Test + public void testFailureReasons() { + PackageWatchdog watchdog = createWatchdog(); + TestObserver observer1 = new TestObserver(OBSERVER_NAME_1); + TestObserver observer2 = new TestObserver(OBSERVER_NAME_2); + TestObserver observer3 = new TestObserver(OBSERVER_NAME_3); + TestObserver observer4 = new TestObserver(OBSERVER_NAME_4); + + watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION); + watchdog.startObservingHealth(observer2, Arrays.asList(APP_B), SHORT_DURATION); + watchdog.startObservingHealth(observer3, Arrays.asList(APP_C), SHORT_DURATION); + watchdog.startObservingHealth(observer4, Arrays.asList(APP_D), SHORT_DURATION); + + for (int i = 0; i < watchdog.getTriggerFailureCount(); i++) { + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_A, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_NATIVE_CRASH); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_B, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_EXPLICIT_HEALTH_CHECK); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_C, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_APP_CRASH); + watchdog.onPackageFailure(Arrays.asList(new VersionedPackage(APP_D, VERSION_CODE)), + PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING); + } + + // Run handler so requests are dispatched to the controller + mTestLooper.dispatchAll(); + + assertTrue(observer1.getLastFailureReason() + == PackageWatchdog.FAILURE_REASON_NATIVE_CRASH); + assertTrue(observer2.getLastFailureReason() + == PackageWatchdog.FAILURE_REASON_EXPLICIT_HEALTH_CHECK); + assertTrue(observer3.getLastFailureReason() + == PackageWatchdog.FAILURE_REASON_APP_CRASH); + assertTrue(observer4.getLastFailureReason() + == PackageWatchdog.FAILURE_REASON_APP_NOT_RESPONDING); + } + private void adoptShellPermissions(String... permissions) { InstrumentationRegistry .getInstrumentation() @@ -801,6 +848,7 @@ public class PackageWatchdogTest { private static class TestObserver implements PackageHealthObserver { private final String mName; private int mImpact; + private int mLastFailureReason; final List mFailedPackages = new ArrayList<>(); TestObserver(String name) { @@ -817,14 +865,19 @@ public class PackageWatchdogTest { return mImpact; } - public boolean execute(VersionedPackage versionedPackage) { + public boolean execute(VersionedPackage versionedPackage, int failureReason) { mFailedPackages.add(versionedPackage.getPackageName()); + mLastFailureReason = failureReason; return true; } public String getName() { return mName; } + + public int getLastFailureReason() { + return mLastFailureReason; + } } private static class TestController extends ExplicitHealthCheckController {