diff options
5 files changed, 51 insertions, 33 deletions
diff --git a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java index b7f47543fc..51a01b363d 100644 --- a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java @@ -17,6 +17,7 @@ package com.android.server.art; import static com.android.server.art.model.ArtFlags.ScheduleStatus; +import static com.android.server.art.prereboot.PreRebootDriver.PreRebootResult; import static com.android.server.art.proto.PreRebootStats.Status; import android.annotation.NonNull; @@ -158,9 +159,10 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { // return value of `onStopJob` will be respected, and this call will be ignored. jobService.jobFinished(params, false /* wantsReschedule */); }; - // No need to handle exceptions thrown by the future because exceptions are handled inside - // the future itself. - startLocked(onJobFinishedLocked); + startLocked(onJobFinishedLocked).exceptionally(t -> { + AsLog.e("Fatal error", t); + return null; + }); } @Override @@ -357,10 +359,15 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { var cancellationSignal = mCancellationSignal = new CancellationSignal(); mRunningJob = new CompletableFuture().runAsync(() -> { markHasStarted(true); + PreRebootStatsReporter statsReporter = mInjector.getStatsReporter(); try { - mInjector.getPreRebootDriver().run(otaSlot, mapSnapshotsForOta, cancellationSignal); + statsReporter.recordJobStarted(); + PreRebootResult result = mInjector.getPreRebootDriver().run( + otaSlot, mapSnapshotsForOta, cancellationSignal); + statsReporter.recordJobEnded(result); } catch (RuntimeException e) { - AsLog.e("Fatal error", e); + statsReporter.recordJobEnded(new PreRebootResult(false /* success */)); + throw e; } finally { synchronized (this) { if (onJobFinishedLocked != null) { diff --git a/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java b/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java index 7c1a1c55fa..c29e0b4b2c 100644 --- a/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java +++ b/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java @@ -86,20 +86,16 @@ public class PreRebootDriver { } /** - * Runs Pre-reboot Dexopt and returns whether it is successful. Returns false if Pre-reboot - * dexopt failed, the system requirement check failed, or system requirements are not met. + * Runs Pre-reboot Dexopt and returns the result. * * @param otaSlot The slot that contains the OTA update, "_a" or "_b", or null for a Mainline * update. * @param mapSnapshotsForOta Whether to map/unmap snapshots. Only applicable to an OTA update. */ - public boolean run(@Nullable String otaSlot, boolean mapSnapshotsForOta, + public @NonNull PreRebootResult run(@Nullable String otaSlot, boolean mapSnapshotsForOta, @NonNull CancellationSignal cancellationSignal) { - var statsReporter = new PreRebootStatsReporter(); - boolean success = false; boolean systemRequirementCheckFailed = false; try { - statsReporter.recordJobStarted(); try (var snapshot = mInjector.getPackageManagerLocal().withFilteredSnapshot()) { BatchDexoptParams params = mInjector.getArtManagerLocal().getBatchDexoptParams( snapshot, ReasonMapping.REASON_PRE_REBOOT_DEXOPT, cancellationSignal); @@ -108,8 +104,7 @@ public class PreRebootDriver { runFromChroot(cancellationSignal, snapshot, params); } } - success = true; - return true; + return new PreRebootResult(true /* success */); } catch (RemoteException e) { Utils.logArtdException(e); } catch (ServiceSpecificException e) { @@ -139,11 +134,9 @@ public class PreRebootDriver { Utils.logArtdException(e); } catch (ServiceSpecificException | IOException e) { AsLog.e("Failed to tear down chroot", e); - } finally { - statsReporter.recordJobEnded(success, systemRequirementCheckFailed); } } - return false; + return new PreRebootResult(false /* success */, systemRequirementCheckFailed); } public void test() { @@ -257,6 +250,16 @@ public class PreRebootDriver { } /** + * @param success whether Pre-reboot Dexopt is successful. False if Pre-reboot dexopt failed, + * the system requirement check failed, or system requirements are not met. + */ + public record PreRebootResult(boolean success, boolean systemRequirementCheckFailed) { + public PreRebootResult(boolean success) { + this(success, false /* systemRequirementCheckFailed */); + } + } + + /** * Injector pattern for testing purpose. * * @hide diff --git a/libartservice/service/java/com/android/server/art/prereboot/PreRebootStatsReporter.java b/libartservice/service/java/com/android/server/art/prereboot/PreRebootStatsReporter.java index fbf242db62..ec5c1acf31 100644 --- a/libartservice/service/java/com/android/server/art/prereboot/PreRebootStatsReporter.java +++ b/libartservice/service/java/com/android/server/art/prereboot/PreRebootStatsReporter.java @@ -16,6 +16,7 @@ package com.android.server.art.prereboot; +import static com.android.server.art.prereboot.PreRebootDriver.PreRebootResult; import static com.android.server.art.proto.PreRebootStats.JobRun; import static com.android.server.art.proto.PreRebootStats.JobType; import static com.android.server.art.proto.PreRebootStats.Status; @@ -141,7 +142,7 @@ public class PreRebootStatsReporter { } } - public void recordJobEnded(boolean success, boolean systemRequirementCheckFailed) { + public void recordJobEnded(@NonNull PreRebootResult result) { PreRebootStats.Builder statsBuilder = load(); if (statsBuilder.getStatus() == Status.STATUS_UNKNOWN) { // Failed to load, the error is already logged. @@ -157,7 +158,7 @@ public class PreRebootStatsReporter { mInjector.getCurrentTimeMillis()); Status status; - if (success) { + if (result.success()) { // The job is cancelled if it hasn't done package scanning (total package count is 0), // or it's interrupted in the middle of package processing (package counts don't add up // to the total). @@ -172,7 +173,7 @@ public class PreRebootStatsReporter { status = Status.STATUS_CANCELLED; } } else { - if (systemRequirementCheckFailed) { + if (result.systemRequirementCheckFailed()) { status = Status.STATUS_ABORTED_SYSTEM_REQUIREMENTS; } else { status = Status.STATUS_FAILED; diff --git a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java index 4aaf1d5be9..0d99db20e7 100644 --- a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java +++ b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java @@ -17,6 +17,7 @@ package com.android.server.art; import static com.android.server.art.PreRebootDexoptJob.JOB_ID; +import static com.android.server.art.prereboot.PreRebootDriver.PreRebootResult; import static com.google.common.truth.Truth.assertThat; @@ -236,7 +237,7 @@ public class PreRebootDexoptJobTest { when(mPreRebootDriver.run(any(), eq(true) /* mapSnapshotsForOta */, any())) .thenAnswer(invocation -> { jobStarted.release(); - return true; + return new PreRebootResult(true /* success */); }); when(ArtJni.setProperty("dalvik.vm.pre-reboot.has-started", "true")) @@ -259,7 +260,7 @@ public class PreRebootDexoptJobTest { @Test public void testSyncStart() throws Exception { when(mPreRebootDriver.run(any(), eq(false) /* mapSnapshotsForOta */, any())) - .thenReturn(true); + .thenReturn(new PreRebootResult(true /* success */)); CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( null /* otaSlot */, false /* mapSnapshotsForOta */); @@ -276,7 +277,7 @@ public class PreRebootDexoptJobTest { cancellationSignal.setOnCancelListener(() -> dexoptCancelled.release()); assertThat(dexoptCancelled.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); jobExited.release(); - return true; + return new PreRebootResult(true /* success */); }); mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */); @@ -297,7 +298,7 @@ public class PreRebootDexoptJobTest { cancellationSignal.setOnCancelListener(() -> dexoptCancelled.release()); assertThat(dexoptCancelled.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); jobExited.release(); - return true; + return new PreRebootResult(true /* success */); }); CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( @@ -314,7 +315,8 @@ public class PreRebootDexoptJobTest { mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */); mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */); - when(mPreRebootDriver.run(eq("_b"), anyBoolean(), any())).thenReturn(true); + when(mPreRebootDriver.run(eq("_b"), anyBoolean(), any())) + .thenReturn(new PreRebootResult(true /* success */)); mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); mPreRebootDexoptJob.waitForRunningJob(); @@ -325,7 +327,8 @@ public class PreRebootDexoptJobTest { mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */); mPreRebootDexoptJob.onUpdateReadyImpl("_a" /* otaSlot */); - when(mPreRebootDriver.run(eq("_a"), anyBoolean(), any())).thenReturn(true); + when(mPreRebootDriver.run(eq("_a"), anyBoolean(), any())) + .thenReturn(new PreRebootResult(true /* success */)); mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); mPreRebootDexoptJob.waitForRunningJob(); @@ -336,7 +339,8 @@ public class PreRebootDexoptJobTest { mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */); mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */); - when(mPreRebootDriver.run(isNull(), anyBoolean(), any())).thenReturn(true); + when(mPreRebootDriver.run(isNull(), anyBoolean(), any())) + .thenReturn(new PreRebootResult(true /* success */)); mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); mPreRebootDexoptJob.waitForRunningJob(); @@ -347,7 +351,8 @@ public class PreRebootDexoptJobTest { mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */); mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */); - when(mPreRebootDriver.run(eq("_b"), anyBoolean(), any())).thenReturn(true); + when(mPreRebootDriver.run(eq("_b"), anyBoolean(), any())) + .thenReturn(new PreRebootResult(true /* success */)); mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); mPreRebootDexoptJob.waitForRunningJob(); @@ -375,7 +380,7 @@ public class PreRebootDexoptJobTest { when(mPreRebootDriver.run(any(), anyBoolean(), any())).thenAnswer(invocation -> { // Simulate that the job takes a while to exit, no matter it's cancelled or not. assertThat(jobBlocker.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); - return true; + return new PreRebootResult(true /* success */); }); // An update arrives. A job is scheduled. @@ -461,7 +466,7 @@ public class PreRebootDexoptJobTest { cancellationSignal.setOnCancelListener(() -> dexoptCancelled.release()); assertThat(dexoptCancelled.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); jobExited.release(); - return true; + return new PreRebootResult(true /* success */); }); // An update arrives. A job is scheduled. diff --git a/libartservice/service/javatests/com/android/server/art/prereboot/PreRebootStatsReporterTest.java b/libartservice/service/javatests/com/android/server/art/prereboot/PreRebootStatsReporterTest.java index 59291b7471..846a835b75 100644 --- a/libartservice/service/javatests/com/android/server/art/prereboot/PreRebootStatsReporterTest.java +++ b/libartservice/service/javatests/com/android/server/art/prereboot/PreRebootStatsReporterTest.java @@ -17,6 +17,7 @@ package com.android.server.art.prereboot; import static com.android.server.art.model.DexoptStatus.DexContainerFileDexoptStatus; +import static com.android.server.art.prereboot.PreRebootDriver.PreRebootResult; import static com.android.server.art.proto.PreRebootStats.JobRun; import static com.android.server.art.proto.PreRebootStats.JobType; import static com.android.server.art.proto.PreRebootStats.Status; @@ -118,7 +119,7 @@ public class PreRebootStatsReporterTest { .build()); doReturn(300l).when(mInjector).getCurrentTimeMillis(); - reporter.recordJobEnded(true /* success */, false /* systemRequirementCheckFailed */); + reporter.recordJobEnded(new PreRebootResult(true /* success */)); checkProto(PreRebootStats.newBuilder() .setStatus(Status.STATUS_CANCELLED) .setJobType(JobType.JOB_TYPE_MAINLINE) @@ -174,7 +175,7 @@ public class PreRebootStatsReporterTest { .build()); doReturn(600l).when(mInjector).getCurrentTimeMillis(); - reporter.recordJobEnded(true /* success */, false /* systemRequirementCheckFailed */); + reporter.recordJobEnded(new PreRebootResult(true /* success */)); checkProto(PreRebootStats.newBuilder() .setStatus(Status.STATUS_FINISHED) .setJobType(JobType.JOB_TYPE_MAINLINE) @@ -290,7 +291,7 @@ public class PreRebootStatsReporterTest { .build()); doReturn(300l).when(mInjector).getCurrentTimeMillis(); - reporter.recordJobEnded(true /* success */, false /* systemRequirementCheckFailed */); + reporter.recordJobEnded(new PreRebootResult(true /* success */)); checkProto(PreRebootStats.newBuilder() .setStatus(Status.STATUS_FINISHED) .setJobType(JobType.JOB_TYPE_OTA) @@ -350,7 +351,8 @@ public class PreRebootStatsReporterTest { .build()); doReturn(300l).when(mInjector).getCurrentTimeMillis(); - reporter.recordJobEnded(false /* success */, systemRequirementCheckFailed); + reporter.recordJobEnded( + new PreRebootResult(false /* success */, systemRequirementCheckFailed)); checkProto(PreRebootStats.newBuilder() .setStatus(systemRequirementCheckFailed ? Status.STATUS_ABORTED_SYSTEM_REQUIREMENTS |