From c98c7bccdccbff1ceeda501e6a6f8c21d61649ca Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 7 Dec 2016 14:57:34 -0700 Subject: [PATCH] Prepare to move dexopt calls to Binder. Since InstallerConnection is about to be replaced by a new installd Binder interface, this change moves OtaDexoptService to override and manually cook up the 'dexopt' command line that it expects to collect from PackageDexOptimizer. Since OtaDexoptService is designed to be run in isolation, add a new mode to Installer which ignores calls that aren't being intercepted. Also moves to a single dexopt() method instead of having overloads. Test: builds, boots, fake OTA works Bug: 13758960, 30944031 Change-Id: I3a6a115289f1542d6df3e2993b9720118b7d1e8d --- .../android/internal/os/InstallerConnection.java | 7 -- core/java/com/android/internal/os/ZygoteInit.java | 12 ++- .../core/java/com/android/server/pm/Installer.java | 113 ++++++++++++--------- .../com/android/server/pm/OtaDexoptService.java | 85 +++++++++------- .../android/server/pm/PackageManagerService.java | 5 +- 5 files changed, 124 insertions(+), 98 deletions(-) diff --git a/core/java/com/android/internal/os/InstallerConnection.java b/core/java/com/android/internal/os/InstallerConnection.java index 419c3d85599d..62f674c38329 100644 --- a/core/java/com/android/internal/os/InstallerConnection.java +++ b/core/java/com/android/internal/os/InstallerConnection.java @@ -134,13 +134,6 @@ public class InstallerConnection { return resRaw; } - public void dexopt(String apkPath, int uid, String instructionSet, int dexoptNeeded, - int dexFlags, String compilerFilter, String volumeUuid, String sharedLibraries) - throws InstallerException { - dexopt(apkPath, uid, "*", instructionSet, dexoptNeeded, null /*outputPath*/, dexFlags, - compilerFilter, volumeUuid, sharedLibraries); - } - public void dexopt(String apkPath, int uid, String pkgName, String instructionSet, int dexoptNeeded, String outputPath, int dexFlags, String compilerFilter, String volumeUuid, String sharedLibraries) throws InstallerException { diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java index e1118d84ed7b..5b46a09eb9af 100644 --- a/core/java/com/android/internal/os/ZygoteInit.java +++ b/core/java/com/android/internal/os/ZygoteInit.java @@ -31,6 +31,7 @@ import android.os.SystemClock; import android.os.SystemProperties; import android.os.Trace; import android.os.ZygoteProcess; +import android.os.storage.StorageManager; import android.security.keystore.AndroidKeyStoreProvider; import android.system.ErrnoException; import android.system.Os; @@ -522,10 +523,15 @@ public class ZygoteInit { } if (dexoptNeeded != DexFile.NO_DEXOPT_NEEDED) { + final String packageName = "*"; + final String outputPath = null; + final int dexFlags = 0; + final String compilerFilter = "speed"; + final String uuid = StorageManager.UUID_PRIVATE_INTERNAL; try { - installer.dexopt(classPathElement, Process.SYSTEM_UID, instructionSet, - dexoptNeeded, 0 /*dexFlags*/, "speed", null /*volumeUuid*/, - sharedLibraries); + installer.dexopt(classPathElement, Process.SYSTEM_UID, packageName, + instructionSet, dexoptNeeded, outputPath, dexFlags, compilerFilter, + uuid, sharedLibraries); } catch (InstallerException e) { // Ignore (but log), we need this on the classpath for fallback mode. Log.w(TAG, "Failed compiling classpath element for system server: " diff --git a/services/core/java/com/android/server/pm/Installer.java b/services/core/java/com/android/server/pm/Installer.java index bff68d85b4e5..f00065f9a654 100644 --- a/services/core/java/com/android/server/pm/Installer.java +++ b/services/core/java/com/android/server/pm/Installer.java @@ -34,7 +34,7 @@ import dalvik.system.VMRuntime; import java.util.Arrays; -public final class Installer extends SystemService { +public class Installer extends SystemService { private static final String TAG = "Installer"; /* *************************************************************************** @@ -58,25 +58,33 @@ public final class Installer extends SystemService { public static final int FLAG_CLEAR_CACHE_ONLY = 1 << 8; public static final int FLAG_CLEAR_CODE_CACHE_ONLY = 1 << 9; + private final boolean mIsolated; + + // TODO: reconnect if installd restarts private final InstallerConnection mInstaller; private final IInstalld mInstalld; private volatile Object mWarnIfHeld; public Installer(Context context) { - super(context); - mInstaller = new InstallerConnection(); - // TODO: reconnect if installd restarts - mInstalld = IInstalld.Stub.asInterface(ServiceManager.getService("installd")); + this(context, false); } - // Package-private installer that accepts a custom InstallerConnection. Used for - // OtaDexoptService. - Installer(Context context, InstallerConnection connection) { + /** + * @param isolated indicates if this object should not connect to + * the real {@code installd}. All remote calls will be ignored + * unless you extend this class and intercept them. + */ + public Installer(Context context, boolean isolated) { super(context); - mInstaller = connection; - // TODO: reconnect if installd restarts - mInstalld = IInstalld.Stub.asInterface(ServiceManager.getService("installd")); + mIsolated = isolated; + if (isolated) { + mInstaller = null; + mInstalld = null; + } else { + mInstaller = new InstallerConnection(); + mInstalld = IInstalld.Stub.asInterface(ServiceManager.getService("installd")); + } } /** @@ -84,26 +92,41 @@ public final class Installer extends SystemService { * the given object. */ public void setWarnIfHeld(Object warnIfHeld) { - mInstaller.setWarnIfHeld(warnIfHeld); + if (mInstaller != null) { + mInstaller.setWarnIfHeld(warnIfHeld); + } mWarnIfHeld = warnIfHeld; } @Override public void onStart() { - Slog.i(TAG, "Waiting for installd to be ready."); - mInstaller.waitForConnection(); + if (mInstaller != null) { + Slog.i(TAG, "Waiting for installd to be ready."); + mInstaller.waitForConnection(); + } } - private void checkLock() { + /** + * Do several pre-flight checks before making a remote call. + * + * @return if the remote call should continue. + */ + private boolean checkBeforeRemote() { if (mWarnIfHeld != null && Thread.holdsLock(mWarnIfHeld)) { Slog.wtf(TAG, "Calling thread " + Thread.currentThread().getName() + " is holding 0x" + Integer.toHexString(System.identityHashCode(mWarnIfHeld)), new Throwable()); } + if (mIsolated) { + Slog.i(TAG, "Ignoring request because this installer is isolated"); + return false; + } else { + return true; + } } public void createAppData(String uuid, String packageName, int userId, int flags, int appId, String seInfo, int targetSdkVersion) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.createAppData(uuid, packageName, userId, flags, appId, seInfo, targetSdkVersion); @@ -114,7 +137,7 @@ public final class Installer extends SystemService { public void restoreconAppData(String uuid, String packageName, int userId, int flags, int appId, String seInfo) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.restoreconAppData(uuid, packageName, userId, flags, appId, seInfo); } catch (RemoteException | ServiceSpecificException e) { @@ -124,7 +147,7 @@ public final class Installer extends SystemService { public void migrateAppData(String uuid, String packageName, int userId, int flags) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.migrateAppData(uuid, packageName, userId, flags); } catch (RemoteException | ServiceSpecificException e) { @@ -134,7 +157,7 @@ public final class Installer extends SystemService { public void clearAppData(String uuid, String packageName, int userId, int flags, long ceDataInode) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.clearAppData(uuid, packageName, userId, flags, ceDataInode); } catch (RemoteException | ServiceSpecificException e) { @@ -144,7 +167,7 @@ public final class Installer extends SystemService { public void destroyAppData(String uuid, String packageName, int userId, int flags, long ceDataInode) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.destroyAppData(uuid, packageName, userId, flags, ceDataInode); } catch (RemoteException | ServiceSpecificException e) { @@ -155,7 +178,7 @@ public final class Installer extends SystemService { public void moveCompleteApp(String fromUuid, String toUuid, String packageName, String dataAppName, int appId, String seInfo, int targetSdkVersion) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.moveCompleteApp(fromUuid, toUuid, packageName, dataAppName, appId, seInfo, targetSdkVersion); @@ -166,6 +189,7 @@ public final class Installer extends SystemService { public void getAppSize(String uuid, String pkgname, int userid, int flags, long ceDataInode, String codePath, PackageStats stats) throws InstallerException { + if (!checkBeforeRemote()) return; final String[] res = mInstaller.execute("get_app_size", uuid, pkgname, userid, flags, ceDataInode, codePath); try { @@ -179,7 +203,7 @@ public final class Installer extends SystemService { public long getAppDataInode(String uuid, String packageName, int userId, int flags) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return -1; try { return mInstalld.getAppDataInode(uuid, packageName, userId, flags); } catch (RemoteException | ServiceSpecificException e) { @@ -187,25 +211,18 @@ public final class Installer extends SystemService { } } - public void dexopt(String apkPath, int uid, String instructionSet, int dexoptNeeded, - int dexFlags, String compilerFilter, String volumeUuid, String sharedLibraries) - throws InstallerException { - assertValidInstructionSet(instructionSet); - mInstaller.dexopt(apkPath, uid, instructionSet, dexoptNeeded, dexFlags, - compilerFilter, volumeUuid, sharedLibraries); - } - - public void dexopt(String apkPath, int uid, String pkgName, String instructionSet, + public void dexopt(String apkPath, int uid, @Nullable String pkgName, String instructionSet, int dexoptNeeded, @Nullable String outputPath, int dexFlags, - String compilerFilter, String volumeUuid, String sharedLibraries) + String compilerFilter, @Nullable String volumeUuid, @Nullable String sharedLibraries) throws InstallerException { assertValidInstructionSet(instructionSet); + if (!checkBeforeRemote()) return; mInstaller.dexopt(apkPath, uid, pkgName, instructionSet, dexoptNeeded, outputPath, dexFlags, compilerFilter, volumeUuid, sharedLibraries); } public boolean mergeProfiles(int uid, String packageName) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return false; try { return mInstalld.mergeProfiles(uid, packageName); } catch (RemoteException | ServiceSpecificException e) { @@ -215,7 +232,7 @@ public final class Installer extends SystemService { public boolean dumpProfiles(int uid, String packageName, String codePaths) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return false; try { return mInstalld.dumpProfiles(uid, packageName, codePaths); } catch (RemoteException | ServiceSpecificException e) { @@ -225,7 +242,7 @@ public final class Installer extends SystemService { public void idmap(String targetApkPath, String overlayApkPath, int uid) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.idmap(targetApkPath, overlayApkPath, uid); } catch (RemoteException | ServiceSpecificException e) { @@ -235,7 +252,7 @@ public final class Installer extends SystemService { public void rmdex(String codePath, String instructionSet) throws InstallerException { assertValidInstructionSet(instructionSet); - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.rmdex(codePath, instructionSet); } catch (RemoteException | ServiceSpecificException e) { @@ -244,7 +261,7 @@ public final class Installer extends SystemService { } public void rmPackageDir(String packageDir) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.rmPackageDir(packageDir); } catch (RemoteException | ServiceSpecificException e) { @@ -253,7 +270,7 @@ public final class Installer extends SystemService { } public void clearAppProfiles(String packageName) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.clearAppProfiles(packageName); } catch (RemoteException | ServiceSpecificException e) { @@ -262,7 +279,7 @@ public final class Installer extends SystemService { } public void destroyAppProfiles(String packageName) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.destroyAppProfiles(packageName); } catch (RemoteException | ServiceSpecificException e) { @@ -272,7 +289,7 @@ public final class Installer extends SystemService { public void createUserData(String uuid, int userId, int userSerial, int flags) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.createUserData(uuid, userId, userSerial, flags); } catch (RemoteException | ServiceSpecificException e) { @@ -281,7 +298,7 @@ public final class Installer extends SystemService { } public void destroyUserData(String uuid, int userId, int flags) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.destroyUserData(uuid, userId, flags); } catch (RemoteException | ServiceSpecificException e) { @@ -291,7 +308,7 @@ public final class Installer extends SystemService { public void markBootComplete(String instructionSet) throws InstallerException { assertValidInstructionSet(instructionSet); - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.markBootComplete(instructionSet); } catch (RemoteException | ServiceSpecificException e) { @@ -300,7 +317,7 @@ public final class Installer extends SystemService { } public void freeCache(String uuid, long freeStorageSize) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.freeCache(uuid, freeStorageSize); } catch (RemoteException | ServiceSpecificException e) { @@ -315,7 +332,7 @@ public final class Installer extends SystemService { */ public void linkNativeLibraryDirectory(String uuid, String packageName, String nativeLibPath32, int userId) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.linkNativeLibraryDirectory(uuid, packageName, nativeLibPath32, userId); } catch (RemoteException | ServiceSpecificException e) { @@ -325,7 +342,7 @@ public final class Installer extends SystemService { public void createOatDir(String oatDir, String dexInstructionSet) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.createOatDir(oatDir, dexInstructionSet); } catch (RemoteException | ServiceSpecificException e) { @@ -335,7 +352,7 @@ public final class Installer extends SystemService { public void linkFile(String relativePath, String fromBase, String toBase) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.linkFile(relativePath, fromBase, toBase); } catch (RemoteException | ServiceSpecificException e) { @@ -345,7 +362,7 @@ public final class Installer extends SystemService { public void moveAb(String apkPath, String instructionSet, String outputPath) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.moveAb(apkPath, instructionSet, outputPath); } catch (RemoteException | ServiceSpecificException e) { @@ -355,7 +372,7 @@ public final class Installer extends SystemService { public void deleteOdex(String apkPath, String instructionSet, String outputPath) throws InstallerException { - checkLock(); + if (!checkBeforeRemote()) return; try { mInstalld.deleteOdex(apkPath, instructionSet, outputPath); } catch (RemoteException | ServiceSpecificException e) { diff --git a/services/core/java/com/android/server/pm/OtaDexoptService.java b/services/core/java/com/android/server/pm/OtaDexoptService.java index 689917cd670a..c0ae2729fac1 100644 --- a/services/core/java/com/android/server/pm/OtaDexoptService.java +++ b/services/core/java/com/android/server/pm/OtaDexoptService.java @@ -21,6 +21,7 @@ import static com.android.server.pm.InstructionSets.getAppDexInstructionSets; import static com.android.server.pm.InstructionSets.getDexCodeInstructionSets; import static com.android.server.pm.PackageManagerServiceCompilerMapping.getCompilerFilterForReason; +import android.annotation.Nullable; import android.content.Context; import android.content.pm.IOtaDexopt; import android.content.pm.PackageParser; @@ -29,15 +30,17 @@ import android.os.RemoteException; import android.os.ResultReceiver; import android.os.ServiceManager; import android.os.storage.StorageManager; +import android.text.TextUtils; import android.util.Log; import android.util.Slog; + import com.android.internal.logging.MetricsLogger; -import com.android.internal.os.InstallerConnection; import com.android.internal.os.InstallerConnection.InstallerException; import java.io.File; import java.io.FileDescriptor; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.concurrent.TimeUnit; @@ -276,9 +279,27 @@ public class OtaDexoptService extends IOtaDexopt.Stub { */ private synchronized List generatePackageDexopts(PackageParser.Package pkg, int compilationReason) { - // Use our custom connection that just collects the commands. - RecordingInstallerConnection collectingConnection = new RecordingInstallerConnection(); - Installer collectingInstaller = new Installer(mContext, collectingConnection); + // Intercept and collect dexopt requests + final List commands = new ArrayList(); + final Installer collectingInstaller = new Installer(mContext, true) { + @Override + public void dexopt(String apkPath, int uid, @Nullable String pkgName, + String instructionSet, int dexoptNeeded, @Nullable String outputPath, + int dexFlags, String compilerFilter, @Nullable String volumeUuid, + @Nullable String sharedLibraries) throws InstallerException { + commands.add(buildCommand("dexopt", + apkPath, + uid, + pkgName, + instructionSet, + dexoptNeeded, + outputPath, + dexFlags, + compilerFilter, + volumeUuid, + sharedLibraries)); + } + }; // Use the package manager install and install lock here for the OTA dex optimizer. PackageDexOptimizer optimizer = new OTADexoptPackageDexOptimizer( @@ -295,7 +316,7 @@ public class OtaDexoptService extends IOtaDexopt.Stub { getCompilerFilterForReason(compilationReason), null /* CompilerStats.PackageStats */); - return collectingConnection.commands; + return commands; } @Override @@ -414,39 +435,27 @@ public class OtaDexoptService extends IOtaDexopt.Stub { } - private static class RecordingInstallerConnection extends InstallerConnection { - public List commands = new ArrayList(1); - - @Override - public void setWarnIfHeld(Object warnIfHeld) { - throw new IllegalStateException("Should not reach here"); - } - - @Override - public synchronized String transact(String cmd) { - commands.add(cmd); - return "0"; - } - - @Override - public boolean mergeProfiles(int uid, String pkgName) throws InstallerException { - throw new IllegalStateException("Should not reach here"); - } - - @Override - public boolean dumpProfiles(String gid, String packageName, String codePaths) - throws InstallerException { - throw new IllegalStateException("Should not reach here"); - } - - @Override - public void disconnect() { - throw new IllegalStateException("Should not reach here"); - } - - @Override - public void waitForConnection() { - throw new IllegalStateException("Should not reach here"); + /** + * Cook up argument list in the format that {@code installd} expects. + */ + private static String buildCommand(Object... args) { + final StringBuilder builder = new StringBuilder(); + for (Object arg : args) { + String escaped; + if (arg == null) { + escaped = ""; + } else { + escaped = String.valueOf(arg); + } + if (escaped.indexOf('\0') != -1 || escaped.indexOf(' ') != -1 || "!".equals(escaped)) { + throw new IllegalArgumentException( + "Invalid argument while executing " + Arrays.toString(args)); + } + if (TextUtils.isEmpty(escaped)) { + escaped = "!"; + } + builder.append(' ').append(escaped); } + return builder.toString(); } } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 9102fee89422..81c31b8bed5d 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2228,8 +2228,9 @@ public class PackageManagerService extends IPackageManager.Stub { getCompilerFilterForReason(REASON_SHARED_APK), false /* newProfile */); if (dexoptNeeded != DexFile.NO_DEXOPT_NEEDED) { - mInstaller.dexopt(lib, Process.SYSTEM_UID, dexCodeInstructionSet, - dexoptNeeded, DEXOPT_PUBLIC /*dexFlags*/, + mInstaller.dexopt(lib, Process.SYSTEM_UID, "*", + dexCodeInstructionSet, dexoptNeeded, null, + DEXOPT_PUBLIC, getCompilerFilterForReason(REASON_SHARED_APK), StorageManager.UUID_PRIVATE_INTERNAL, SKIP_SHARED_LIBRARY_CHECK); -- 2.11.0