Reinstall on IOException when patching the CL am: 0813385e7b

am: 830d092ae0

Change-Id: I16307ce84b1156a150514a25f7581c39e6f42fb1

2 files changed, 123 insertions, 78 deletions

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 @@ public final class MultiDex {
                     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 @@ public final class MultiDex {
                     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 @@ public final class MultiDex {
      * @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 @@ public final class MultiDex {
             }
 
             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 @@ public final class MultiDex {
                 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 @@ public final class MultiDex {
                 }
 
                 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 @@ import java.util.zip.ZipOutputStream;
 /**
  * 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 @@ final class MultiDexExtractor {
     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 @@ final class MultiDexExtractor {
      * @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);
+        if (!cacheLock.isValid()) {
+            throw new IllegalStateException("MultiDexExtractor was closed");
+        }
 
-        // 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,
+        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);
             }
-        } 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);
+        } else {
+            if (forceReload) {
+                Log.i(TAG, "Forced extraction must be performed.");
+            } else {
+                Log.i(TAG, "Detected that extraction must be performed.");
             }
-            closeQuietly(lockRaf);
-        }
-
-        if (releaseLockException != null) {
-            throw releaseLockException;
+            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 @@ final class MultiDexExtractor {
         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 @@ final class MultiDexExtractor {
                     }
 
                     // 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 @@ final class MultiDexExtractor {
     }
 
     /**
-     * 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;