Do not attempt to retrieve the job scheduler or the app hibernation
manager before they should be available.
SystemServiceRegistry.getSystemService does not allow querying service
instances before they have been registered, so we cannot
opportunistically do that in code paths that may be run from the
ArtManagerLocal constructor or onBoot. Hence we need to switch to a
more principled approach to avoid calling it in those cases.
JobScheduler may not be requested from the constructor, so delay
creation of the BackgroundDexoptJob instance until the getter is
called.
AppHibernationManager may not be requested in onBoot. Continue to use
the compilation reason to identify it. That way those reasons behave
consistently even if a later dexopt operation uses one of them.
Test: Boot with dalvik.vm.useartservice=true and check that ART Service
comes up and that logcat that no "Manager wrapper not available" wtf
messages are logged from getSystemService.
Test: atest ArtServiceTests art_libartservice_tests
Bug: 266028904
Ignore-AOSP-First: ART Services
Change-Id: I387be74b32f361141c0446ac97cb8383a7286bf3
diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
index 5066a9e..07d77aa 100644
--- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
+++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java
@@ -337,6 +337,8 @@
* @throws IllegalArgumentException if the package is not found or the params are illegal
* @throws IllegalStateException if the operation encounters an error that should never happen
* (e.g., an internal logic error).
+ * @throws RuntimeException if called during boot before the app hibernation manager has
+ * started.
*/
@NonNull
public DexoptResult dexoptPackage(@NonNull PackageManagerLocal.FilteredSnapshot snapshot,
@@ -354,8 +356,8 @@
public DexoptResult dexoptPackage(@NonNull PackageManagerLocal.FilteredSnapshot snapshot,
@NonNull String packageName, @NonNull DexoptParams params,
@NonNull CancellationSignal cancellationSignal) {
- return mInjector.getDexoptHelper().dexopt(
- snapshot, List.of(packageName), params, cancellationSignal, Runnable::run);
+ return mInjector.getDexoptHelper(mInjector.getAppHibernationManager())
+ .dexopt(snapshot, List.of(packageName), params, cancellationSignal, Runnable::run);
}
/**
@@ -441,8 +443,28 @@
@NonNull CancellationSignal cancellationSignal,
@Nullable @CallbackExecutor Executor progressCallbackExecutor,
@Nullable Consumer<OperationProgress> progressCallback) {
- List<String> defaultPackages =
- Collections.unmodifiableList(getDefaultPackages(snapshot, reason));
+ // We cannot assume the app hibernation manager has been initialized yet in the boot time
+ // compilation, because ArtManagerLocal.onBoot needs to run early to ensure apps are
+ // compiled before the system server fires them up. This means the boot time compilation
+ // will ignore the hibernation states of the packages.
+ //
+ // TODO(b/265782156): When hibernated packages get compiled this way, the file GC will
+ // delete them again in the next background dexopt run. That means they are likely to get
+ // recreated again in the next boot dexopt (i.e. for OTA or Mainline update).
+ AppHibernationManager appHibernationManager;
+ switch (reason) {
+ case ReasonMapping.REASON_FIRST_BOOT:
+ case ReasonMapping.REASON_BOOT_AFTER_OTA:
+ case ReasonMapping.REASON_BOOT_AFTER_MAINLINE_UPDATE:
+ appHibernationManager = null;
+ break;
+ default:
+ appHibernationManager = mInjector.getAppHibernationManager();
+ break;
+ }
+
+ List<String> defaultPackages = Collections.unmodifiableList(
+ getDefaultPackages(snapshot, reason, appHibernationManager));
DexoptParams defaultDexoptParams = new DexoptParams.Builder(reason).build();
var builder = new BatchDexoptParams.Builder(defaultPackages, defaultDexoptParams);
Callback<BatchDexoptStartCallback, Void> callback =
@@ -465,9 +487,10 @@
cancellationSignal, dexoptExecutor);
}
Log.i(TAG, "Dexopting packages");
- return mInjector.getDexoptHelper().dexopt(snapshot, params.getPackages(),
- params.getDexoptParams(), cancellationSignal, dexoptExecutor,
- progressCallbackExecutor, progressCallback);
+ return mInjector.getDexoptHelper(appHibernationManager)
+ .dexopt(snapshot, params.getPackages(), params.getDexoptParams(),
+ cancellationSignal, dexoptExecutor, progressCallbackExecutor,
+ progressCallback);
} finally {
dexoptExecutor.shutdown();
}
@@ -526,6 +549,8 @@
* When the job ends (either completed or cancelled), the result is sent to the callbacks added
* by {@link #addDexoptDoneCallback(Executor, DexoptDoneCallback)} with the
* reason {@link ReasonMapping#REASON_BG_DEXOPT}.
+ *
+ * @throws RuntimeException if called during boot before the job scheduler service has started.
*/
public @ScheduleStatus int scheduleBackgroundDexoptJob() {
return mInjector.getBackgroundDexoptJob().schedule();
@@ -803,16 +828,18 @@
@NonNull Set<String> excludedPackages, @NonNull CancellationSignal cancellationSignal,
@NonNull Executor executor) {
if (shouldDowngrade()) {
- List<String> packages = getDefaultPackages(snapshot, ReasonMapping.REASON_INACTIVE)
+ List<String> packages = getDefaultPackages(
+ snapshot, ReasonMapping.REASON_INACTIVE, mInjector.getAppHibernationManager())
.stream()
.filter(pkg -> !excludedPackages.contains(pkg))
.collect(Collectors.toList());
if (!packages.isEmpty()) {
Log.i(TAG, "Storage is low. Downgrading inactive packages");
- mInjector.getDexoptHelper().dexopt(snapshot, packages,
- new DexoptParams.Builder(ReasonMapping.REASON_INACTIVE).build(),
- cancellationSignal, executor, null /* processCallbackExecutor */,
- null /* progressCallback */);
+ DexoptParams params =
+ new DexoptParams.Builder(ReasonMapping.REASON_INACTIVE).build();
+ mInjector.getDexoptHelper(mInjector.getAppHibernationManager())
+ .dexopt(snapshot, packages, params, cancellationSignal, executor,
+ null /* processCallbackExecutor */, null /* progressCallback */);
} else {
Log.i(TAG,
"Storage is low, but downgrading is disabled or there's nothing to "
@@ -833,25 +860,9 @@
/** Returns the list of packages to process for the given reason. */
@NonNull
- private List<String> getDefaultPackages(
- @NonNull PackageManagerLocal.FilteredSnapshot snapshot, @NonNull String reason) {
- // We probably won't have an app hibernation manager in the boot time compilation, because
- // ArtManagerLocal.onBoot needs to run early to ensure apps are compiled before the system
- // server fires them up. This means the boot time compilation will ignore the hibernation
- // states of the packages.
- //
- // TODO(b/265782156): When hibernated packages get compiled this way, the file GC will
- // delete them again in the next background dexopt run. That means they are likely to get
- // recreated again in the next boot dexopt (i.e. for OTA or Mainline update).
- var appHibernationManager = mInjector.getAppHibernationManager();
- if (reason != ReasonMapping.REASON_FIRST_BOOT
- && reason != ReasonMapping.REASON_BOOT_AFTER_OTA
- && reason != ReasonMapping.REASON_BOOT_AFTER_MAINLINE_UPDATE) {
- // Check that it's present for other compilation reasons, to ensure we don't regress
- // silently.
- Objects.requireNonNull(appHibernationManager);
- }
-
+ private List<String> getDefaultPackages(@NonNull PackageManagerLocal.FilteredSnapshot snapshot,
+ @NonNull /* @BatchDexoptReason|REASON_INACTIVE */ String reason,
+ @Nullable AppHibernationManager appHibernationManager) {
// Filter out hibernating packages even if the reason is REASON_INACTIVE. This is because
// artifacts for hibernating packages are already deleted.
Stream<PackageState> packages = snapshot.getPackageStates().values().stream().filter(
@@ -1009,23 +1020,24 @@
*/
@VisibleForTesting
public static class Injector {
+ @Nullable private final ArtManagerLocal mArtManagerLocal;
@Nullable private final Context mContext;
@Nullable private final PackageManagerLocal mPackageManagerLocal;
@Nullable private final Config mConfig;
- @Nullable private final BackgroundDexoptJob mBgDexoptJob;
+ @Nullable private BackgroundDexoptJob mBgDexoptJob = null;
Injector(@NonNull ArtManagerLocal artManagerLocal, @Nullable Context context) {
+ mArtManagerLocal = artManagerLocal;
mContext = context;
if (context != null) {
// We only need them on Android U and above, where a context is passed.
mPackageManagerLocal = Objects.requireNonNull(
LocalManagerRegistry.getManager(PackageManagerLocal.class));
mConfig = new Config();
- mBgDexoptJob = new BackgroundDexoptJob(context, artManagerLocal, mConfig);
// Call the getters for the dependencies that aren't optional, to ensure correct
// initialization order.
- getDexoptHelper();
+ getDexoptHelper(null);
getUserManager();
getDexUseManager();
getStorageManager();
@@ -1033,7 +1045,6 @@
} else {
mPackageManagerLocal = null;
mConfig = null;
- mBgDexoptJob = null;
}
}
@@ -1052,9 +1063,16 @@
return Utils.getArtd();
}
+ /**
+ * Returns a new {@link DexoptHelper} instance.
+ *
+ * The {@link AppHibernationManager} reference may be null for boot time compilation runs,
+ * when the app hibernation manager hasn't yet been initialized. It should not be null
+ * otherwise. See comment in {@link ArtManagerLocal.dexoptPackages} for more details.
+ */
@NonNull
- public DexoptHelper getDexoptHelper() {
- return new DexoptHelper(getContext(), getConfig());
+ public DexoptHelper getDexoptHelper(@Nullable AppHibernationManager appHibernationManager) {
+ return new DexoptHelper(getContext(), getConfig(), appHibernationManager);
}
@NonNull
@@ -1063,20 +1081,28 @@
}
/**
- * Returns the registered AppHibernationManager instance.
+ * Returns the registered {@link AppHibernationManager} instance.
*
- * It may be null because ArtManagerLocal needs to be available early to compile packages at
- * boot with {@link onBoot}, before the hibernation manager has been initialized. It should
- * not be null for other dexopt calls.
+ * @throws RuntimeException if called during boot before the app hibernation manager has
+ * started.
*/
- @Nullable
+ @NonNull
public AppHibernationManager getAppHibernationManager() {
- return mContext.getSystemService(AppHibernationManager.class);
+ return Objects.requireNonNull(mContext.getSystemService(AppHibernationManager.class));
}
+ /**
+ * Returns the {@link BackgroundDexoptJob} instance.
+ *
+ * @throws RuntimeException if called during boot before the job scheduler service has
+ * started.
+ */
@NonNull
- public BackgroundDexoptJob getBackgroundDexoptJob() {
- return Objects.requireNonNull(mBgDexoptJob);
+ public synchronized BackgroundDexoptJob getBackgroundDexoptJob() {
+ if (mBgDexoptJob == null) {
+ mBgDexoptJob = new BackgroundDexoptJob(mContext, mArtManagerLocal, mConfig);
+ }
+ return mBgDexoptJob;
}
@NonNull
diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java
index 9f03107..e6ba579 100644
--- a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java
+++ b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java
@@ -42,6 +42,7 @@
import com.google.auto.value.AutoValue;
+import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
@@ -291,7 +292,8 @@
@NonNull
public PackageManagerLocal getPackageManagerLocal() {
- return LocalManagerRegistry.getManager(PackageManagerLocal.class);
+ return Objects.requireNonNull(
+ LocalManagerRegistry.getManager(PackageManagerLocal.class));
}
@NonNull
@@ -301,7 +303,7 @@
@NonNull
public JobScheduler getJobScheduler() {
- return mContext.getSystemService(JobScheduler.class);
+ return Objects.requireNonNull(mContext.getSystemService(JobScheduler.class));
}
}
}
diff --git a/libartservice/service/java/com/android/server/art/DexoptHelper.java b/libartservice/service/java/com/android/server/art/DexoptHelper.java
index d139bfa..cc08a84 100644
--- a/libartservice/service/java/com/android/server/art/DexoptHelper.java
+++ b/libartservice/service/java/com/android/server/art/DexoptHelper.java
@@ -76,8 +76,16 @@
@NonNull private final Injector mInjector;
- public DexoptHelper(@NonNull Context context, @NonNull Config config) {
- this(new Injector(context, config));
+ /**
+ * Constructs a new instance.
+ *
+ * The {@link AppHibernationManager} reference may be null for boot time compilation runs, when
+ * the app hibernation manager hasn't yet been initialized. It should not be null otherwise. See
+ * comment in {@link ArtManagerLocal.dexoptPackages} for more details.
+ */
+ public DexoptHelper(@NonNull Context context, @NonNull Config config,
+ @Nullable AppHibernationManager appHibernationManager) {
+ this(new Injector(context, config, appHibernationManager));
}
@VisibleForTesting
@@ -307,10 +315,13 @@
public static class Injector {
@NonNull private final Context mContext;
@NonNull private final Config mConfig;
+ @Nullable private final AppHibernationManager mAppHibernationManager;
- Injector(@NonNull Context context, @NonNull Config config) {
+ Injector(@NonNull Context context, @NonNull Config config,
+ @Nullable AppHibernationManager appHibernationManager) {
mContext = context;
mConfig = config;
+ mAppHibernationManager = appHibernationManager;
// Call the getters for the dependencies that aren't optional, to ensure correct
// initialization order.
@@ -331,16 +342,9 @@
return new SecondaryDexopter(mContext, pkgState, pkg, params, cancellationSignal);
}
- /**
- * Returns the registered AppHibernationManager instance.
- *
- * It may be null because ArtManagerLocal needs to be available early to compile packages at
- * boot with {@link onBoot}, before the hibernation manager has been initialized. It should
- * not be null for other dexopt calls.
- */
@Nullable
public AppHibernationManager getAppHibernationManager() {
- return mContext.getSystemService(AppHibernationManager.class);
+ return mAppHibernationManager;
}
@NonNull
diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
index 54a6a77..08db07f 100644
--- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
+++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
@@ -140,7 +140,7 @@
// that each test case examines.
lenient().when(mInjector.getPackageManagerLocal()).thenReturn(mPackageManagerLocal);
lenient().when(mInjector.getArtd()).thenReturn(mArtd);
- lenient().when(mInjector.getDexoptHelper()).thenReturn(mDexoptHelper);
+ lenient().when(mInjector.getDexoptHelper(any())).thenReturn(mDexoptHelper);
lenient().when(mInjector.getConfig()).thenReturn(mConfig);
lenient().when(mInjector.getAppHibernationManager()).thenReturn(mAppHibernationManager);
lenient().when(mInjector.getUserManager()).thenReturn(mUserManager);