Offload `PreRebootDexoptJob.onUpdateReady` to a separate thread.

Before this change, `PreRebootDexoptJob.onUpdateReady` is called on the
package manager thread during an apex installation and triggered a
pre-watchdog (1/4 of the watchdog timeout). This change fixes it.

Bug: 366387132
Test: atest ArtServiceTests
Change-Id: Ie50c04c1bfa81d38178ed1a2aa37a766f2da8a4a

3 files changed, 40 insertions, 30 deletions

diff --git a/libartservice/service/java/com/android/server/art/ArtShellCommand.java b/libartservice/service/java/com/android/server/art/ArtShellCommand.java
index a5ad18dbbc..f7d26cb36f 100644
--- a/libartservice/service/java/com/android/server/art/ArtShellCommand.java
+++ b/libartservice/service/java/com/android/server/art/ArtShellCommand.java
@@ -817,7 +817,7 @@ public final class ArtShellCommand extends BasicShellCommandHandler {
 
     @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM)
     private int handleSchedulePrDexoptJob(@NonNull PrintWriter pw, @Nullable String otaSlot) {
-        int code = mArtManagerLocal.getPreRebootDexoptJob().onUpdateReady(otaSlot);
+        int code = mArtManagerLocal.getPreRebootDexoptJob().onUpdateReadyImpl(otaSlot);
         switch (code) {
             case ArtFlags.SCHEDULE_SUCCESS:
                 pw.println("Pre-reboot Dexopt job scheduled");
diff --git a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java
index c560300b48..65062612a3 100644
--- a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java
+++ b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java
@@ -90,6 +90,8 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface {
     /**
      * Offloads `onStartJob` and `onStopJob` calls from the main thread while keeping the execution
      * order as the main thread does.
+     * Also offloads `onUpdateReady` calls from the package manager thread. We reuse this executor
+     * just for simplicity. The execution order does not matter.
      */
     @NonNull
     private final ThreadPoolExecutor mSerializedExecutor =
@@ -181,7 +183,14 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface {
      * @param otaSlot The slot that contains the OTA update, "_a" or "_b", or null for a Mainline
      *         update.
      */
-    public synchronized @ScheduleStatus int onUpdateReady(@Nullable String otaSlot) {
+    public synchronized void onUpdateReady(@Nullable String otaSlot) {
+        // `onUpdateReadyImpl` can take time, especially on `resetLocked` when there are staged
+        // files from a previous run to be cleaned up, so we put it on a separate thread.
+        mSerializedExecutor.execute(() -> onUpdateReadyImpl(otaSlot));
+    }
+
+    /** For internal and testing use only. */
+    public synchronized @ScheduleStatus int onUpdateReadyImpl(@Nullable String otaSlot) {
         cancelAnyLocked();
         resetLocked();
         updateOtaSlotLocked(otaSlot);
@@ -190,7 +199,8 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface {
     }
 
     /**
-     * Same as above, but starts the job immediately, instead of going through the job scheduler.
+     * Same as {@link #onUpdateReady}, but starts the job immediately, instead of going through the
+     * job scheduler.
      *
      * @param mapSnapshotsForOta whether to map/unmap snapshots. Only applicable to an OTA update.
      * @return The future of the job, or null if Pre-reboot Dexopt is not enabled.
diff --git a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java
index fc83a32c9a..4aaf1d5be9 100644
--- a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java
+++ b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java
@@ -134,7 +134,7 @@ public class PreRebootDexoptJobTest {
 
     @Test
     public void testSchedule() throws Exception {
-        assertThat(mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */))
+        assertThat(mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */))
                 .isEqualTo(ArtFlags.SCHEDULE_SUCCESS);
 
         assertThat(mJobInfo.isPeriodic()).isFalse();
@@ -148,7 +148,7 @@ public class PreRebootDexoptJobTest {
         when(SystemProperties.getBoolean(eq("pm.dexopt.disable_bg_dexopt"), anyBoolean()))
                 .thenReturn(true);
 
-        assertThat(mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */))
+        assertThat(mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */))
                 .isEqualTo(ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP);
 
         verify(mJobScheduler, never()).schedule(any());
@@ -171,7 +171,7 @@ public class PreRebootDexoptJobTest {
         when(SystemProperties.getBoolean(eq("dalvik.vm.enable_pr_dexopt"), anyBoolean()))
                 .thenReturn(false);
 
-        assertThat(mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */))
+        assertThat(mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */))
                 .isEqualTo(ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP);
 
         verify(mJobScheduler, never()).schedule(any());
@@ -199,7 +199,7 @@ public class PreRebootDexoptJobTest {
                         eq(DeviceConfig.NAMESPACE_RUNTIME), eq("enable_pr_dexopt"), anyBoolean()))
                 .thenReturn(true);
 
-        assertThat(mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */))
+        assertThat(mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */))
                 .isEqualTo(ArtFlags.SCHEDULE_SUCCESS);
 
         verify(mJobScheduler).schedule(any());
@@ -218,7 +218,7 @@ public class PreRebootDexoptJobTest {
                      eq("force_disable_pr_dexopt"), anyBoolean()))
                 .thenReturn(true);
 
-        assertThat(mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */))
+        assertThat(mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */))
                 .isEqualTo(ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP);
 
         verify(mJobScheduler, never()).schedule(any());
@@ -226,7 +226,7 @@ public class PreRebootDexoptJobTest {
 
     @Test
     public void testUnschedule() {
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
         verify(mJobScheduler).cancel(JOB_ID);
     }
 
@@ -248,7 +248,7 @@ public class PreRebootDexoptJobTest {
                 });
 
         assertThat(mPreRebootDexoptJob.hasStarted()).isFalse();
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
         mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters);
         assertThat(jobStarted.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue();
         assertThat(mPreRebootDexoptJob.hasStarted()).isTrue();
@@ -279,7 +279,7 @@ public class PreRebootDexoptJobTest {
             return true;
         });
 
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
         mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters);
         mPreRebootDexoptJob.onStopJobImpl(mJobParameters);
 
@@ -311,8 +311,8 @@ public class PreRebootDexoptJobTest {
 
     @Test
     public void testUpdateOtaSlotOtaThenMainline() {
-        mPreRebootDexoptJob.onUpdateReady("_b" /* otaSlot */);
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
 
         when(mPreRebootDriver.run(eq("_b"), anyBoolean(), any())).thenReturn(true);
 
@@ -322,8 +322,8 @@ public class PreRebootDexoptJobTest {
 
     @Test
     public void testUpdateOtaSlotMainlineThenOta() {
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
-        mPreRebootDexoptJob.onUpdateReady("_a" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_a" /* otaSlot */);
 
         when(mPreRebootDriver.run(eq("_a"), anyBoolean(), any())).thenReturn(true);
 
@@ -333,8 +333,8 @@ public class PreRebootDexoptJobTest {
 
     @Test
     public void testUpdateOtaSlotMainlineThenMainline() {
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
 
         when(mPreRebootDriver.run(isNull(), anyBoolean(), any())).thenReturn(true);
 
@@ -344,8 +344,8 @@ public class PreRebootDexoptJobTest {
 
     @Test
     public void testUpdateOtaSlotOtaThenOta() {
-        mPreRebootDexoptJob.onUpdateReady("_b" /* otaSlot */);
-        mPreRebootDexoptJob.onUpdateReady("_b" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */);
 
         when(mPreRebootDriver.run(eq("_b"), anyBoolean(), any())).thenReturn(true);
 
@@ -355,13 +355,13 @@ public class PreRebootDexoptJobTest {
 
     @Test(expected = IllegalStateException.class)
     public void testUpdateOtaSlotOtaThenOtaDifferentSlots() {
-        mPreRebootDexoptJob.onUpdateReady("_b" /* otaSlot */);
-        mPreRebootDexoptJob.onUpdateReady("_a" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_b" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_a" /* otaSlot */);
     }
 
     @Test(expected = IllegalStateException.class)
     public void testUpdateOtaSlotOtaBogusSlot() {
-        mPreRebootDexoptJob.onUpdateReady("_bogus" /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl("_bogus" /* otaSlot */);
     }
 
     /**
@@ -379,7 +379,7 @@ public class PreRebootDexoptJobTest {
         });
 
         // An update arrives. A job is scheduled.
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
 
         // The job scheduler starts the job.
         mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters);
@@ -390,11 +390,11 @@ public class PreRebootDexoptJobTest {
             // Another update arrives. A new job is scheduled, replacing the old job. The old job
             // doesn't exit immediately, so this call is blocked.
             JobParameters oldParameters = mJobParameters;
-            mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+            mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
 
             // The job scheduler tries to cancel the old job because of the new update. This call
             // doesn't matter because the job has already been cancelled by ourselves during the
-            // `onUpdateReady` call above.
+            // `onUpdateReadyImpl` call above.
             mPreRebootDexoptJob.onStopJobImpl(oldParameters);
 
             // The job scheduler starts the new job.
@@ -432,15 +432,15 @@ public class PreRebootDexoptJobTest {
     @Test
     public void testRace2() throws Exception {
         // An update arrives. A job is scheduled.
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
         JobParameters oldParameters = mJobParameters;
 
         // The job scheduler starts the job. In the meantime, another update arrives. It's not
-        // possible that `onStartJob` is called for the old job after `onUpdateReady` is called
-        // because `onUpdateReady` unschedules the old job. However, since both calls acquire a
+        // possible that `onStartJob` is called for the old job after `onUpdateReadyImpl` is called
+        // because `onUpdateReadyImpl` unschedules the old job. However, since both calls acquire a
         // lock, the order of execution may be reversed. When this happens, the `onStartJob` request
         // should not succeed.
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
         mPreRebootDexoptJob.onStartJobImpl(mJobService, oldParameters);
         assertThat(mPreRebootDexoptJob.hasRunningJob()).isFalse();
 
@@ -465,7 +465,7 @@ public class PreRebootDexoptJobTest {
         });
 
         // An update arrives. A job is scheduled.
-        mPreRebootDexoptJob.onUpdateReady(null /* otaSlot */);
+        mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */);
 
         // The job scheduler starts the job.
         mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters);