Reinstall on IOException when patching the CL
am: 0813385e7b
Change-Id: If5635db156bdb02bf45912a6a597f22b169e5fd1
diff --git a/library/src/android/support/multidex/MultiDex.java b/library/src/android/support/multidex/MultiDex.java
index ab7f668..646e023 100644
--- a/library/src/android/support/multidex/MultiDex.java
+++ b/library/src/android/support/multidex/MultiDex.java
@@ -114,7 +114,8 @@
new File(applicationInfo.sourceDir),
new File(applicationInfo.dataDir),
CODE_CACHE_SECONDARY_FOLDER_NAME,
- NO_KEY_PREFIX);
+ NO_KEY_PREFIX,
+ true);
} catch (Exception e) {
Log.e(TAG, "MultiDex installation failure", e);
@@ -171,13 +172,15 @@
new File(instrumentationInfo.sourceDir),
dataDir,
instrumentationPrefix + CODE_CACHE_SECONDARY_FOLDER_NAME,
- instrumentationPrefix);
+ instrumentationPrefix,
+ false);
doInstallation(targetContext,
new File(applicationInfo.sourceDir),
dataDir,
CODE_CACHE_SECONDARY_FOLDER_NAME,
- NO_KEY_PREFIX);
+ NO_KEY_PREFIX,
+ false);
} catch (Exception e) {
Log.e(TAG, "MultiDex installation failure", e);
throw new RuntimeException("MultiDex installation failed (" + e.getMessage() + ").");
@@ -192,9 +195,12 @@
* @param dataDir data directory to use for code cache simulation.
* @param secondaryFolderName name of the folder for storing extractions.
* @param prefsKeyPrefix prefix of all stored preference keys.
+ * @param reinstallOnPatchRecoverableException if set to true, will attempt a clean extraction
+ * if a possibly recoverable exception occurs during classloader patching.
*/
private static void doInstallation(Context mainContext, File sourceApk, File dataDir,
- String secondaryFolderName, String prefsKeyPrefix) throws IOException,
+ String secondaryFolderName, String prefsKeyPrefix,
+ boolean reinstallOnPatchRecoverableException) throws IOException,
IllegalArgumentException, IllegalAccessException, NoSuchFieldException,
InvocationTargetException, NoSuchMethodException {
synchronized (installedApk) {
@@ -245,9 +251,38 @@
}
File dexDir = getDexDir(mainContext, dataDir, secondaryFolderName);
- List<? extends File> files =
- MultiDexExtractor.load(mainContext, sourceApk, dexDir, prefsKeyPrefix, false);
- installSecondaryDexes(loader, dexDir, files);
+ // MultiDexExtractor is taking the file lock and keeping it until it is closed.
+ // Keep it open during installSecondaryDexes and through forced extraction to ensure no
+ // extraction or optimizing dexopt is running in parallel.
+ MultiDexExtractor extractor = new MultiDexExtractor(sourceApk, dexDir);
+ IOException closeException = null;
+ try {
+ List<? extends File> files =
+ extractor.load(mainContext, prefsKeyPrefix, false);
+ try {
+ installSecondaryDexes(loader, dexDir, files);
+ // Some IOException causes may be fixed by a clean extraction.
+ } catch (IOException e) {
+ if (!reinstallOnPatchRecoverableException) {
+ throw e;
+ }
+ Log.w(TAG, "Failed to install extracted secondary dex files, retrying with "
+ + "forced extraction", e);
+ files = extractor.load(mainContext, prefsKeyPrefix, true);
+ installSecondaryDexes(loader, dexDir, files);
+ }
+ } finally {
+ try {
+ extractor.close();
+ } catch (IOException e) {
+ // Delay throw of close exception to ensure we don't override some exception
+ // thrown during the try block.
+ closeException = e;
+ }
+ }
+ if (closeException != null) {
+ throw closeException;
+ }
}
}
@@ -464,7 +499,8 @@
List<? extends File> additionalClassPathEntries,
File optimizedDirectory)
throws IllegalArgumentException, IllegalAccessException,
- NoSuchFieldException, InvocationTargetException, NoSuchMethodException {
+ NoSuchFieldException, InvocationTargetException, NoSuchMethodException,
+ IOException {
/* The patched class loader is expected to be a descendant of
* dalvik.system.BaseDexClassLoader. We modify its
* dalvik.system.DexPathList pathList field to append additional DEX
@@ -500,6 +536,10 @@
}
suppressedExceptionsField.set(dexPathList, dexElementsSuppressedExceptions);
+
+ IOException exception = new IOException("I/O exception during makeDexElement");
+ exception.initCause(suppressedExceptions.get(0));
+ throw exception;
}
}
diff --git a/library/src/android/support/multidex/MultiDexExtractor.java b/library/src/android/support/multidex/MultiDexExtractor.java
index 39b6bf7..ed3c125 100644
--- a/library/src/android/support/multidex/MultiDexExtractor.java
+++ b/library/src/android/support/multidex/MultiDexExtractor.java
@@ -40,8 +40,10 @@
/**
* Exposes application secondary dex files as files in the application data
* directory.
+ * {@link MultiDexExtractor} is taking the file lock in the dex dir on creation and release it
+ * during close.
*/
-final class MultiDexExtractor {
+final class MultiDexExtractor implements Closeable {
/**
* Zip file containing one secondary dex file.
@@ -82,6 +84,35 @@
private static final long NO_VALUE = -1L;
private static final String LOCK_FILENAME = "MultiDex.lock";
+ private final File sourceApk;
+ private final long sourceCrc;
+ private final File dexDir;
+ private final RandomAccessFile lockRaf;
+ private final FileChannel lockChannel;
+ private final FileLock cacheLock;
+
+ MultiDexExtractor(File sourceApk, File dexDir) throws IOException {
+ Log.i(TAG, "MultiDexExtractor(" + sourceApk.getPath() + ", " + dexDir.getPath() + ")");
+ this.sourceApk = sourceApk;
+ this.dexDir = dexDir;
+ sourceCrc = getZipCrc(sourceApk);
+ File lockFile = new File(dexDir, LOCK_FILENAME);
+ lockRaf = new RandomAccessFile(lockFile, "rw");
+ try {
+ lockChannel = lockRaf.getChannel();
+ try {
+ Log.i(TAG, "Blocking on lock " + lockFile.getPath());
+ cacheLock = lockChannel.lock();
+ } catch (IOException | RuntimeException | Error e) {
+ closeQuietly(lockChannel);
+ throw e;
+ }
+ Log.i(TAG, lockFile.getPath() + " locked");
+ } catch (IOException | RuntimeException | Error e) {
+ closeQuietly(lockRaf);
+ throw e;
+ }
+ }
/**
* Extracts application secondary dexes into files in the application data
@@ -92,74 +123,54 @@
* @throws IOException if encounters a problem while reading or writing
* secondary dex files
*/
- static List<? extends File> load(Context context, File sourceApk, File dexDir,
- String prefsKeyPrefix,
- boolean forceReload) throws IOException {
+ List<? extends File> load(Context context, String prefsKeyPrefix, boolean forceReload)
+ throws IOException {
Log.i(TAG, "MultiDexExtractor.load(" + sourceApk.getPath() + ", " + forceReload + ", " +
prefsKeyPrefix + ")");
- long currentCrc = getZipCrc(sourceApk);
-
- // Validity check and extraction must be done only while the lock file has been taken.
- File lockFile = new File(dexDir, LOCK_FILENAME);
- RandomAccessFile lockRaf = new RandomAccessFile(lockFile, "rw");
- FileChannel lockChannel = null;
- FileLock cacheLock = null;
- List<ExtractedDex> files;
- IOException releaseLockException = null;
- try {
- lockChannel = lockRaf.getChannel();
- Log.i(TAG, "Blocking on lock " + lockFile.getPath());
- cacheLock = lockChannel.lock();
- Log.i(TAG, lockFile.getPath() + " locked");
-
- if (!forceReload && !isModified(context, sourceApk, currentCrc, prefsKeyPrefix)) {
- try {
- files = loadExistingExtractions(context, sourceApk, dexDir, prefsKeyPrefix);
- } catch (IOException ioe) {
- Log.w(TAG, "Failed to reload existing extracted secondary dex files,"
- + " falling back to fresh extraction", ioe);
- files = performExtractions(sourceApk, dexDir);
- putStoredApkInfo(context, prefsKeyPrefix, getTimeStamp(sourceApk), currentCrc,
- files);
- }
- } else {
- Log.i(TAG, "Detected that extraction must be performed.");
- files = performExtractions(sourceApk, dexDir);
- putStoredApkInfo(context, prefsKeyPrefix, getTimeStamp(sourceApk), currentCrc,
- files);
- }
- } finally {
- if (cacheLock != null) {
- try {
- cacheLock.release();
- } catch (IOException e) {
- Log.e(TAG, "Failed to release lock on " + lockFile.getPath());
- // Exception while releasing the lock is bad, we want to report it, but not at
- // the price of overriding any already pending exception.
- releaseLockException = e;
- }
- }
- if (lockChannel != null) {
- closeQuietly(lockChannel);
- }
- closeQuietly(lockRaf);
+ if (!cacheLock.isValid()) {
+ throw new IllegalStateException("MultiDexExtractor was closed");
}
- if (releaseLockException != null) {
- throw releaseLockException;
+ List<ExtractedDex> files;
+ if (!forceReload && !isModified(context, sourceApk, sourceCrc, prefsKeyPrefix)) {
+ try {
+ files = loadExistingExtractions(context, prefsKeyPrefix);
+ } catch (IOException ioe) {
+ Log.w(TAG, "Failed to reload existing extracted secondary dex files,"
+ + " falling back to fresh extraction", ioe);
+ files = performExtractions();
+ putStoredApkInfo(context, prefsKeyPrefix, getTimeStamp(sourceApk), sourceCrc,
+ files);
+ }
+ } else {
+ if (forceReload) {
+ Log.i(TAG, "Forced extraction must be performed.");
+ } else {
+ Log.i(TAG, "Detected that extraction must be performed.");
+ }
+ files = performExtractions();
+ putStoredApkInfo(context, prefsKeyPrefix, getTimeStamp(sourceApk), sourceCrc,
+ files);
}
Log.i(TAG, "load found " + files.size() + " secondary dex files");
return files;
}
+ @Override
+ public void close() throws IOException {
+ cacheLock.release();
+ lockChannel.close();
+ lockRaf.close();
+ }
+
/**
* Load previously extracted secondary dex files. Should be called only while owning the lock on
* {@link #LOCK_FILENAME}.
*/
- private static List<ExtractedDex> loadExistingExtractions(
- Context context, File sourceApk, File dexDir,
+ private List<ExtractedDex> loadExistingExtractions(
+ Context context,
String prefsKeyPrefix)
throws IOException {
Log.i(TAG, "loading existing secondary dex files");
@@ -228,16 +239,14 @@
return computedValue;
}
- private static List<ExtractedDex> performExtractions(File sourceApk, File dexDir)
- throws IOException {
+ private List<ExtractedDex> performExtractions() throws IOException {
final String extractedFilePrefix = sourceApk.getName() + EXTRACTED_NAME_EXT;
- // Ensure that whatever deletions happen in prepareDexDir only happen if the zip that
- // contains a secondary dex file in there is not consistent with the latest apk. Otherwise,
- // multi-process race conditions can cause a crash loop where one process deletes the zip
- // while another had created it.
- prepareDexDir(dexDir, extractedFilePrefix);
+ // It is safe to fully clear the dex dir because we own the file lock so no other process is
+ // extracting or running optimizing dexopt. It may cause crash of already running
+ // applications if for whatever reason we end up extracting again over a valid extraction.
+ clearDexDir();
List<ExtractedDex> files = new ArrayList<ExtractedDex>();
@@ -272,9 +281,9 @@
}
// Log size and crc of the extracted zip file
- Log.i(TAG, "Extraction " + (isExtractionSuccessful ? "succeeded" : "failed") +
- " - length " + extractedFile.getAbsolutePath() + ": " +
- extractedFile.length() + " - crc: " + extractedFile.crc);
+ Log.i(TAG, "Extraction " + (isExtractionSuccessful ? "succeeded" : "failed")
+ + " '" + extractedFile.getAbsolutePath() + "': length "
+ + extractedFile.length() + " - crc: " + extractedFile.crc);
if (!isExtractionSuccessful) {
// Delete the extracted file
extractedFile.delete();
@@ -339,19 +348,15 @@
}
/**
- * This removes old files.
+ * Clear the dex dir from all files but the lock.
*/
- private static void prepareDexDir(File dexDir, final String extractedFilePrefix) {
- FileFilter filter = new FileFilter() {
-
+ private void clearDexDir() {
+ File[] files = dexDir.listFiles(new FileFilter() {
@Override
public boolean accept(File pathname) {
- String name = pathname.getName();
- return !(name.startsWith(extractedFilePrefix)
- || name.equals(LOCK_FILENAME));
+ return !pathname.getName().equals(LOCK_FILENAME);
}
- };
- File[] files = dexDir.listFiles(filter);
+ });
if (files == null) {
Log.w(TAG, "Failed to list secondary dex dir content (" + dexDir.getPath() + ").");
return;