Merge "Fixed PackageWatchdog health check state" into qt-dev
diff --git a/services/core/java/com/android/server/ExplicitHealthCheckController.java b/services/core/java/com/android/server/ExplicitHealthCheckController.java
index 27ad208..19ab33e 100644
--- a/services/core/java/com/android/server/ExplicitHealthCheckController.java
+++ b/services/core/java/com/android/server/ExplicitHealthCheckController.java
@@ -35,7 +35,9 @@
import android.os.UserHandle;
import android.service.watchdog.ExplicitHealthCheckService;
import android.service.watchdog.IExplicitHealthCheckService;
+import android.service.watchdog.PackageInfo;
import android.text.TextUtils;
+import android.util.ArraySet;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
@@ -69,7 +71,7 @@
// To prevent deadlocks between the controller and watchdog threads, we have
// a lock invariant to ALWAYS acquire the PackageWatchdog#mLock before #mLock in this class.
// It's easier to just NOT hold #mLock when calling into watchdog code on this consumer.
- @GuardedBy("mLock") @Nullable private Consumer<List<String>> mSupportedConsumer;
+ @GuardedBy("mLock") @Nullable private Consumer<List<PackageInfo>> mSupportedConsumer;
// Called everytime we need to notify the watchdog to sync requests between itself and the
// health check service. In practice, should never be null after it has been #setEnabled.
// To prevent deadlocks between the controller and watchdog threads, we have
@@ -104,7 +106,7 @@
* ensure a happens-before relationship of the set parameters and visibility on other threads.
*/
public void setCallbacks(Consumer<String> passedConsumer,
- Consumer<List<String>> supportedConsumer, Runnable notifySyncRunnable) {
+ Consumer<List<PackageInfo>> supportedConsumer, Runnable notifySyncRunnable) {
synchronized (mLock) {
if (mPassedConsumer != null || mSupportedConsumer != null
|| mNotifySyncRunnable != null) {
@@ -144,14 +146,18 @@
return;
}
- getSupportedPackages(supportedPackages -> {
+ getSupportedPackages(supportedPackageInfos -> {
// Notify the watchdog without lock held
- mSupportedConsumer.accept(supportedPackages);
+ mSupportedConsumer.accept(supportedPackageInfos);
getRequestedPackages(previousRequestedPackages -> {
synchronized (mLock) {
// Hold lock so requests and cancellations are sent atomically.
// It is important we don't mix requests from multiple threads.
+ Set<String> supportedPackages = new ArraySet<>();
+ for (PackageInfo info : supportedPackageInfos) {
+ supportedPackages.add(info.getPackageName());
+ }
// Note, this may modify newRequestedPackages
newRequestedPackages.retainAll(supportedPackages);
@@ -229,7 +235,7 @@
* Returns the packages that we can request explicit health checks for.
* The packages will be returned to the {@code consumer}.
*/
- private void getSupportedPackages(Consumer<List<String>> consumer) {
+ private void getSupportedPackages(Consumer<List<PackageInfo>> consumer) {
synchronized (mLock) {
if (!prepareServiceLocked("get health check supported packages")) {
return;
@@ -238,7 +244,8 @@
Slog.d(TAG, "Getting health check supported packages");
try {
mRemoteService.getSupportedPackages(new RemoteCallback(result -> {
- List<String> packages = result.getStringArrayList(EXTRA_SUPPORTED_PACKAGES);
+ List<PackageInfo> packages =
+ result.getParcelableArrayList(EXTRA_SUPPORTED_PACKAGES);
Slog.i(TAG, "Explicit health check supported packages " + packages);
consumer.accept(packages);
}));
diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java
index 0c681df..7d0d834 100644
--- a/services/core/java/com/android/server/PackageWatchdog.java
+++ b/services/core/java/com/android/server/PackageWatchdog.java
@@ -27,6 +27,7 @@
import android.os.Handler;
import android.os.Looper;
import android.os.SystemClock;
+import android.service.watchdog.PackageInfo;
import android.text.TextUtils;
import android.util.ArrayMap;
import android.util.ArraySet;
@@ -57,6 +58,7 @@
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
@@ -102,16 +104,10 @@
private boolean mIsHealthCheckEnabled = true;
@GuardedBy("mLock")
private boolean mIsPackagesReady;
- // SystemClock#uptimeMillis when we last executed #pruneObservers.
+ // SystemClock#uptimeMillis when we last executed #syncState
// 0 if no prune is scheduled.
@GuardedBy("mLock")
- private long mUptimeAtLastPruneMs;
- // Duration in millis that the last prune was scheduled for.
- // Used along with #mUptimeAtLastPruneMs after scheduling a prune to determine the remaining
- // duration before #pruneObservers will be executed.
- // 0 if no prune is scheduled.
- @GuardedBy("mLock")
- private long mDurationAtLastPrune;
+ private long mUptimeAtLastStateSync;
private PackageWatchdog(Context context) {
// Needs to be constructed inline
@@ -156,7 +152,7 @@
mHealthCheckController.setCallbacks(packageName -> onHealthCheckPassed(packageName),
packages -> onSupportedPackages(packages),
() -> syncRequestsAsync());
- // Controller is initially disabled until here where we may enable it and sync requests
+ // Controller is initially disabled until here where we may enable it and sync our state
setExplicitHealthCheckEnabled(mIsHealthCheckEnabled);
}
}
@@ -173,10 +169,6 @@
if (internalObserver != null) {
internalObserver.mRegisteredObserver = observer;
}
- if (mDurationAtLastPrune == 0) {
- // Nothing running, prune
- pruneAndSchedule();
- }
}
}
@@ -214,6 +206,12 @@
packages.add(new MonitoredPackage(packageNames.get(i), durationMs, false));
}
+ // Sync before we add the new packages to the observers. This will #pruneObservers,
+ // causing any elapsed time to be deducted from all existing packages before we add new
+ // packages. This maintains the invariant that the elapsed time for ALL (new and existing)
+ // packages is the same.
+ syncState("observing new packages");
+
synchronized (mLock) {
ObserverInternal oldObserver = mAllObservers.get(observer.getName());
if (oldObserver == null) {
@@ -224,16 +222,16 @@
} else {
Slog.d(TAG, observer.getName() + " added the following "
+ "packages to monitor " + packageNames);
- oldObserver.updatePackages(packages);
+ oldObserver.updatePackagesLocked(packages);
}
}
+
+ // Register observer in case not already registered
registerHealthObserver(observer);
- // Always prune because we may have received packges requiring an earlier
- // schedule than we are currently scheduled for.
- pruneAndSchedule();
- Slog.i(TAG, "Syncing health check requests, observing packages " + packageNames);
- syncRequestsAsync();
- saveToFileAsync();
+
+ // Sync after we add the new packages to the observers. We may have received packges
+ // requiring an earlier schedule than we are currently scheduled for.
+ syncState("updated observers");
}
/**
@@ -245,7 +243,7 @@
synchronized (mLock) {
mAllObservers.remove(observer.getName());
}
- saveToFileAsync();
+ syncState("unregistering observer: " + observer.getName());
}
/**
@@ -296,7 +294,8 @@
ObserverInternal observer = mAllObservers.valueAt(oIndex);
PackageHealthObserver registeredObserver = observer.mRegisteredObserver;
if (registeredObserver != null
- && observer.onPackageFailure(versionedPackage.getPackageName())) {
+ && observer.onPackageFailureLocked(
+ versionedPackage.getPackageName())) {
int impact = registeredObserver.onHealthCheckFailed(versionedPackage);
if (impact != PackageHealthObserverImpact.USER_IMPACT_NONE
&& impact < currentObserverImpact) {
@@ -321,9 +320,11 @@
/** Writes the package information to file during shutdown. */
public void writeNow() {
synchronized (mLock) {
+ // Must only run synchronous tasks as this runs on the ShutdownThread and no other
+ // thread is guaranteed to run during shutdown.
if (!mAllObservers.isEmpty()) {
- mLongTaskHandler.removeCallbacks(this::saveToFile);
- pruneObservers(SystemClock.uptimeMillis() - mUptimeAtLastPruneMs);
+ mLongTaskHandler.removeCallbacks(this::saveToFileAsync);
+ pruneObserversLocked();
saveToFile();
Slog.i(TAG, "Last write to update package durations");
}
@@ -341,9 +342,8 @@
synchronized (mLock) {
mIsHealthCheckEnabled = enabled;
mHealthCheckController.setEnabled(enabled);
- Slog.i(TAG, "Syncing health check requests, explicit health check is "
- + (enabled ? "enabled" : "disabled"));
- syncRequestsAsync();
+ // Prune to update internal state whenever health check is enabled/disabled
+ syncState("health check state " + (enabled ? "enabled" : "disabled"));
}
}
@@ -393,9 +393,8 @@
* Serializes and syncs health check requests with the {@link ExplicitHealthCheckController}.
*/
private void syncRequestsAsync() {
- if (!mShortTaskHandler.hasCallbacks(this::syncRequests)) {
- mShortTaskHandler.post(this::syncRequests);
- }
+ mShortTaskHandler.removeCallbacks(this::syncRequests);
+ mShortTaskHandler.post(this::syncRequests);
}
/**
@@ -414,6 +413,7 @@
// Call outside lock to avoid holding lock when calling into the controller.
if (packages != null) {
+ Slog.i(TAG, "Syncing health check requests for packages: " + packages);
mHealthCheckController.syncRequests(packages);
}
}
@@ -426,86 +426,73 @@
* 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.
+ * {@link #onPackageFailureLocked} before the package expires.
*
* <p> Triggered by components outside the system server when they are fully functional after an
* update.
*/
private void onHealthCheckPassed(String packageName) {
Slog.i(TAG, "Health check passed for package: " + packageName);
- boolean shouldUpdateFile = false;
+ boolean isStateChanged = 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 (monitoredPackage != null) {
+ int oldState = monitoredPackage.getHealthCheckStateLocked();
+ int newState = monitoredPackage.tryPassHealthCheckLocked();
+ isStateChanged |= oldState != newState;
}
}
}
- // So we can unbind from the service if this was the last result we expected
- Slog.i(TAG, "Syncing health check requests, health check passed for " + packageName);
- syncRequestsAsync();
-
- if (shouldUpdateFile) {
- saveToFileAsync();
+ if (isStateChanged) {
+ syncState("health check passed for " + packageName);
}
}
- private void onSupportedPackages(List<String> supportedPackages) {
- boolean shouldUpdateFile = false;
- boolean shouldPrune = false;
+ private void onSupportedPackages(List<PackageInfo> supportedPackages) {
+ boolean isStateChanged = false;
+
+ Map<String, Long> supportedPackageTimeouts = new ArrayMap<>();
+ Iterator<PackageInfo> it = supportedPackages.iterator();
+ while (it.hasNext()) {
+ PackageInfo info = it.next();
+ supportedPackageTimeouts.put(info.getPackageName(), info.getHealthCheckTimeoutMillis());
+ }
synchronized (mLock) {
Slog.d(TAG, "Received supported packages " + supportedPackages);
Iterator<ObserverInternal> oit = mAllObservers.values().iterator();
while (oit.hasNext()) {
- ObserverInternal observer = oit.next();
- Iterator<MonitoredPackage> pit =
- observer.mPackages.values().iterator();
+ Iterator<MonitoredPackage> pit = oit.next().mPackages.values().iterator();
while (pit.hasNext()) {
MonitoredPackage monitoredPackage = pit.next();
- String packageName = monitoredPackage.mName;
- int healthCheckState = monitoredPackage.getHealthCheckState();
+ String packageName = monitoredPackage.getName();
+ int oldState = monitoredPackage.getHealthCheckStateLocked();
+ int newState;
- if (healthCheckState != MonitoredPackage.STATE_PASSED) {
- // Have to update file, we will either transition state or reduce
- // health check duration
- shouldUpdateFile = true;
-
- if (supportedPackages.contains(packageName)) {
- // Supports health check, transition to ACTIVE if not already.
- // We need to prune packages earlier than already scheduled.
- shouldPrune = true;
-
- // TODO: Get healthCheckDuration from supportedPackages
- long healthCheckDuration = monitoredPackage.mDurationMs;
- monitoredPackage.mHealthCheckDurationMs = Math.min(healthCheckDuration,
- monitoredPackage.mDurationMs);
- Slog.i(TAG, packageName + " health check state is now: ACTIVE("
- + monitoredPackage.mHealthCheckDurationMs + "ms)");
- } else {
- // Does not support health check, transistion to PASSED
- monitoredPackage.mHasPassedHealthCheck = true;
- Slog.i(TAG, packageName + " health check state is now: PASSED");
- }
+ if (supportedPackageTimeouts.containsKey(packageName)) {
+ // Supported packages become ACTIVE if currently INACTIVE
+ newState = monitoredPackage.setHealthCheckActiveLocked(
+ supportedPackageTimeouts.get(packageName));
} else {
- Slog.i(TAG, packageName + " does not support health check, state: PASSED");
+ // Unsupported packages are marked as PASSED unless already FAILED
+ newState = monitoredPackage.tryPassHealthCheckLocked();
}
+ isStateChanged |= oldState != newState;
}
}
}
- if (shouldUpdateFile) {
- saveToFileAsync();
- }
- if (shouldPrune) {
- pruneAndSchedule();
+ if (isStateChanged) {
+ syncState("updated health check supported packages " + supportedPackages);
}
}
+ @GuardedBy("mLock")
private Set<String> getPackagesPendingHealthChecksLocked() {
Slog.d(TAG, "Getting all observed packages pending health checks");
Set<String> packages = new ArraySet<>();
@@ -516,8 +503,9 @@
observer.mPackages.values().iterator();
while (pit.hasNext()) {
MonitoredPackage monitoredPackage = pit.next();
- String packageName = monitoredPackage.mName;
- if (!monitoredPackage.mHasPassedHealthCheck) {
+ String packageName = monitoredPackage.getName();
+ if (monitoredPackage.getHealthCheckStateLocked()
+ != MonitoredPackage.STATE_PASSED) {
packages.add(packageName);
}
}
@@ -525,88 +513,91 @@
return packages;
}
- /** Executes {@link #pruneObservers} and schedules the next execution. */
- private void pruneAndSchedule() {
+ /**
+ * Syncs the state of the observers.
+ *
+ * <p> Prunes all observers, saves new state to disk, syncs health check requests with the
+ * health check service and schedules the next state sync.
+ */
+ private void syncState(String reason) {
synchronized (mLock) {
- long nextDurationToScheduleMs = getNextPruneScheduleMillisLocked();
- if (nextDurationToScheduleMs == Long.MAX_VALUE) {
- Slog.i(TAG, "No monitored packages, ending prune");
- mDurationAtLastPrune = 0;
- mUptimeAtLastPruneMs = 0;
- return;
- }
- long uptimeMs = SystemClock.uptimeMillis();
- // O if not running
- long elapsedDurationMs = mUptimeAtLastPruneMs == 0
- ? 0 : uptimeMs - mUptimeAtLastPruneMs;
- // Less than O if unexpectedly didn't run yet even though
- // we are past the last duration scheduled to run
- long remainingDurationMs = mDurationAtLastPrune - elapsedDurationMs;
- if (mUptimeAtLastPruneMs == 0
- || remainingDurationMs <= 0
- || nextDurationToScheduleMs < remainingDurationMs) {
- // First schedule or an earlier reschedule
- pruneObservers(elapsedDurationMs);
- // We don't use Handler#hasCallbacks because we want to update the schedule delay
- mShortTaskHandler.removeCallbacks(this::pruneAndSchedule);
- mShortTaskHandler.postDelayed(this::pruneAndSchedule, nextDurationToScheduleMs);
- mDurationAtLastPrune = nextDurationToScheduleMs;
- mUptimeAtLastPruneMs = uptimeMs;
- }
+ Slog.i(TAG, "Syncing state, reason: " + reason);
+ pruneObserversLocked();
+
+ saveToFileAsync();
+ syncRequestsAsync();
+
+ // Done syncing state, schedule the next state sync
+ scheduleNextSyncStateLocked();
+ }
+ }
+
+ private void syncStateWithScheduledReason() {
+ syncState("scheduled");
+ }
+
+ @GuardedBy("mLock")
+ private void scheduleNextSyncStateLocked() {
+ long durationMs = getNextStateSyncMillisLocked();
+ mShortTaskHandler.removeCallbacks(this::syncStateWithScheduledReason);
+ if (durationMs == Long.MAX_VALUE) {
+ Slog.i(TAG, "Cancelling state sync, nothing to sync");
+ mUptimeAtLastStateSync = 0;
+ } else {
+ Slog.i(TAG, "Scheduling next state sync in " + durationMs + "ms");
+ mUptimeAtLastStateSync = SystemClock.uptimeMillis();
+ mShortTaskHandler.postDelayed(this::syncStateWithScheduledReason, durationMs);
}
}
/**
- * Returns the next time in millis to schedule a prune.
+ * Returns the next duration in millis to sync the watchdog state.
*
* @returns Long#MAX_VALUE if there are no observed packages.
*/
- private long getNextPruneScheduleMillisLocked() {
+ @GuardedBy("mLock")
+ private long getNextStateSyncMillisLocked() {
long shortestDurationMs = Long.MAX_VALUE;
for (int oIndex = 0; oIndex < mAllObservers.size(); oIndex++) {
ArrayMap<String, MonitoredPackage> packages = mAllObservers.valueAt(oIndex).mPackages;
for (int pIndex = 0; pIndex < packages.size(); pIndex++) {
MonitoredPackage mp = packages.valueAt(pIndex);
- long duration = Math.min(mp.mDurationMs, mp.mHealthCheckDurationMs);
+ long duration = mp.getShortestScheduleDurationMsLocked();
if (duration < shortestDurationMs) {
shortestDurationMs = duration;
}
}
}
- Slog.i(TAG, "Next prune will be scheduled in " + shortestDurationMs + "ms");
-
return shortestDurationMs;
}
/**
- * Removes {@code elapsedMs} milliseconds from all durations on monitored packages.
- *
- * <p> Prunes all observers with {@link ObserverInternal#prunePackages} and discards observers
- * without any packages left.
+ * Removes {@code elapsedMs} milliseconds from all durations on monitored packages
+ * and updates other internal state.
*/
- private void pruneObservers(long elapsedMs) {
- if (elapsedMs == 0) {
+ @GuardedBy("mLock")
+ private void pruneObserversLocked() {
+ long elapsedMs = mUptimeAtLastStateSync == 0
+ ? 0 : SystemClock.uptimeMillis() - mUptimeAtLastStateSync;
+ if (elapsedMs <= 0) {
+ Slog.i(TAG, "Not pruning observers, elapsed time: " + elapsedMs + "ms");
return;
}
- synchronized (mLock) {
- Slog.d(TAG, "Removing expired packages after " + elapsedMs + "ms");
- Iterator<ObserverInternal> it = mAllObservers.values().iterator();
- while (it.hasNext()) {
- ObserverInternal observer = it.next();
- Set<MonitoredPackage> failedPackages =
- observer.prunePackages(elapsedMs);
- if (!failedPackages.isEmpty()) {
- onHealthCheckFailed(observer, failedPackages);
- }
- if (observer.mPackages.isEmpty()) {
- Slog.i(TAG, "Discarding observer " + observer.mName + ". All packages expired");
- it.remove();
- }
+
+ Slog.i(TAG, "Removing " + elapsedMs + "ms from all packages on all observers");
+ Iterator<ObserverInternal> it = mAllObservers.values().iterator();
+ while (it.hasNext()) {
+ ObserverInternal observer = it.next();
+ Set<MonitoredPackage> failedPackages =
+ observer.prunePackagesLocked(elapsedMs);
+ if (!failedPackages.isEmpty()) {
+ onHealthCheckFailed(observer, failedPackages);
+ }
+ if (observer.mPackages.isEmpty()) {
+ Slog.i(TAG, "Discarding observer " + observer.mName + ". All packages expired");
+ it.remove();
}
}
- Slog.i(TAG, "Syncing health check requests, pruned observers");
- syncRequestsAsync();
- saveToFileAsync();
}
private void onHealthCheckFailed(ObserverInternal observer,
@@ -618,7 +609,7 @@
PackageManager pm = mContext.getPackageManager();
Iterator<MonitoredPackage> it = failedPackages.iterator();
while (it.hasNext()) {
- String failedPackage = it.next().mName;
+ String failedPackage = it.next().getName();
long versionCode = 0;
Slog.i(TAG, "Explicit health check failed for package " + failedPackage);
try {
@@ -673,6 +664,7 @@
* Persists mAllObservers to file. Threshold information is ignored.
*/
private boolean saveToFile() {
+ Slog.i(TAG, "Saving observer state to file");
synchronized (mLock) {
FileOutputStream stream;
try {
@@ -689,7 +681,7 @@
out.startTag(null, TAG_PACKAGE_WATCHDOG);
out.attribute(null, ATTR_VERSION, Integer.toString(DB_VERSION));
for (int oIndex = 0; oIndex < mAllObservers.size(); oIndex++) {
- mAllObservers.valueAt(oIndex).write(out);
+ mAllObservers.valueAt(oIndex).writeLocked(out);
}
out.endTag(null, TAG_PACKAGE_WATCHDOG);
out.endDocument();
@@ -730,7 +722,7 @@
ObserverInternal(String name, List<MonitoredPackage> packages) {
mName = name;
- updatePackages(packages);
+ updatePackagesLocked(packages);
}
/**
@@ -738,20 +730,13 @@
* Does not persist any package failure thresholds.
*/
@GuardedBy("mLock")
- public boolean write(XmlSerializer out) {
+ public boolean writeLocked(XmlSerializer out) {
try {
out.startTag(null, TAG_OBSERVER);
out.attribute(null, ATTR_NAME, mName);
for (int i = 0; i < mPackages.size(); i++) {
MonitoredPackage p = mPackages.valueAt(i);
- 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_EXPLICIT_HEALTH_CHECK_DURATION,
- String.valueOf(p.mHealthCheckDurationMs));
- out.attribute(null, ATTR_PASSED_HEALTH_CHECK,
- String.valueOf(p.mHasPassedHealthCheck));
- out.endTag(null, TAG_PACKAGE);
+ p.writeLocked(out);
}
out.endTag(null, TAG_OBSERVER);
return true;
@@ -762,7 +747,7 @@
}
@GuardedBy("mLock")
- public void updatePackages(List<MonitoredPackage> packages) {
+ public void updatePackagesLocked(List<MonitoredPackage> packages) {
for (int pIndex = 0; pIndex < packages.size(); pIndex++) {
MonitoredPackage p = packages.get(pIndex);
mPackages.put(p.mName, p);
@@ -775,37 +760,24 @@
* observation. If any health check duration is less than 0, the health check result
* is evaluated.
*
- * @returns a {@link Set} of packages that were removed from the observer without explicit
+ * @return a {@link Set} of packages that were removed from the observer without explicit
* health check passing, or an empty list if no package expired for which an explicit health
* check was still pending
*/
@GuardedBy("mLock")
- private Set<MonitoredPackage> prunePackages(long elapsedMs) {
+ private Set<MonitoredPackage> prunePackagesLocked(long elapsedMs) {
Set<MonitoredPackage> failedPackages = new ArraySet<>();
Iterator<MonitoredPackage> it = mPackages.values().iterator();
while (it.hasNext()) {
MonitoredPackage p = it.next();
- int healthCheckState = p.getHealthCheckState();
-
- // Handle health check timeouts
- if (healthCheckState == MonitoredPackage.STATE_ACTIVE) {
- // Only reduce duration if state is active
- p.mHealthCheckDurationMs -= elapsedMs;
- // Check duration after reducing duration
- if (p.mHealthCheckDurationMs <= 0) {
- failedPackages.add(p);
- }
+ int oldState = p.getHealthCheckStateLocked();
+ int newState = p.handleElapsedTimeLocked(elapsedMs);
+ if (oldState != MonitoredPackage.STATE_FAILED
+ && newState == MonitoredPackage.STATE_FAILED) {
+ Slog.i(TAG, "Package " + p.mName + " failed health check");
+ failedPackages.add(p);
}
-
- // Handle package expiry
- p.mDurationMs -= elapsedMs;
- // Check duration after reducing duration
- if (p.mDurationMs <= 0) {
- if (healthCheckState == MonitoredPackage.STATE_INACTIVE) {
- Slog.w(TAG, "Package " + p.mName
- + " expiring without starting health check, failing");
- failedPackages.add(p);
- }
+ if (p.isExpiredLocked()) {
it.remove();
}
}
@@ -817,10 +789,10 @@
* @returns {@code true} if failure threshold is exceeded, {@code false} otherwise
*/
@GuardedBy("mLock")
- public boolean onPackageFailure(String packageName) {
+ public boolean onPackageFailureLocked(String packageName) {
MonitoredPackage p = mPackages.get(packageName);
if (p != null) {
- return p.onFailure();
+ return p.onFailureLocked();
}
return false;
}
@@ -877,33 +849,45 @@
}
/**
- * Represents a package along with the time it should be monitored for.
+ * Represents a package and its health check state along with the time
+ * it should be monitored for.
*
* <p> Note, the PackageWatchdog#mLock must always be held when reading or writing
* instances of this class.
*/
- //TODO(b/120598832): Remove 'm' from non-private fields
- private static class MonitoredPackage {
+ static class MonitoredPackage {
// Health check states
+ // TODO(b/120598832): Prefix with HEALTH_CHECK
// mName has not passed health check but has requested a health check
- public static int STATE_ACTIVE = 0;
+ public static final int STATE_ACTIVE = 0;
// mName has not passed health check and has not requested a health check
- public static int STATE_INACTIVE = 1;
+ public static final int STATE_INACTIVE = 1;
// mName has passed health check
- public static int STATE_PASSED = 2;
+ public static final int STATE_PASSED = 2;
+ // mName has failed health check
+ public static final int STATE_FAILED = 3;
- public final String mName;
- // Whether an explicit health check has passed
+ //TODO(b/120598832): VersionedPackage?
+ private final String mName;
+ // One of STATE_[ACTIVE|INACTIVE|PASSED|FAILED]. Updated on construction and after
+ // methods that could change the health check state: handleElapsedTimeLocked and
+ // tryPassHealthCheckLocked
+ private int mHealthCheckState = STATE_INACTIVE;
+ // Whether an explicit health check has passed.
+ // This value in addition with mHealthCheckDurationMs determines the health check state
+ // of the package, see #getHealthCheckStateLocked
@GuardedBy("mLock")
- public boolean mHasPassedHealthCheck;
- // System uptime duration to monitor package
+ private boolean mHasPassedHealthCheck;
+ // System uptime duration to monitor package.
@GuardedBy("mLock")
- public long mDurationMs;
+ private long mDurationMs;
// System uptime duration to check the result of an explicit health check
// Initially, MAX_VALUE until we get a value from the health check service
// and request health checks.
+ // This value in addition with mHasPassedHealthCheck determines the health check state
+ // of the package, see #getHealthCheckStateLocked
@GuardedBy("mLock")
- public long mHealthCheckDurationMs = Long.MAX_VALUE;
+ private long mHealthCheckDurationMs = Long.MAX_VALUE;
// System uptime of first package failure
@GuardedBy("mLock")
private long mUptimeStartMs;
@@ -921,6 +905,20 @@
mDurationMs = durationMs;
mHealthCheckDurationMs = healthCheckDurationMs;
mHasPassedHealthCheck = hasPassedHealthCheck;
+ updateHealthCheckStateLocked();
+ }
+
+ /** Writes the salient fields to disk using {@code out}. */
+ @GuardedBy("mLock")
+ public void writeLocked(XmlSerializer out) throws IOException {
+ out.startTag(null, TAG_PACKAGE);
+ out.attribute(null, ATTR_NAME, mName);
+ out.attribute(null, ATTR_DURATION, String.valueOf(mDurationMs));
+ out.attribute(null, ATTR_EXPLICIT_HEALTH_CHECK_DURATION,
+ String.valueOf(mHealthCheckDurationMs));
+ out.attribute(null, ATTR_PASSED_HEALTH_CHECK,
+ String.valueOf(mHasPassedHealthCheck));
+ out.endTag(null, TAG_PACKAGE);
}
/**
@@ -929,7 +927,7 @@
* @return {@code true} if failure count exceeds a threshold, {@code false} otherwise
*/
@GuardedBy("mLock")
- public boolean onFailure() {
+ public boolean onFailureLocked() {
final long now = SystemClock.uptimeMillis();
final long duration = now - mUptimeStartMs;
if (duration > TRIGGER_DURATION_MS) {
@@ -949,18 +947,141 @@
}
/**
- * Returns any of the health check states of {@link #STATE_ACTIVE},
+ * Sets the initial health check duration.
+ *
+ * @return the new health check state
+ */
+ @GuardedBy("mLock")
+ public int setHealthCheckActiveLocked(long initialHealthCheckDurationMs) {
+ if (initialHealthCheckDurationMs <= 0) {
+ Slog.wtf(TAG, "Cannot set non-positive health check duration "
+ + initialHealthCheckDurationMs + "ms for package " + mName
+ + ". Using total duration " + mDurationMs + "ms instead");
+ initialHealthCheckDurationMs = mDurationMs;
+ }
+ if (mHealthCheckState == STATE_INACTIVE) {
+ // Transitions to ACTIVE
+ mHealthCheckDurationMs = initialHealthCheckDurationMs;
+ }
+ return updateHealthCheckStateLocked();
+ }
+
+ /**
+ * Updates the monitoring durations of the package.
+ *
+ * @return the new health check state
+ */
+ @GuardedBy("mLock")
+ public int handleElapsedTimeLocked(long elapsedMs) {
+ if (elapsedMs <= 0) {
+ Slog.w(TAG, "Cannot handle non-positive elapsed time for package " + mName);
+ return mHealthCheckState;
+ }
+ // Transitions to FAILED if now <= 0 and health check not passed
+ mDurationMs -= elapsedMs;
+ if (mHealthCheckState == STATE_ACTIVE) {
+ // We only update health check durations if we have #setHealthCheckActiveLocked
+ // This ensures we don't leave the INACTIVE state for an unexpected elapsed time
+ // Transitions to FAILED if now <= 0 and health check not passed
+ mHealthCheckDurationMs -= elapsedMs;
+ }
+ return updateHealthCheckStateLocked();
+ }
+
+ /**
+ * Marks the health check as passed and transitions to {@link #STATE_PASSED}
+ * if not yet {@link #STATE_FAILED}.
+ *
+ * @return the new health check state
+ */
+ @GuardedBy("mLock")
+ public int tryPassHealthCheckLocked() {
+ if (mHealthCheckState != STATE_FAILED) {
+ // FAILED is a final state so only pass if we haven't failed
+ // Transition to PASSED
+ mHasPassedHealthCheck = true;
+ }
+ return updateHealthCheckStateLocked();
+ }
+
+ /** Returns the monitored package name. */
+ private String getName() {
+ return mName;
+ }
+
+ //TODO(b/120598832): IntDef
+ /**
+ * Returns the current health check state, any of {@link #STATE_ACTIVE},
* {@link #STATE_INACTIVE} or {@link #STATE_PASSED}
*/
@GuardedBy("mLock")
- public int getHealthCheckState() {
+ public int getHealthCheckStateLocked() {
+ return mHealthCheckState;
+ }
+
+ /**
+ * Returns the shortest duration before the package should be scheduled for a prune.
+ *
+ * @return the duration or {@link Long#MAX_VALUE} if the package should not be scheduled
+ */
+ @GuardedBy("mLock")
+ public long getShortestScheduleDurationMsLocked() {
+ return Math.min(toPositive(mDurationMs), toPositive(mHealthCheckDurationMs));
+ }
+
+ /**
+ * Returns {@code true} if the total duration left to monitor the package is less than or
+ * equal to 0 {@code false} otherwise.
+ */
+ @GuardedBy("mLock")
+ public boolean isExpiredLocked() {
+ return mDurationMs <= 0;
+ }
+
+ /**
+ * Updates the health check state based on {@link #mHasPassedHealthCheck}
+ * and {@link #mHealthCheckDurationMs}.
+ *
+ * @return the new health check state
+ */
+ @GuardedBy("mLock")
+ private int updateHealthCheckStateLocked() {
+ int oldState = mHealthCheckState;
if (mHasPassedHealthCheck) {
- return STATE_PASSED;
+ // Set final state first to avoid ambiguity
+ mHealthCheckState = STATE_PASSED;
+ } else if (mHealthCheckDurationMs <= 0 || mDurationMs <= 0) {
+ // Set final state first to avoid ambiguity
+ mHealthCheckState = STATE_FAILED;
} else if (mHealthCheckDurationMs == Long.MAX_VALUE) {
- return STATE_INACTIVE;
+ mHealthCheckState = STATE_INACTIVE;
} else {
- return STATE_ACTIVE;
+ mHealthCheckState = STATE_ACTIVE;
}
+ Slog.i(TAG, "Updated health check state for package " + mName + ": "
+ + toString(oldState) + " -> " + toString(mHealthCheckState));
+ return mHealthCheckState;
+ }
+
+ /** Returns a {@link String} representation of the current health check state. */
+ private static String toString(int state) {
+ switch (state) {
+ case STATE_ACTIVE:
+ return "ACTIVE";
+ case STATE_INACTIVE:
+ return "INACTIVE";
+ case STATE_PASSED:
+ return "PASSED";
+ case STATE_FAILED:
+ return "FAILED";
+ default:
+ return "UNKNOWN";
+ }
+ }
+
+ /** Returns {@code value} if it is greater than 0 or {@link Long#MAX_VALUE} otherwise. */
+ private static long toPositive(long value) {
+ return value > 0 ? value : Long.MAX_VALUE;
}
}
}
diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
index b308982..fa7bf61 100644
--- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
+++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java
@@ -16,6 +16,7 @@
package com.android.server;
+import static com.android.server.PackageWatchdog.MonitoredPackage;
import static com.android.server.PackageWatchdog.TRIGGER_FAILURE_COUNT;
import static org.junit.Assert.assertEquals;
@@ -27,6 +28,7 @@
import android.content.pm.VersionedPackage;
import android.os.Handler;
import android.os.test.TestLooper;
+import android.service.watchdog.PackageInfo;
import android.util.AtomicFile;
import androidx.test.InstrumentationRegistry;
@@ -143,6 +145,31 @@
assertNull(watchdog.getPackages(observer3));
}
+ /** Observing already observed package extends the observation time. */
+ @Test
+ public void testObserveAlreadyObservedPackage() throws Exception {
+ PackageWatchdog watchdog = createWatchdog();
+ TestObserver observer = new TestObserver(OBSERVER_NAME_1);
+
+ // Start observing APP_A
+ watchdog.startObservingHealth(observer, Arrays.asList(APP_A), SHORT_DURATION);
+
+ // Then advance time half-way
+ Thread.sleep(SHORT_DURATION / 2);
+ mTestLooper.dispatchAll();
+
+ // Start observing APP_A again
+ watchdog.startObservingHealth(observer, Arrays.asList(APP_A), SHORT_DURATION);
+
+ // Then advance time such that it should have expired were it not for the second observation
+ Thread.sleep((SHORT_DURATION / 2) + 1);
+ mTestLooper.dispatchAll();
+
+ // Verify that APP_A not expired since second observation extended the time
+ assertEquals(1, watchdog.getPackages(observer).size());
+ assertTrue(watchdog.getPackages(observer).contains(APP_A));
+ }
+
/**
* Test package observers are persisted and loaded on startup
*/
@@ -577,6 +604,84 @@
assertEquals(APP_C, observer.mFailedPackages.get(0));
}
+ /**
+ * Tests failure when health check duration is different from package observation duration
+ * Failure is also notified only once.
+ */
+ @Test
+ public void testExplicitHealthCheckFailureBeforeExpiry() throws Exception {
+ TestController controller = new TestController();
+ PackageWatchdog watchdog = createWatchdog(controller, true /* withPackagesReady */);
+ TestObserver observer = new TestObserver(OBSERVER_NAME_1,
+ PackageHealthObserverImpact.USER_IMPACT_MEDIUM);
+
+ // Start observing with explicit health checks for APP_A and
+ // package observation duration == LONG_DURATION
+ // health check duration == SHORT_DURATION (set by default in the TestController)
+ controller.setSupportedPackages(Arrays.asList(APP_A));
+ watchdog.startObservingHealth(observer, Arrays.asList(APP_A), LONG_DURATION);
+
+ // Then APP_A has exceeded health check duration
+ Thread.sleep(SHORT_DURATION);
+ mTestLooper.dispatchAll();
+
+ // Verify that health check is failed
+ assertEquals(1, observer.mFailedPackages.size());
+ assertEquals(APP_A, observer.mFailedPackages.get(0));
+
+ // Then clear failed packages and start observing a random package so requests are synced
+ // and PackageWatchdog#onSupportedPackages is called and APP_A has a chance to fail again
+ // this time due to package expiry.
+ observer.mFailedPackages.clear();
+ watchdog.startObservingHealth(observer, Arrays.asList(APP_B), LONG_DURATION);
+
+ // Verify that health check failure is not notified again
+ assertTrue(observer.mFailedPackages.isEmpty());
+ }
+
+ /** Tests {@link MonitoredPackage} health check state transitions. */
+ @Test
+ public void testPackageHealthCheckStateTransitions() {
+ MonitoredPackage m1 = new MonitoredPackage(APP_A, LONG_DURATION,
+ false /* hasPassedHealthCheck */);
+ MonitoredPackage m2 = new MonitoredPackage(APP_B, LONG_DURATION, false);
+ MonitoredPackage m3 = new MonitoredPackage(APP_C, LONG_DURATION, false);
+ MonitoredPackage m4 = new MonitoredPackage(APP_D, LONG_DURATION, SHORT_DURATION, true);
+
+ // Verify transition: inactive -> active -> passed
+ // Verify initially inactive
+ assertEquals(MonitoredPackage.STATE_INACTIVE, m1.getHealthCheckStateLocked());
+ // Verify still inactive, until we #setHealthCheckActiveLocked
+ assertEquals(MonitoredPackage.STATE_INACTIVE, m1.handleElapsedTimeLocked(SHORT_DURATION));
+ // Verify now active
+ assertEquals(MonitoredPackage.STATE_ACTIVE, m1.setHealthCheckActiveLocked(SHORT_DURATION));
+ // Verify now passed
+ assertEquals(MonitoredPackage.STATE_PASSED, m1.tryPassHealthCheckLocked());
+
+ // Verify transition: inactive -> active -> failed
+ // Verify initially inactive
+ assertEquals(MonitoredPackage.STATE_INACTIVE, m2.getHealthCheckStateLocked());
+ // Verify now active
+ assertEquals(MonitoredPackage.STATE_ACTIVE, m2.setHealthCheckActiveLocked(SHORT_DURATION));
+ // Verify now failed
+ assertEquals(MonitoredPackage.STATE_FAILED, m2.handleElapsedTimeLocked(SHORT_DURATION));
+
+ // Verify transition: inactive -> failed
+ // Verify initially inactive
+ assertEquals(MonitoredPackage.STATE_INACTIVE, m3.getHealthCheckStateLocked());
+ // Verify now failed because package expired
+ assertEquals(MonitoredPackage.STATE_FAILED, m3.handleElapsedTimeLocked(LONG_DURATION));
+ // Verify remains failed even when asked to pass
+ assertEquals(MonitoredPackage.STATE_FAILED, m3.tryPassHealthCheckLocked());
+
+ // Verify transition: passed
+ assertEquals(MonitoredPackage.STATE_PASSED, m4.getHealthCheckStateLocked());
+ // Verify remains passed even if health check fails
+ assertEquals(MonitoredPackage.STATE_PASSED, m4.handleElapsedTimeLocked(SHORT_DURATION));
+ // Verify remains passed even if package expires
+ assertEquals(MonitoredPackage.STATE_PASSED, m4.handleElapsedTimeLocked(LONG_DURATION));
+ }
+
private PackageWatchdog createWatchdog() {
return createWatchdog(new TestController(), true /* withPackagesReady */);
}
@@ -636,7 +741,7 @@
private List<String> mSupportedPackages = new ArrayList<>();
private List<String> mRequestedPackages = new ArrayList<>();
private Consumer<String> mPassedConsumer;
- private Consumer<List<String>> mSupportedConsumer;
+ private Consumer<List<PackageInfo>> mSupportedConsumer;
private Runnable mNotifySyncRunnable;
@Override
@@ -649,7 +754,7 @@
@Override
public void setCallbacks(Consumer<String> passedConsumer,
- Consumer<List<String>> supportedConsumer, Runnable notifySyncRunnable) {
+ Consumer<List<PackageInfo>> supportedConsumer, Runnable notifySyncRunnable) {
mPassedConsumer = passedConsumer;
mSupportedConsumer = supportedConsumer;
mNotifySyncRunnable = notifySyncRunnable;
@@ -661,7 +766,11 @@
if (mIsEnabled) {
packages.retainAll(mSupportedPackages);
mRequestedPackages.addAll(packages);
- mSupportedConsumer.accept(mSupportedPackages);
+ List<PackageInfo> packageInfos = new ArrayList<>();
+ for (String packageName: packages) {
+ packageInfos.add(new PackageInfo(packageName, SHORT_DURATION));
+ }
+ mSupportedConsumer.accept(packageInfos);
} else {
mSupportedConsumer.accept(Collections.emptyList());
}