Merge "Hardening: removing deadlocks and an infinite loop."
This commit is contained in:
committed by
Android (Google) Code Review
commit
7d6b3cc8fc
@@ -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;
|
||||
|
||||
@@ -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<std::chrono::milliseconds>(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<bool> mStopReceiving = false;
|
||||
std::atomic<bool> mReadLogsEnabled = false;
|
||||
std::chrono::milliseconds mWaitOnEofInterval{WaitOnEofMinInterval};
|
||||
/** Tracks which files have been requested */
|
||||
std::unordered_set<FileIdx> mRequestedFiles;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user