Reinstall on IOException when patching the CL

When an IOException occurs during installSecondaryDexes of
MultiDex.install, clear the extraction dir and make one supplementary
attempt to extract and install.
The obective is to recover from some cases of corrupted extractions or
corrupted odex files whitout requiring manual clearing of application
data.

The extraction, patching of the classloader, and recover is now done under
file lock protection to avoid clearing the cache directory while another
process would be using it. This should not cause more ANRs because
extraction was already done under file lock and dexopt which is the main
part of classloader patching is running under its own lock protection.

MultiDex.installInstrumentation isn't attempting this recover to keep
test failing in case of corruption and keep corrupted files and
hopefully allow more precise investigations. Note that adding recovering
capability to MultiDex.installInstrumentation would require changing
locking strategy.

Bug: 28832787
Test: MultiDexLegacyTestServicesTests2
Change-Id: I247918c1fbec8686ade12b37b8680539688a61a9

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;