diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java
index ec40971c38ee2..0d17c13fcae1b 100644
--- a/services/core/java/com/android/server/PackageWatchdog.java
+++ b/services/core/java/com/android/server/PackageWatchdog.java
@@ -76,6 +76,7 @@ public class PackageWatchdog {
private static final String ATTR_VERSION = "version";
private static final String ATTR_NAME = "name";
private static final String ATTR_DURATION = "duration";
+ private static final String ATTR_PASSED_HEALTH_CHECK = "passed-health-check";
private static PackageWatchdog sPackageWatchdog;
@@ -102,7 +103,7 @@ public class PackageWatchdog {
// 0 if mPackageCleanup not running.
private long mDurationAtLastReschedule;
- // TODO(zezeozue): Remove redundant context param
+ // TODO(b/120598832): Remove redundant context param
private PackageWatchdog(Context context) {
mContext = context;
mPolicyFile = new AtomicFile(new File(new File(Environment.getDataDirectory(), "system"),
@@ -161,21 +162,29 @@ public class PackageWatchdog {
* Starts observing the health of the {@code packages} for {@code observer} and notifies
* {@code observer} of any package failures within the monitoring duration.
*
+ * If monitoring a package with {@code withExplicitHealthCheck}, at the end of the monitoring
+ * duration if {@link #onExplicitHealthCheckPassed} was never called,
+ * {@link PackageHealthObserver#execute} will be called as if the package failed.
+ *
*
If {@code observer} is already monitoring a package in {@code packageNames},
- * the monitoring window of that package will be reset to {@code durationMs}.
+ * the monitoring window of that package will be reset to {@code durationMs} and the health
+ * check state will be reset to a default depending on {@code withExplictHealthCheck}.
*
* @throws IllegalArgumentException if {@code packageNames} is empty
* or {@code durationMs} is less than 1
*/
public void startObservingHealth(PackageHealthObserver observer, List packageNames,
- long durationMs) {
+ long durationMs, boolean withExplicitHealthCheck) {
if (packageNames.isEmpty() || durationMs < 1) {
throw new IllegalArgumentException("Observation not started, no packages specified"
+ "or invalid duration");
}
List packages = new ArrayList<>();
for (int i = 0; i < packageNames.size(); i++) {
- packages.add(new MonitoredPackage(packageNames.get(i), durationMs));
+ // When observing packages withExplicitHealthCheck,
+ // MonitoredPackage#mHasExplicitHealthCheckPassed will be false initially.
+ packages.add(new MonitoredPackage(packageNames.get(i), durationMs,
+ !withExplicitHealthCheck));
}
synchronized (mLock) {
ObserverInternal oldObserver = mAllObservers.get(observer.getName());
@@ -276,7 +285,38 @@ public class PackageWatchdog {
});
}
- // TODO(zezeozue): Optimize write? Maybe only write a separate smaller file?
+ /**
+ * Updates the observers monitoring {@code packageName} that explicit health check has passed.
+ *
+ * This update is strictly for registered observers at the time of the call
+ * Observers that register after this signal will have no knowledge of prior signals and will
+ * effectively behave as if the explicit health check hasn't passed for {@code packageName}.
+ *
+ *
{@code packageName} can still be considered failed if reported by
+ * {@link #onPackageFailure} before the package expires.
+ *
+ *
Triggered by components outside the system server when they are fully functional after an
+ * update.
+ */
+ public void onExplicitHealthCheckPassed(String packageName) {
+ Slog.i(TAG, "Health check passed for package: " + packageName);
+ boolean shouldUpdateFile = false;
+ synchronized (mLock) {
+ for (int observerIdx = 0; observerIdx < mAllObservers.size(); observerIdx++) {
+ ObserverInternal observer = mAllObservers.valueAt(observerIdx);
+ MonitoredPackage monitoredPackage = observer.mPackages.get(packageName);
+ if (monitoredPackage != null && !monitoredPackage.mHasPassedHealthCheck) {
+ monitoredPackage.mHasPassedHealthCheck = true;
+ shouldUpdateFile = true;
+ }
+ }
+ }
+ if (shouldUpdateFile) {
+ saveToFileAsync();
+ }
+ }
+
+ // TODO(b/120598832): Optimize write? Maybe only write a separate smaller file?
// This currently adds about 7ms extra to shutdown thread
/** Writes the package information to file during shutdown. */
public void writeNow() {
@@ -322,7 +362,7 @@ public class PackageWatchdog {
*/
boolean execute(VersionedPackage versionedPackage);
- // TODO(zezeozue): Ensure uniqueness?
+ // TODO(b/120598832): Ensure uniqueness?
/**
* Identifier for the observer, should not change across device updates otherwise the
* watchdog may drop observing packages with the old name.
@@ -505,6 +545,8 @@ public class PackageWatchdog {
out.startTag(null, TAG_PACKAGE);
out.attribute(null, ATTR_NAME, p.mName);
out.attribute(null, ATTR_DURATION, String.valueOf(p.mDurationMs));
+ out.attribute(null, ATTR_PASSED_HEALTH_CHECK,
+ String.valueOf(p.mHasPassedHealthCheck));
out.endTag(null, TAG_PACKAGE);
}
out.endTag(null, TAG_OBSERVER);
@@ -524,6 +566,8 @@ public class PackageWatchdog {
}
}
+ // TODO(b/120598832): For packages failing explicit health check, delay removal of the
+ // observer until after PackageHealthObserver#execute
/**
* Reduces the monitoring durations of all packages observed by this observer by
* {@code elapsedMs}. If any duration is less than 0, the package is removed from
@@ -574,6 +618,7 @@ public class PackageWatchdog {
if (TAG_OBSERVER.equals(parser.getName())) {
observerName = parser.getAttributeValue(null, ATTR_NAME);
if (TextUtils.isEmpty(observerName)) {
+ Slog.wtf(TAG, "Unable to read observer name");
return null;
}
}
@@ -582,17 +627,24 @@ public class PackageWatchdog {
try {
while (XmlUtils.nextElementWithin(parser, innerDepth)) {
if (TAG_PACKAGE.equals(parser.getName())) {
- String packageName = parser.getAttributeValue(null, ATTR_NAME);
- long duration = Long.parseLong(
- parser.getAttributeValue(null, ATTR_DURATION));
- if (!TextUtils.isEmpty(packageName)) {
- packages.add(new MonitoredPackage(packageName, duration));
+ try {
+ String packageName = parser.getAttributeValue(null, ATTR_NAME);
+ long duration = Long.parseLong(
+ parser.getAttributeValue(null, ATTR_DURATION));
+ boolean hasPassedHealthCheck = Boolean.parseBoolean(
+ parser.getAttributeValue(null, ATTR_PASSED_HEALTH_CHECK));
+ if (!TextUtils.isEmpty(packageName)) {
+ packages.add(new MonitoredPackage(packageName, duration,
+ hasPassedHealthCheck));
+ }
+ } catch (NumberFormatException e) {
+ Slog.wtf(TAG, "Skipping package for observer " + observerName, e);
+ continue;
}
}
}
- } catch (IOException e) {
- return null;
- } catch (XmlPullParserException e) {
+ } catch (XmlPullParserException | IOException e) {
+ Slog.wtf(TAG, "Unable to read observer " + observerName, e);
return null;
}
if (packages.isEmpty()) {
@@ -605,6 +657,8 @@ public class PackageWatchdog {
/** Represents a package along with the time it should be monitored for. */
static class MonitoredPackage {
public final String mName;
+ // Whether an explicit health check has passed
+ public boolean mHasPassedHealthCheck;
// System uptime duration to monitor package
public long mDurationMs;
// System uptime of first package failure
@@ -612,9 +666,10 @@ public class PackageWatchdog {
// Number of failures since mUptimeStartMs
private int mFailures;
- MonitoredPackage(String name, long durationMs) {
+ MonitoredPackage(String name, long durationMs, boolean hasPassedHealthCheck) {
mName = name;
mDurationMs = durationMs;
+ mHasPassedHealthCheck = hasPassedHealthCheck;
}
/**
@@ -626,7 +681,7 @@ public class PackageWatchdog {
final long now = SystemClock.uptimeMillis();
final long duration = now - mUptimeStartMs;
if (duration > TRIGGER_DURATION_MS) {
- // TODO(zezeozue): Reseting to 1 is not correct
+ // TODO(b/120598832): Reseting to 1 is not correct
// because there may be more than 1 failure in the last trigger window from now
// This is the RescueParty impl, will leave for now
mFailures = 1;
diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
index d24f21781b398..9beeff2f59e02 100644
--- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
+++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
@@ -149,7 +149,8 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
* This may cause {@code packages} to be rolled back if they crash too freqeuntly.
*/
public void startObservingHealth(List packages, long durationMs) {
- PackageWatchdog.getInstance(mContext).startObservingHealth(this, packages, durationMs);
+ PackageWatchdog.getInstance(mContext).startObservingHealth(this, packages, durationMs,
+ false /* withExplicitHealthCheck */);
}
private Pair getAvailableRollback(RollbackManager rollbackManager,
diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
index 77cd62e4cbd37..6a13252a23895 100644
--- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
+++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
@@ -30,6 +30,7 @@ import com.android.server.PackageWatchdog.PackageHealthObserver;
import com.android.server.PackageWatchdog.PackageHealthObserverImpact;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import java.io.File;
@@ -76,11 +77,12 @@ public class PackageWatchdogTest {
TestObserver observer3 = new TestObserver(OBSERVER_NAME_3);
// Start observing for observer1 which will be unregistered
- watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION);
+ watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION, false);
// Start observing for observer2 which will expire
- watchdog.startObservingHealth(observer2, Arrays.asList(APP_A, APP_B), SHORT_DURATION);
+ watchdog.startObservingHealth(observer2, Arrays.asList(APP_A, APP_B), SHORT_DURATION,
+ false);
// Start observing for observer3 which will have expiry duration reduced
- watchdog.startObservingHealth(observer3, Arrays.asList(APP_A), LONG_DURATION);
+ watchdog.startObservingHealth(observer3, Arrays.asList(APP_A), LONG_DURATION, false);
// Verify packages observed at start
// 1
@@ -143,8 +145,9 @@ public class PackageWatchdogTest {
TestObserver observer1 = new TestObserver(OBSERVER_NAME_1);
TestObserver observer2 = new TestObserver(OBSERVER_NAME_2);
- watchdog1.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION);
- watchdog1.startObservingHealth(observer2, Arrays.asList(APP_A, APP_B), SHORT_DURATION);
+ watchdog1.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION, false);
+ watchdog1.startObservingHealth(observer2, Arrays.asList(APP_A, APP_B), SHORT_DURATION,
+ false);
// Verify 2 observers are registered and saved internally
// 1
@@ -190,8 +193,8 @@ public class PackageWatchdogTest {
TestObserver observer1 = new TestObserver(OBSERVER_NAME_1);
TestObserver observer2 = new TestObserver(OBSERVER_NAME_2);
- watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION);
- watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION);
+ watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION, false);
+ watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION, false);
// Then fail APP_A below the threshold
for (int i = 0; i < TRIGGER_FAILURE_COUNT - 1; i++) {
@@ -217,8 +220,8 @@ public class PackageWatchdogTest {
TestObserver observer2 = new TestObserver(OBSERVER_NAME_2);
- watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION);
- watchdog.startObservingHealth(observer1, Arrays.asList(APP_B), SHORT_DURATION);
+ watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION, false);
+ watchdog.startObservingHealth(observer1, Arrays.asList(APP_B), SHORT_DURATION, false);
// Then fail APP_C (not observed) above the threshold
for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
@@ -252,7 +255,7 @@ public class PackageWatchdogTest {
}
};
- watchdog.startObservingHealth(observer, Arrays.asList(APP_A), SHORT_DURATION);
+ watchdog.startObservingHealth(observer, Arrays.asList(APP_A), SHORT_DURATION, false);
// Then fail APP_A (different version) above the threshold
for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
@@ -285,13 +288,13 @@ public class PackageWatchdogTest {
// Start observing for all impact observers
watchdog.startObservingHealth(observerNone, Arrays.asList(APP_A, APP_B, APP_C, APP_D),
- SHORT_DURATION);
+ SHORT_DURATION, false);
watchdog.startObservingHealth(observerHigh, Arrays.asList(APP_A, APP_B, APP_C),
- SHORT_DURATION);
+ SHORT_DURATION, false);
watchdog.startObservingHealth(observerMid, Arrays.asList(APP_A, APP_B),
- SHORT_DURATION);
+ SHORT_DURATION, false);
watchdog.startObservingHealth(observerLow, Arrays.asList(APP_A),
- SHORT_DURATION);
+ SHORT_DURATION, false);
// Then fail all apps above the threshold
for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
@@ -343,8 +346,8 @@ public class PackageWatchdogTest {
PackageHealthObserverImpact.USER_IMPACT_MEDIUM);
// Start observing for observerFirst and observerSecond with failure handling
- watchdog.startObservingHealth(observerFirst, Arrays.asList(APP_A), LONG_DURATION);
- watchdog.startObservingHealth(observerSecond, Arrays.asList(APP_A), LONG_DURATION);
+ watchdog.startObservingHealth(observerFirst, Arrays.asList(APP_A), LONG_DURATION, false);
+ watchdog.startObservingHealth(observerSecond, Arrays.asList(APP_A), LONG_DURATION, false);
// Then fail APP_A above the threshold
for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
@@ -421,8 +424,8 @@ public class PackageWatchdogTest {
PackageHealthObserverImpact.USER_IMPACT_HIGH);
// Start observing for observer1 and observer2 with failure handling
- watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION);
- watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION);
+ watchdog.startObservingHealth(observer2, Arrays.asList(APP_A), SHORT_DURATION, false);
+ watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION, false);
// Then fail APP_A above the threshold
for (int i = 0; i < TRIGGER_FAILURE_COUNT; i++) {
@@ -438,6 +441,52 @@ public class PackageWatchdogTest {
assertEquals(0, observer2.mFailedPackages.size());
}
+ // TODO: Unignore test after package failure is triggered on observer expiry with failing
+ // explicit health check
+ /**
+ * Test explicit health check status determines package failure or success on expiry
+ */
+ @Ignore
+ @Test
+ public void testPackageFailureExplicitHealthCheck() throws Exception {
+ PackageWatchdog watchdog = createWatchdog();
+ TestObserver observer1 = new TestObserver(OBSERVER_NAME_1,
+ PackageHealthObserverImpact.USER_IMPACT_HIGH);
+ TestObserver observer2 = new TestObserver(OBSERVER_NAME_2,
+ PackageHealthObserverImpact.USER_IMPACT_HIGH);
+ TestObserver observer3 = new TestObserver(OBSERVER_NAME_3,
+ PackageHealthObserverImpact.USER_IMPACT_HIGH);
+
+
+ // Start observing with explicit health checks for APP_A and APP_B respectively
+ // with observer1 and observer2
+ watchdog.startObservingHealth(observer1, Arrays.asList(APP_A), SHORT_DURATION, true);
+ watchdog.startObservingHealth(observer2, Arrays.asList(APP_B), SHORT_DURATION, true);
+ // Explicit health check passed for APP_A (observer1 is aware)
+ watchdog.onExplicitHealthCheckPassed(APP_A);
+ // Start observing APP_A with explicit health checks for observer3.
+ // Observer3 didn't exist when we got the explicit health check above, so
+ // it starts out with a non-passing explicit health check and has to wait for a pass
+ // otherwise it would be notified of APP_A failure on expiry
+ watchdog.startObservingHealth(observer3, Arrays.asList(APP_A), SHORT_DURATION, true);
+
+ // Then expire observers
+ Thread.sleep(SHORT_DURATION);
+ // Run handler so package failures are dispatched to observers
+ mTestLooper.dispatchAll();
+
+ // Verify observer1 is not notified
+ assertEquals(0, observer1.mFailedPackages.size());
+
+ // Verify observer2 is notifed because health checks for APP_B never passed
+ assertEquals(1, observer2.mFailedPackages.size());
+ assertEquals(APP_B, observer2.mFailedPackages.get(0));
+
+ // Verify observer3 is notifed because health checks for APP_A did not pass before expiry
+ assertEquals(1, observer3.mFailedPackages.size());
+ assertEquals(APP_A, observer3.mFailedPackages.get(0));
+ }
+
private PackageWatchdog createWatchdog() {
return new PackageWatchdog(InstrumentationRegistry.getContext(),
mTestLooper.getLooper());