diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 608580cd97476..b65fc73584713 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -331,8 +331,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private boolean mCommitted = false; @GuardedBy("mLock") private boolean mRelinquished = false; - @GuardedBy("mLock") - private boolean mDestroyed = false; /** Permissions have been accepted by the user (see {@link #setPermissionsResult}) */ @GuardedBy("mLock") @@ -762,8 +760,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private boolean mVerityFound; - @GuardedBy("mLock") - private boolean mDataLoaderFinished = false; + /** + * Both flags should be guarded with mLock whenever changes need to be in lockstep. + * Ok to check without mLock in case the proper check is done later, e.g. status callbacks + * for DataLoaders with deferred processing. + */ + private volatile boolean mDestroyed = false; + private volatile boolean mDataLoaderFinished = false; @GuardedBy("mLock") private IncrementalFileStorages mIncrementalFileStorages; @@ -3562,19 +3565,15 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return; } - synchronized (mLock) { - if (mDestroyed || mDataLoaderFinished) { - // No need to worry about post installation - return; - } + if (mDestroyed || mDataLoaderFinished) { + // No need to worry about post installation + return; } try { IDataLoader dataLoader = dataLoaderManager.getDataLoader(dataLoaderId); if (dataLoader == null) { - synchronized (mLock) { - mDataLoaderFinished = true; - } + mDataLoaderFinished = true; dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, "Failure to obtain data loader"); return; @@ -3607,9 +3606,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { break; } case IDataLoaderStatusListener.DATA_LOADER_IMAGE_READY: { - synchronized (mLock) { - mDataLoaderFinished = true; - } + mDataLoaderFinished = true; if (hasParentSessionId()) { mSessionProvider.getSession( getParentSessionId()).dispatchSessionSealed(); @@ -3622,9 +3619,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { break; } case IDataLoaderStatusListener.DATA_LOADER_IMAGE_NOT_READY: { - synchronized (mLock) { - mDataLoaderFinished = true; - } + mDataLoaderFinished = true; dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, "Failed to prepare image."); if (manualStartAndDestroy) { @@ -3643,9 +3638,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { break; } case IDataLoaderStatusListener.DATA_LOADER_UNRECOVERABLE: - synchronized (mLock) { - mDataLoaderFinished = true; - } + mDataLoaderFinished = true; dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, "DataLoader reported unrecoverable failure."); break; @@ -3683,11 +3676,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { final IStorageHealthListener healthListener = new IStorageHealthListener.Stub() { @Override public void onHealthStatus(int storageId, int status) { - synchronized (mLock) { - if (mDestroyed || mDataLoaderFinished) { - // No need to worry about post installation - return; - } + if (mDestroyed || mDataLoaderFinished) { + // No need to worry about post installation + return; } switch (status) { @@ -3702,9 +3693,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // fallthrough case IStorageHealthListener.HEALTH_STATUS_UNHEALTHY: // Even ADB installation can't wait for missing pages for too long. - synchronized (mLock) { - mDataLoaderFinished = true; - } + mDataLoaderFinished = true; dispatchSessionValidationFailure(INSTALL_FAILED_MEDIA_UNAVAILABLE, "Image is missing pages required for installation."); break; diff --git a/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp b/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp index 631e185fffce5..7b379e54a40ef 100644 --- a/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp +++ b/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp @@ -68,6 +68,9 @@ static constexpr MagicType INCR = 0x52434e49; // BE INCR static constexpr auto PollTimeoutMs = 5000; static constexpr auto TraceTagCheckInterval = 1s; +static constexpr auto WaitOnEofMinInterval = 10ms; +static constexpr auto WaitOnEofMaxInterval = 1s; + struct JniIds { jclass packageManagerShellCommandDataLoader; jmethodID pmscdLookupShellCommand; @@ -485,14 +488,16 @@ private: if (read == 0) { if (waitOnEof) { // eof of stdin, waiting... - ALOGE("eof of stdin, waiting...: %d, remaining: %d, block: %d, read: %d", - int(totalSize), int(remaining), int(blockIdx), int(read)); - using namespace std::chrono_literals; - std::this_thread::sleep_for(10ms); - continue; + if (doWaitOnEof()) { + continue; + } else { + return false; + } } break; } + resetWaitOnEof(); + if (read < 0) { return false; } @@ -776,6 +781,21 @@ private: return fileId; } + // Waiting with exponential backoff, maximum total time ~1.2sec. + bool doWaitOnEof() { + if (mWaitOnEofInterval >= WaitOnEofMaxInterval) { + resetWaitOnEof(); + return false; + } + auto result = mWaitOnEofInterval; + mWaitOnEofInterval = + std::min(mWaitOnEofInterval * 2, WaitOnEofMaxInterval); + std::this_thread::sleep_for(result); + return true; + } + + void resetWaitOnEof() { mWaitOnEofInterval = WaitOnEofMinInterval; } + JavaVM* const mJvm; std::string mArgs; android::dataloader::FilesystemConnectorPtr mIfs = nullptr; @@ -786,6 +806,7 @@ private: std::thread mReceiverThread; std::atomic mStopReceiving = false; std::atomic mReadLogsEnabled = false; + std::chrono::milliseconds mWaitOnEofInterval{WaitOnEofMinInterval}; /** Tracks which files have been requested */ std::unordered_set mRequestedFiles; };