diff options
| author | 2014-09-06 12:20:37 -0700 | |
|---|---|---|
| committer | 2014-09-08 09:31:49 -0700 | |
| commit | 77d218e1869e69c8d436b09cd11dcfe45e50b2cf (patch) | |
| tree | 300bdb350c3927fea1651edea80b03b9535201d2 | |
| parent | 5341f94a191ae57b138f97cd516842bcf04c0609 (diff) | |
Delayed ASEC allocation, refine progress handling.
For restore use-case, session creation needs to complete quickly, so
delay ASEC allocation until session is opened. When preflighting
size checks, only consider external when we have a known size for the
container. Also relax size checks when using MODE_INHERIT_EXISTING
on external, since we don't know how much of existing app will be
copied over.
Consider session as "active" while commit is ongoing, until we're
either finished or pending user interaction.
Always publish first client needle movement away from 0. Use 25% of
internal progress to reflect ASEC allocation.
Avoid CloseGuard messages about leaking PFDs.
Bug: 17405741, 17402982
Change-Id: I6247a1d335d26621549c701c4c4575a8d16ef8c2
6 files changed, 228 insertions, 158 deletions
diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java index 0a211cfa8644..06d4c4ab8d4f 100644 --- a/core/java/android/content/pm/PackageInstaller.java +++ b/core/java/android/content/pm/PackageInstaller.java @@ -285,6 +285,9 @@ public class PackageInstaller { * * @throws IOException if parameters were unsatisfiable, such as lack of * disk space or unavailable media. + * @throws SecurityException when installation services are unavailable, + * such as when called from a restricted user. + * @throws IllegalArgumentException when {@link SessionParams} is invalid. * @return positive, non-zero unique ID that represents the created session. * This ID remains consistent across device reboots until the * session is finalized. IDs are not reused during a given boot. @@ -303,6 +306,11 @@ public class PackageInstaller { /** * Open an existing session to actively perform work. To succeed, the caller * must be the owner of the install session. + * + * @throws IOException if parameters were unsatisfiable, such as lack of + * disk space or unavailable media. + * @throws SecurityException when the caller does not own the session, or + * the session is invalid. */ public @NonNull Session openSession(int sessionId) throws IOException { try { @@ -319,6 +327,9 @@ public class PackageInstaller { * Update the icon representing the app being installed in a specific * session. This should be roughly * {@link ActivityManager#getLauncherLargeIconSize()} in both dimensions. + * + * @throws SecurityException when the caller does not own the session, or + * the session is invalid. */ public void updateSessionAppIcon(int sessionId, @Nullable Bitmap appIcon) { try { @@ -331,6 +342,9 @@ public class PackageInstaller { /** * Update the label representing the app being installed in a specific * session. + * + * @throws SecurityException when the caller does not own the session, or + * the session is invalid. */ public void updateSessionAppLabel(int sessionId, @Nullable CharSequence appLabel) { try { @@ -341,6 +355,15 @@ public class PackageInstaller { } } + /** + * Completely abandon the given session, destroying all staged data and + * rendering it invalid. Abandoned sessions will be reported to + * {@link SessionCallback} listeners as failures. This is equivalent to + * opening the session and calling {@link Session#abandon()}. + * + * @throws SecurityException when the caller does not own the session, or + * the session is invalid. + */ public void abandonSession(int sessionId) { try { mInstaller.abandonSession(sessionId); @@ -350,7 +373,11 @@ public class PackageInstaller { } /** - * Return details for a specific session. + * Return details for a specific session. No special permissions are + * required to retrieve these details. + * + * @return details for the requested session, or {@code null} if the session + * does not exist. */ public @Nullable SessionInfo getSessionInfo(int sessionId) { try { @@ -361,7 +388,7 @@ public class PackageInstaller { } /** - * Return list of all active install sessions, regardless of the installer. + * Return list of all known install sessions, regardless of the installer. */ public @NonNull List<SessionInfo> getAllSessions() { final ApplicationInfo info = mContext.getApplicationInfo(); @@ -379,7 +406,7 @@ public class PackageInstaller { } /** - * Return list of all install sessions owned by the calling app. + * Return list of all known install sessions owned by the calling app. */ public @NonNull List<SessionInfo> getMySessions() { try { @@ -547,7 +574,8 @@ public class PackageInstaller { } /** - * Register to watch for session lifecycle events. + * Register to watch for session lifecycle events. No special permissions + * are required to watch for these events. */ public void registerSessionCallback(@NonNull SessionCallback callback) { registerSessionCallback(callback, new Handler()); @@ -560,7 +588,8 @@ public class PackageInstaller { } /** - * Register to watch for session lifecycle events. + * Register to watch for session lifecycle events. No special permissions + * are required to watch for these events. * * @param handler to dispatch callback events through, otherwise uses * calling thread. @@ -593,7 +622,7 @@ public class PackageInstaller { } /** - * Unregister an existing callback. + * Unregister a previously registered callback. */ public void unregisterSessionCallback(@NonNull SessionCallback callback) { synchronized (mDelegates) { @@ -686,6 +715,12 @@ public class PackageInstaller { * start at the beginning of the file. * @param lengthBytes total size of the file being written, used to * preallocate the underlying disk space, or -1 if unknown. + * The system may clear various caches as needed to allocate + * this space. + * @throws IOException if trouble opening the file for writing, such as + * lack of disk space or unavailable media. + * @throws SecurityException if called after the session has been + * committed or abandoned. */ public @NonNull OutputStream openWrite(@NonNull String name, long offsetBytes, long lengthBytes) throws IOException { @@ -719,6 +754,9 @@ public class PackageInstaller { * <p> * This returns all names which have been previously written through * {@link #openWrite(String, long, long)} as part of this session. + * + * @throws SecurityException if called after the session has been + * committed or abandoned. */ public @NonNull String[] getNames() throws IOException { try { @@ -738,6 +776,9 @@ public class PackageInstaller { * through {@link #openWrite(String, long, long)} as part of this * session. For example, this stream may be used to calculate a * {@link MessageDigest} of a written APK before committing. + * + * @throws SecurityException if called after the session has been + * committed or abandoned. */ public @NonNull InputStream openRead(@NonNull String name) throws IOException { try { @@ -759,6 +800,9 @@ public class PackageInstaller { * Once this method is called, no additional mutations may be performed * on the session. If the device reboots before the session has been * finalized, you may commit the session again. + * + * @throws SecurityException if streams opened through + * {@link #openWrite(String, long, long)} are still open. */ public void commit(@NonNull IntentSender statusReceiver) { try { @@ -783,7 +827,9 @@ public class PackageInstaller { /** * Completely abandon this session, destroying all staged data and - * rendering it invalid. + * rendering it invalid. Abandoned sessions will be reported to + * {@link SessionCallback} listeners as failures. This is equivalent to + * opening the session and calling {@link Session#abandon()}. */ public void abandon() { try { @@ -937,6 +983,18 @@ public class PackageInstaller { } /** {@hide} */ + public void setInstallFlagsInternal() { + installFlags |= PackageManager.INSTALL_INTERNAL; + installFlags &= ~PackageManager.INSTALL_EXTERNAL; + } + + /** {@hide} */ + public void setInstallFlagsExternal() { + installFlags |= PackageManager.INSTALL_EXTERNAL; + installFlags &= ~PackageManager.INSTALL_INTERNAL; + } + + /** {@hide} */ public void dump(IndentingPrintWriter pw) { pw.printPair("mode", mode); pw.printHexPair("installFlags", installFlags); diff --git a/core/java/android/os/FileBridge.java b/core/java/android/os/FileBridge.java index 022a106e5cd3..0acf24bac52a 100644 --- a/core/java/android/os/FileBridge.java +++ b/core/java/android/os/FileBridge.java @@ -75,6 +75,13 @@ public class FileBridge extends Thread { return mClosed; } + public void forceClose() { + IoUtils.closeQuietly(mTarget); + IoUtils.closeQuietly(mServer); + IoUtils.closeQuietly(mClient); + mClosed = true; + } + public void setTargetFile(FileDescriptor target) { mTarget = target; } @@ -89,7 +96,6 @@ public class FileBridge extends Thread { try { while (IoBridge.read(mServer, temp, 0, MSG_LENGTH) == MSG_LENGTH) { final int cmd = Memory.peekInt(temp, 0, ByteOrder.BIG_ENDIAN); - if (cmd == CMD_WRITE) { // Shuttle data into local file int len = Memory.peekInt(temp, 4, ByteOrder.BIG_ENDIAN); @@ -118,15 +124,10 @@ public class FileBridge extends Thread { } } - } catch (ErrnoException e) { - Log.wtf(TAG, "Failed during bridge", e); - } catch (IOException e) { + } catch (ErrnoException | IOException e) { Log.wtf(TAG, "Failed during bridge", e); } finally { - IoUtils.closeQuietly(mTarget); - IoUtils.closeQuietly(mServer); - IoUtils.closeQuietly(mClient); - mClosed = true; + forceClose(); } } @@ -151,6 +152,7 @@ public class FileBridge extends Thread { writeCommandAndBlock(CMD_CLOSE, "close()"); } finally { IoBridge.closeAndSignalBlockedThreads(mClient); + IoUtils.closeQuietly(mClientPfd); } } diff --git a/core/java/com/android/internal/content/PackageHelper.java b/core/java/com/android/internal/content/PackageHelper.java index c17f4eeb41f4..7bdb4be2b35c 100644 --- a/core/java/com/android/internal/content/PackageHelper.java +++ b/core/java/com/android/internal/content/PackageHelper.java @@ -390,7 +390,10 @@ public class PackageHelper { if (!emulated && (checkBoth || prefer == RECOMMEND_INSTALL_EXTERNAL)) { final File target = new UserEnvironment(UserHandle.USER_OWNER) .getExternalStorageDirectory(); - fitsOnExternal = (sizeBytes <= storage.getStorageBytesUntilLow(target)); + // External is only an option when size is known + if (sizeBytes > 0) { + fitsOnExternal = (sizeBytes <= storage.getStorageBytesUntilLow(target)); + } } if (prefer == RECOMMEND_INSTALL_INTERNAL) { diff --git a/core/java/com/android/internal/util/XmlUtils.java b/core/java/com/android/internal/util/XmlUtils.java index 7db70bacec59..45d790b0dd9b 100644 --- a/core/java/com/android/internal/util/XmlUtils.java +++ b/core/java/com/android/internal/util/XmlUtils.java @@ -1440,6 +1440,16 @@ public class XmlUtils { return Boolean.parseBoolean(value); } + public static boolean readBooleanAttribute(XmlPullParser in, String name, + boolean defaultValue) { + final String value = in.getAttributeValue(null, name); + if (value == null) { + return defaultValue; + } else { + return Boolean.parseBoolean(value); + } + } + public static void writeBooleanAttribute(XmlSerializer out, String name, boolean value) throws IOException { out.attribute(null, name, Boolean.toString(value)); diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 89ea9052f425..496c136f9131 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -42,7 +42,6 @@ import android.content.Context; import android.content.Intent; import android.content.IntentSender; import android.content.IntentSender.SendIntentException; -import android.content.pm.ApplicationInfo; import android.content.pm.IPackageInstaller; import android.content.pm.IPackageInstallerCallback; import android.content.pm.IPackageInstallerSession; @@ -50,15 +49,11 @@ import android.content.pm.PackageInstaller; import android.content.pm.PackageInstaller.SessionInfo; import android.content.pm.PackageInstaller.SessionParams; import android.content.pm.PackageManager; -import android.content.pm.PackageParser; -import android.content.pm.PackageParser.PackageLite; -import android.content.pm.PackageParser.PackageParserException; import android.graphics.Bitmap; import android.net.Uri; import android.os.Binder; import android.os.Bundle; import android.os.Environment; -import android.os.Environment.UserEnvironment; import android.os.FileUtils; import android.os.Handler; import android.os.HandlerThread; @@ -70,7 +65,6 @@ import android.os.RemoteException; import android.os.SELinux; import android.os.UserHandle; import android.os.UserManager; -import android.os.storage.StorageManager; import android.system.ErrnoException; import android.system.Os; import android.text.TextUtils; @@ -126,6 +120,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub { private static final String ATTR_CREATED_MILLIS = "createdMillis"; private static final String ATTR_SESSION_STAGE_DIR = "sessionStageDir"; private static final String ATTR_SESSION_STAGE_CID = "sessionStageCid"; + private static final String ATTR_PREPARED = "prepared"; private static final String ATTR_SEALED = "sealed"; private static final String ATTR_MODE = "mode"; private static final String ATTR_INSTALL_FLAGS = "installFlags"; @@ -148,7 +143,6 @@ public class PackageInstallerService extends IPackageInstaller.Stub { private final Context mContext; private final PackageManagerService mPm; private final AppOpsManager mAppOps; - private final StorageManager mStorage; private final File mStagingDir; private final HandlerThread mInstallThread; @@ -190,7 +184,6 @@ public class PackageInstallerService extends IPackageInstaller.Stub { mContext = context; mPm = pm; mAppOps = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE); - mStorage = StorageManager.from(mContext); mStagingDir = stagingDir; @@ -266,7 +259,9 @@ public class PackageInstallerService extends IPackageInstaller.Stub { try { final int sessionId = allocateSessionIdLocked(); mLegacySessions.put(sessionId, true); - return prepareInternalStageDir(sessionId); + final File stageDir = buildInternalStageDir(sessionId); + prepareInternalStageDir(stageDir); + return stageDir; } catch (IllegalStateException e) { throw new IOException(e); } @@ -345,6 +340,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub { final String stageDirRaw = readStringAttribute(in, ATTR_SESSION_STAGE_DIR); final File stageDir = (stageDirRaw != null) ? new File(stageDirRaw) : null; final String stageCid = readStringAttribute(in, ATTR_SESSION_STAGE_CID); + final boolean prepared = readBooleanAttribute(in, ATTR_PREPARED, true); final boolean sealed = readBooleanAttribute(in, ATTR_SEALED); final SessionParams params = new SessionParams( @@ -362,7 +358,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub { return new PackageInstallerSession(mInternalCallback, mContext, mPm, mInstallThread.getLooper(), sessionId, userId, installerPackageName, params, - createdMillis, stageDir, stageCid, sealed); + createdMillis, stageDir, stageCid, prepared, sealed); } private void writeSessionsLocked() { @@ -410,6 +406,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub { if (session.stageCid != null) { writeStringAttribute(out, ATTR_SESSION_STAGE_CID, session.stageCid); } + writeBooleanAttribute(out, ATTR_PREPARED, session.isPrepared()); writeBooleanAttribute(out, ATTR_SEALED, session.isSealed()); writeIntAttribute(out, ATTR_MODE, params.mode); @@ -483,14 +480,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub { } } - // TODO: treat INHERIT_EXISTING as install for user - - // Figure out where we're going to be staging session data - final boolean stageInternal; - - if (params.mode == SessionParams.MODE_FULL_INSTALL) { - // Brand new install, use best resolved location. This also verifies - // that target has enough free space for the install. + if (params.mode == SessionParams.MODE_FULL_INSTALL + || params.mode == SessionParams.MODE_INHERIT_EXISTING) { + // Resolve best location for install, based on combination of + // requested install flags, delta size, and manifest settings. final long ident = Binder.clearCallingIdentity(); try { final int resolved = PackageHelper.resolveInstallLocation(mContext, @@ -498,46 +491,15 @@ public class PackageInstallerService extends IPackageInstaller.Stub { params.installFlags); if (resolved == PackageHelper.RECOMMEND_INSTALL_INTERNAL) { - stageInternal = true; + params.setInstallFlagsInternal(); } else if (resolved == PackageHelper.RECOMMEND_INSTALL_EXTERNAL) { - stageInternal = false; + params.setInstallFlagsExternal(); } else { throw new IOException("No storage with enough free space; res=" + resolved); } } finally { Binder.restoreCallingIdentity(ident); } - } else if (params.mode == SessionParams.MODE_INHERIT_EXISTING) { - // Inheriting existing install, so stay on the same storage medium. - final ApplicationInfo existingApp = mPm.getApplicationInfo(params.appPackageName, 0, - userId); - if (existingApp == null) { - throw new IllegalStateException( - "Missing existing app " + params.appPackageName); - } - - final long existingSize; - try { - final PackageLite existingPkg = PackageParser.parsePackageLite( - new File(existingApp.getCodePath()), 0); - existingSize = PackageHelper.calculateInstalledSize(existingPkg, false, - params.abiOverride); - } catch (PackageParserException e) { - throw new IllegalStateException( - "Failed to calculate size of " + params.appPackageName); - } - - if ((existingApp.flags & ApplicationInfo.FLAG_EXTERNAL_STORAGE) == 0) { - // Internal we can link existing install into place, so we only - // need enough space for the new data. - checkInternalStorage(params.sizeBytes); - stageInternal = true; - } else { - // External we're going to copy existing install into our - // container, so we need footprint of both. - checkExternalStorage(params.sizeBytes + existingSize); - stageInternal = false; - } } else { throw new IllegalArgumentException("Invalid install mode: " + params.mode); } @@ -563,15 +525,15 @@ public class PackageInstallerService extends IPackageInstaller.Stub { // We're staging to exactly one location File stageDir = null; String stageCid = null; - if (stageInternal) { - stageDir = prepareInternalStageDir(sessionId); + if ((params.installFlags & PackageManager.INSTALL_INTERNAL) != 0) { + stageDir = buildInternalStageDir(sessionId); } else { - stageCid = prepareExternalStageCid(sessionId, params.sizeBytes); + stageCid = buildExternalStageCid(sessionId); } session = new PackageInstallerSession(mInternalCallback, mContext, mPm, mInstallThread.getLooper(), sessionId, userId, installerPackageName, params, - createdMillis, stageDir, stageCid, false); + createdMillis, stageDir, stageCid, false, false); mSessions.put(sessionId, session); } @@ -615,32 +577,16 @@ public class PackageInstallerService extends IPackageInstaller.Stub { } } - private void checkInternalStorage(long sizeBytes) throws IOException { - if (sizeBytes <= 0) return; - - final File target = Environment.getDataDirectory(); - final long targetBytes = sizeBytes + mStorage.getStorageLowBytes(target); - - mPm.freeStorage(targetBytes); - if (target.getUsableSpace() < targetBytes) { - throw new IOException("Not enough internal space to write " + sizeBytes + " bytes"); - } - } - - private void checkExternalStorage(long sizeBytes) throws IOException { - if (sizeBytes <= 0) return; - - final File target = new UserEnvironment(UserHandle.USER_OWNER) - .getExternalStorageDirectory(); - final long targetBytes = sizeBytes + mStorage.getStorageLowBytes(target); - - if (target.getUsableSpace() < targetBytes) { - throw new IOException("Not enough external space to write " + sizeBytes + " bytes"); + @Override + public IPackageInstallerSession openSession(int sessionId) { + try { + return openSessionInternal(sessionId); + } catch (IOException e) { + throw ExceptionUtils.wrap(e); } } - @Override - public IPackageInstallerSession openSession(int sessionId) { + private IPackageInstallerSession openSessionInternal(int sessionId) throws IOException { synchronized (mSessions) { final PackageInstallerSession session = mSessions.get(sessionId); if (session == null || !isCallingUidOwner(session)) { @@ -665,40 +611,37 @@ public class PackageInstallerService extends IPackageInstaller.Stub { throw new IllegalStateException("Failed to allocate session ID"); } - private File prepareInternalStageDir(int sessionId) throws IOException { - final File file = new File(mStagingDir, "vmdl" + sessionId + ".tmp"); + private File buildInternalStageDir(int sessionId) { + return new File(mStagingDir, "vmdl" + sessionId + ".tmp"); + } - if (file.exists()) { - throw new IOException("Session dir already exists: " + file); + static void prepareInternalStageDir(File stageDir) throws IOException { + if (stageDir.exists()) { + throw new IOException("Session dir already exists: " + stageDir); } try { - Os.mkdir(file.getAbsolutePath(), 0755); - Os.chmod(file.getAbsolutePath(), 0755); + Os.mkdir(stageDir.getAbsolutePath(), 0755); + Os.chmod(stageDir.getAbsolutePath(), 0755); } catch (ErrnoException e) { // This purposefully throws if directory already exists - throw new IOException("Failed to prepare session dir", e); + throw new IOException("Failed to prepare session dir: " + stageDir, e); } - if (!SELinux.restorecon(file)) { - throw new IOException("Failed to restorecon session dir"); + if (!SELinux.restorecon(stageDir)) { + throw new IOException("Failed to restorecon session dir: " + stageDir); } - - return file; } - private String prepareExternalStageCid(int sessionId, long sizeBytes) throws IOException { - if (sizeBytes <= 0) { - throw new IOException("Session must provide valid size for ASEC"); - } + private String buildExternalStageCid(int sessionId) { + return "smdl" + sessionId + ".tmp"; + } - final String cid = "smdl" + sessionId + ".tmp"; - if (PackageHelper.createSdDir(sizeBytes, cid, PackageManagerService.getEncryptKey(), + static void prepareExternalStageCid(String stageCid, long sizeBytes) throws IOException { + if (PackageHelper.createSdDir(sizeBytes, stageCid, PackageManagerService.getEncryptKey(), Process.SYSTEM_UID, true) == null) { - throw new IOException("Failed to create ASEC"); + throw new IOException("Failed to create session cid: " + stageCid); } - - return cid; } @Override @@ -1031,6 +974,12 @@ public class PackageInstallerService extends IPackageInstaller.Stub { writeSessionsAsync(); } + public void onSessionPrepared(PackageInstallerSession session) { + // We prepared the destination to write into; we want to persist + // this, but it's not critical enough to block for. + writeSessionsAsync(); + } + public void onSessionSealed(PackageInstallerSession session) { // It's very important that we block until we've recorded the // session as being sealed, since we never want to allow mutation diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index f8273c039e74..adca46a6b826 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -17,15 +17,15 @@ package com.android.server.pm; import static android.content.pm.PackageManager.INSTALL_FAILED_ABORTED; -import static android.content.pm.PackageManager.INSTALL_FAILED_ALREADY_EXISTS; import static android.content.pm.PackageManager.INSTALL_FAILED_CONTAINER_ERROR; import static android.content.pm.PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; import static android.content.pm.PackageManager.INSTALL_FAILED_INTERNAL_ERROR; import static android.content.pm.PackageManager.INSTALL_FAILED_INVALID_APK; -import static android.content.pm.PackageManager.INSTALL_FAILED_PACKAGE_CHANGED; import static android.system.OsConstants.O_CREAT; import static android.system.OsConstants.O_RDONLY; import static android.system.OsConstants.O_WRONLY; +import static com.android.server.pm.PackageInstallerService.prepareExternalStageCid; +import static com.android.server.pm.PackageInstallerService.prepareInternalStageDir; import android.content.Context; import android.content.Intent; @@ -88,8 +88,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // TODO: enforce INSTALL_ALLOW_TEST // TODO: enforce INSTALL_ALLOW_DOWNGRADE - // TODO: treat INHERIT_EXISTING as installExistingPackage() - private final PackageInstallerService.InternalCallback mCallback; private final Context mContext; private final PackageManagerService mPm; @@ -108,18 +106,23 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { /** Note that UID is not persisted; it's always derived at runtime. */ final int installerUid; - private final AtomicInteger mOpenCount = new AtomicInteger(); + private final AtomicInteger mActiveCount = new AtomicInteger(); private final Object mLock = new Object(); @GuardedBy("mLock") private float mClientProgress = 0; @GuardedBy("mLock") + private float mInternalProgress = 0; + + @GuardedBy("mLock") private float mProgress = 0; @GuardedBy("mLock") private float mReportedProgress = -1; @GuardedBy("mLock") + private boolean mPrepared = false; + @GuardedBy("mLock") private boolean mSealed = false; @GuardedBy("mLock") private boolean mPermissionsAccepted = false; @@ -184,7 +187,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { public PackageInstallerSession(PackageInstallerService.InternalCallback callback, Context context, PackageManagerService pm, Looper looper, int sessionId, int userId, String installerPackageName, SessionParams params, long createdMillis, - File stageDir, String stageCid, boolean sealed) { + File stageDir, String stageCid, boolean prepared, boolean sealed) { mCallback = callback; mContext = context; mPm = pm; @@ -203,6 +206,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { "Exactly one of stageDir or stageCid stage must be set"); } + mPrepared = prepared; mSealed = sealed; // Always derived at runtime @@ -214,8 +218,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } else { mPermissionsAccepted = false; } - - computeProgressLocked(); } public SessionInfo generateInfo() { @@ -227,7 +229,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mResolvedBaseFile.getAbsolutePath() : null; info.progress = mProgress; info.sealed = mSealed; - info.active = mOpenCount.get() > 0; + info.active = mActiveCount.get() > 0; info.mode = params.mode; info.sizeBytes = params.sizeBytes; @@ -238,14 +240,23 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return info; } + public boolean isPrepared() { + synchronized (mLock) { + return mPrepared; + } + } + public boolean isSealed() { synchronized (mLock) { return mSealed; } } - private void assertNotSealed(String cookie) { + private void assertPreparedAndNotSealed(String cookie) { synchronized (mLock) { + if (!mPrepared) { + throw new IllegalStateException(cookie + " before prepared"); + } if (mSealed) { throw new SecurityException(cookie + " not allowed after commit"); } @@ -278,30 +289,26 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public void setClientProgress(float progress) { synchronized (mLock) { + // Always publish first staging movement + final boolean forcePublish = (mClientProgress == 0); mClientProgress = progress; - computeProgressLocked(); + computeProgressLocked(forcePublish); } - maybePublishProgress(); } @Override public void addClientProgress(float progress) { synchronized (mLock) { - mClientProgress += progress; - computeProgressLocked(); + setClientProgress(mClientProgress + progress); } - maybePublishProgress(); } - private void computeProgressLocked() { - if (mProgress <= 0.8f) { - mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f); - } - } + private void computeProgressLocked(boolean forcePublish) { + mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f) + + MathUtils.constrain(mInternalProgress * 0.2f, 0f, 0.2f); - private void maybePublishProgress() { // Only publish when meaningful change - if (Math.abs(mProgress - mReportedProgress) > 0.01) { + if (forcePublish || Math.abs(mProgress - mReportedProgress) >= 0.01) { mReportedProgress = mProgress; mCallback.onSessionProgressChanged(this, mProgress); } @@ -309,7 +316,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public String[] getNames() { - assertNotSealed("getNames"); + assertPreparedAndNotSealed("getNames"); try { return resolveStageDir().list(); } catch (IOException e) { @@ -333,7 +340,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // will block any attempted install transitions. final FileBridge bridge; synchronized (mLock) { - assertNotSealed("openWrite"); + assertPreparedAndNotSealed("openWrite"); bridge = new FileBridge(); mBridges.add(bridge); @@ -387,7 +394,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private ParcelFileDescriptor openReadInternal(String name) throws IOException { - assertNotSealed("openRead"); + assertPreparedAndNotSealed("openRead"); try { if (!FileUtils.isValidExtFilename(name)) { @@ -407,6 +414,30 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { public void commit(IntentSender statusReceiver) { Preconditions.checkNotNull(statusReceiver); + synchronized (mLock) { + if (!mSealed) { + // Verify that all writers are hands-off + for (FileBridge bridge : mBridges) { + if (!bridge.isClosed()) { + throw new SecurityException("Files still open"); + } + } + + // Persist the fact that we've sealed ourselves to prevent + // mutations of any hard links we create. + mSealed = true; + mCallback.onSessionSealed(this); + } + } + + // Client staging is fully done at this point + mClientProgress = 1f; + computeProgressLocked(true); + + // This ongoing commit should keep session active, even though client + // will probably close their end. + mActiveCount.incrementAndGet(); + final PackageInstallObserverAdapter adapter = new PackageInstallObserverAdapter(mContext, statusReceiver, sessionId); mHandler.obtainMessage(MSG_COMMIT, adapter.getBinder()).sendToTarget(); @@ -414,22 +445,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private void commitLocked() throws PackageManagerException { if (mDestroyed) { - throw new PackageManagerException(INSTALL_FAILED_ALREADY_EXISTS, "Invalid session"); + throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, "Session destroyed"); } - - // Verify that all writers are hands-off if (!mSealed) { - for (FileBridge bridge : mBridges) { - if (!bridge.isClosed()) { - throw new PackageManagerException(INSTALL_FAILED_PACKAGE_CHANGED, - "Files still open"); - } - } - mSealed = true; - - // Persist the fact that we've sealed ourselves to prevent mutations - // of any hard links we create below. - mCallback.onSessionSealed(this); + throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, "Session not sealed"); } try { @@ -458,6 +477,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mRemoteObserver.onUserActionRequired(intent); } catch (RemoteException ignored) { } + + // Commit was keeping session marked as active until now; release + // that extra refcount so session appears idle. + close(); return; } @@ -487,8 +510,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } // TODO: surface more granular state from dexopt - mProgress = 0.9f; - maybePublishProgress(); + mInternalProgress = 0.5f; + computeProgressLocked(true); // Unpack native libraries extractNativeLibraries(mResolvedStageDir, params.abiOverride); @@ -831,15 +854,35 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - public void open() { - if (mOpenCount.getAndIncrement() == 0) { + public void open() throws IOException { + if (mActiveCount.getAndIncrement() == 0) { mCallback.onSessionActiveChanged(this, true); } + + synchronized (mLock) { + if (!mPrepared) { + if (stageDir != null) { + prepareInternalStageDir(stageDir); + } else if (stageCid != null) { + prepareExternalStageCid(stageCid, params.sizeBytes); + + // TODO: deliver more granular progress for ASEC allocation + mInternalProgress = 0.25f; + computeProgressLocked(true); + } else { + throw new IllegalArgumentException( + "Exactly one of stageDir or stageCid stage must be set"); + } + + mPrepared = true; + mCallback.onSessionPrepared(this); + } + } } @Override public void close() { - if (mOpenCount.decrementAndGet() == 0) { + if (mActiveCount.decrementAndGet() == 0) { mCallback.onSessionActiveChanged(this, false); } } @@ -869,6 +912,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { synchronized (mLock) { mSealed = true; mDestroyed = true; + + // Force shut down all bridges + for (FileBridge bridge : mBridges) { + bridge.forceClose(); + } } if (stageDir != null) { FileUtils.deleteContents(stageDir); |