From dddda5103b442aaa9354104c7e193c8d2e85e8d9 Mon Sep 17 00:00:00 2001 From: Jiakai Zhang Date: Thu, 24 Nov 2022 19:18:11 +0000 Subject: [PATCH] Report STATUS_FATAL_ERROR if bg dexopt job encounters runtime exception. Bug: 259799817 Test: atest FrameworksMockingServicesTests:BackgroundDexOptServiceUnitTest Change-Id: Ie82097e63ab6200c09a8acb291d4286e6130cbcf --- .../server/pm/BackgroundDexOptService.java | 32 +++++++++++++------ .../server/pm/dex/ArtStatsLogUtils.java | 8 +++-- .../pm/BackgroundDexOptServiceUnitTest.java | 28 ++++++++++++++-- 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/pm/BackgroundDexOptService.java b/services/core/java/com/android/server/pm/BackgroundDexOptService.java index dd41830243149..cda7503400c71 100644 --- a/services/core/java/com/android/server/pm/BackgroundDexOptService.java +++ b/services/core/java/com/android/server/pm/BackgroundDexOptService.java @@ -92,6 +92,8 @@ public final class BackgroundDexOptService { new ComponentName("android", BackgroundDexOptJobService.class.getName()); // Possible return codes of individual optimization steps. + /** Initial value. */ + public static final int STATUS_UNSPECIFIED = -1; /** Ok status: Optimizations finished, All packages were processed, can continue */ public static final int STATUS_OK = 0; /** Optimizations should be aborted. Job scheduler requested it. */ @@ -108,16 +110,20 @@ public final class BackgroundDexOptService { * job will exclude those failed packages. */ public static final int STATUS_DEX_OPT_FAILED = 5; + /** Encountered fatal error, such as a runtime exception. */ + public static final int STATUS_FATAL_ERROR = 6; @IntDef(prefix = {"STATUS_"}, value = { + STATUS_UNSPECIFIED, STATUS_OK, STATUS_ABORT_BY_CANCELLATION, STATUS_ABORT_NO_SPACE_LEFT, STATUS_ABORT_THERMAL, STATUS_ABORT_BATTERY, STATUS_DEX_OPT_FAILED, + STATUS_FATAL_ERROR, }) @Retention(RetentionPolicy.SOURCE) public @interface Status {} @@ -153,7 +159,7 @@ public final class BackgroundDexOptService { // True if JobScheduler invocations of dexopt have been disabled. @GuardedBy("mLock") private boolean mDisableJobSchedulerJobs; - @GuardedBy("mLock") @Status private int mLastExecutionStatus = STATUS_OK; + @GuardedBy("mLock") @Status private int mLastExecutionStatus = STATUS_UNSPECIFIED; @GuardedBy("mLock") private long mLastExecutionStartUptimeMs; @GuardedBy("mLock") private long mLastExecutionDurationMs; @@ -561,18 +567,26 @@ public final class BackgroundDexOptService { private boolean runIdleOptimization( PackageManagerService pm, List pkgs, boolean isPostBootUpdate) { synchronized (mLock) { + mLastExecutionStatus = STATUS_UNSPECIFIED; mLastExecutionStartUptimeMs = SystemClock.uptimeMillis(); mLastExecutionDurationMs = -1; } - long lowStorageThreshold = getLowStorageThreshold(); - int status = idleOptimizePackages(pm, pkgs, lowStorageThreshold, isPostBootUpdate); - logStatus(status); - synchronized (mLock) { - mLastExecutionStatus = status; - mLastExecutionDurationMs = SystemClock.uptimeMillis() - mLastExecutionStartUptimeMs; - } - return status == STATUS_OK || status == STATUS_DEX_OPT_FAILED; + int status = STATUS_UNSPECIFIED; + try { + long lowStorageThreshold = getLowStorageThreshold(); + status = idleOptimizePackages(pm, pkgs, lowStorageThreshold, isPostBootUpdate); + logStatus(status); + return status == STATUS_OK || status == STATUS_DEX_OPT_FAILED; + } catch (RuntimeException e) { + status = STATUS_FATAL_ERROR; + throw e; + } finally { + synchronized (mLock) { + mLastExecutionStatus = status; + mLastExecutionDurationMs = SystemClock.uptimeMillis() - mLastExecutionStartUptimeMs; + } + } } /** Gets the size of the directory. It uses recursion to go over all files. */ diff --git a/services/core/java/com/android/server/pm/dex/ArtStatsLogUtils.java b/services/core/java/com/android/server/pm/dex/ArtStatsLogUtils.java index 14075301d523f..f388e070bb858 100644 --- a/services/core/java/com/android/server/pm/dex/ArtStatsLogUtils.java +++ b/services/core/java/com/android/server/pm/dex/ArtStatsLogUtils.java @@ -303,7 +303,9 @@ public class ArtStatsLogUtils { } private static final Map STATUS_MAP = - Map.of(BackgroundDexOptService.STATUS_OK, + Map.of(BackgroundDexOptService.STATUS_UNSPECIFIED, + ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_UNKNOWN, + BackgroundDexOptService.STATUS_OK, ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_JOB_FINISHED, BackgroundDexOptService.STATUS_ABORT_BY_CANCELLATION, ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_BY_CANCELLATION, @@ -314,7 +316,9 @@ public class ArtStatsLogUtils { BackgroundDexOptService.STATUS_ABORT_BATTERY, ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_BATTERY, BackgroundDexOptService.STATUS_DEX_OPT_FAILED, - ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_JOB_FINISHED); + ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_JOB_FINISHED, + BackgroundDexOptService.STATUS_FATAL_ERROR, + ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_FATAL_ERROR); /** Helper class to write background dexopt job stats to statsd. */ public static class BackgroundDexoptJobStatsLogger { diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/BackgroundDexOptServiceUnitTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/BackgroundDexOptServiceUnitTest.java index 1be7e2e6e30ee..5c3d695477557 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/BackgroundDexOptServiceUnitTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/BackgroundDexOptServiceUnitTest.java @@ -17,6 +17,7 @@ package com.android.server.pm; import static com.android.server.pm.BackgroundDexOptService.STATUS_DEX_OPT_FAILED; +import static com.android.server.pm.BackgroundDexOptService.STATUS_FATAL_ERROR; import static com.android.server.pm.BackgroundDexOptService.STATUS_OK; import static com.google.common.truth.Truth.assertThat; @@ -24,6 +25,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; @@ -263,6 +265,20 @@ public final class BackgroundDexOptServiceUnitTest { assertThat(getFailedPackageNamesSecondary()).isEmpty(); } + @Test + public void testIdleJobFullRunWithFatalError() { + initUntilBootCompleted(); + runFullJob(mJobServiceForPostBoot, mJobParametersForPostBoot, + /* expectedReschedule= */ false, /* expectedStatus= */ STATUS_OK, + /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ null); + + doThrow(RuntimeException.class).when(mDexOptHelper).performDexOptWithStatus(any()); + + runFullJob(mJobServiceForIdle, mJobParametersForIdle, + /* expectedReschedule= */ false, /* expectedStatus= */ STATUS_FATAL_ERROR, + /* totalJobFinishedWithParams= */ 1, /* expectedSkippedPackage= */ null); + } + @Test public void testSystemReadyWhenDisabled() { when(mInjector.isBackgroundDexOptDisabled()).thenReturn(true); @@ -510,13 +526,21 @@ public final class BackgroundDexOptServiceUnitTest { ArgumentCaptor argThreadRunnable = ArgumentCaptor.forClass(Runnable.class); verify(mInjector, atLeastOnce()).createAndStartThread(any(), argThreadRunnable.capture()); - argThreadRunnable.getValue().run(); + try { + argThreadRunnable.getValue().run(); + } catch (RuntimeException e) { + if (expectedStatus != STATUS_FATAL_ERROR) { + throw e; + } + } verify(jobService, times(totalJobFinishedWithParams)).jobFinished(params, expectedReschedule); // Never block verify(mDexOptHelper, never()).controlDexOptBlocking(true); - verifyPerformDexOpt(); + if (expectedStatus != STATUS_FATAL_ERROR) { + verifyPerformDexOpt(); + } assertThat(getLastExecutionStatus()).isEqualTo(expectedStatus); }