Merge "Extend PackageWatchdog with explicit health checks" (101ac589) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/PackageWatchdog.java

+71 −16

Original line number	Diff line number	Diff line
		@@ -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.
		*
		* <p>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.
		*
		* <p>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<String> 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<MonitoredPackage> 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.
		*
		* <p> 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}.
		*
		* <p> {@code packageName} can still be considered failed if reported by
		* {@link #onPackageFailure} before the package expires.
		*
		* <p> 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())) {
		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));
		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;

services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java

+2 −1

Original line number	Diff line number	Diff line
		@@ -166,7 +166,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<String> packages, long durationMs) {
		PackageWatchdog.getInstance(mContext).startObservingHealth(this, packages, durationMs);
		PackageWatchdog.getInstance(mContext).startObservingHealth(this, packages, durationMs,
		false /* withExplicitHealthCheck */);
		}

		/** Verifies the rollback state after a reboot. */

tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java

+67 −18

Original line number	Diff line number	Diff line
		@@ -31,6 +31,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;
		@@ -77,11 +78,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
		@@ -144,8 +146,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
		@@ -191,8 +194,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++) {
		@@ -218,8 +221,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++) {
		@@ -253,7 +256,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++) {
		@@ -286,13 +289,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++) {
		@@ -344,8 +347,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++) {
		@@ -422,8 +425,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++) {
		@@ -439,6 +442,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());