diff options
3 files changed, 436 insertions, 199 deletions
diff --git a/services/core/java/com/android/server/ExplicitHealthCheckController.java b/services/core/java/com/android/server/ExplicitHealthCheckController.java index 27ad208ef8a1..19ab33e85f5e 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.RemoteException; 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 @@ class ExplicitHealthCheckController { // 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 @@ class ExplicitHealthCheckController { * 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 @@ class ExplicitHealthCheckController { 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 @@ class ExplicitHealthCheckController { * 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 @@ class ExplicitHealthCheckController { 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 0c681df036b4..7d0d8344a566 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.Environment; 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.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -102,16 +104,10 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { if (internalObserver != null) { internalObserver.mRegisteredObserver = observer; } - if (mDurationAtLastPrune == 0) { - // Nothing running, prune - pruneAndSchedule(); - } } } @@ -214,6 +206,12 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { } 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 @@ public class PackageWatchdog { synchronized (mLock) { mAllObservers.remove(observer.getName()); } - saveToFileAsync(); + syncState("unregistering observer: " + observer.getName()); } /** @@ -296,7 +294,8 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { /** 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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { * 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 @@ public class PackageWatchdog { // 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 @@ public class PackageWatchdog { * 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(); - - 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"); - } + String packageName = monitoredPackage.getName(); + int oldState = monitoredPackage.getHealthCheckStateLocked(); + int newState; + + 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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { * 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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { ObserverInternal(String name, List<MonitoredPackage> packages) { mName = name; - updatePackages(packages); + updatePackagesLocked(packages); } /** @@ -738,20 +730,13 @@ public class PackageWatchdog { * 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 @@ public class PackageWatchdog { } @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 @@ public class PackageWatchdog { * 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 @@ public class PackageWatchdog { * @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 @@ public class PackageWatchdog { } /** - * 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 final String mName; - // Whether an explicit health check has passed + public static final int STATE_PASSED = 2; + // mName has failed health check + public static final int STATE_FAILED = 3; + + //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 @@ public class PackageWatchdog { 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 @@ public class PackageWatchdog { * @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 @@ public class PackageWatchdog { } /** - * 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 b308982c2343..fa7bf61a07b2 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.Context; 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 @@ public class PackageWatchdogTest { 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 @@ public class PackageWatchdogTest { 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 @@ public class PackageWatchdogTest { 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 @@ public class PackageWatchdogTest { @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 @@ public class PackageWatchdogTest { 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()); } |