From f739ea83bdac2f4c4d000e60eedccfc85382a94c Mon Sep 17 00:00:00 2001 From: Stanislav Baranov Date: Wed, 9 Jan 2019 14:24:46 -0800 Subject: [PATCH] Download dynamic patch to separate file, then rename it to install. This fixes potential race condition when patch gets downloaded on top of zip file that's currently in active use by resource extractor and/or asset manager. This change is necessary since download can happen in the background while normal application operations are in progress. --- .../flutter/app/FlutterActivityDelegate.java | 2 +- .../io/flutter/view/ResourceExtractor.java | 79 +++++++++++++------ .../io/flutter/view/ResourceUpdater.java | 56 +++++++++++-- 3 files changed, 106 insertions(+), 31 deletions(-) diff --git a/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java b/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java index 429f43ad8ae77..a2318caceab1d 100644 --- a/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java +++ b/shell/platform/android/io/flutter/app/FlutterActivityDelegate.java @@ -345,7 +345,7 @@ private void runBundle(String appBundlePath) { FlutterRunArguments args = new FlutterRunArguments(); ArrayList bundlePaths = new ArrayList<>(); if (FlutterMain.getResourceUpdater() != null) { - File patchFile = FlutterMain.getResourceUpdater().getPatch(); + File patchFile = FlutterMain.getResourceUpdater().getInstalledPatch(); bundlePaths.add(patchFile.getPath()); } bundlePaths.add(appBundlePath); diff --git a/shell/platform/android/io/flutter/view/ResourceExtractor.java b/shell/platform/android/io/flutter/view/ResourceExtractor.java index cfcae096678ba..a2a0ef0b9af40 100644 --- a/shell/platform/android/io/flutter/view/ResourceExtractor.java +++ b/shell/platform/android/io/flutter/view/ResourceExtractor.java @@ -51,37 +51,68 @@ private class ExtractTask extends AsyncTask { protected Void doInBackground(Void... unused) { final File dataDir = new File(PathUtils.getDataDirectory(mContext)); - JSONObject updateManifest = readUpdateManifest(); - if (!validateUpdateManifest(updateManifest)) { - updateManifest = null; + ResourceUpdater resourceUpdater = FlutterMain.getResourceUpdater(); + if (resourceUpdater != null) { + // Protect patch file from being overwritten by downloader while + // it's being extracted since downloading happens asynchronously. + resourceUpdater.getInstallationLock().lock(); } - final String timestamp = checkTimestamp(dataDir, updateManifest); - if (timestamp == null) { - return null; - } + try { + if (resourceUpdater != null) { + File updateFile = resourceUpdater.getDownloadedPatch(); + File activeFile = resourceUpdater.getInstalledPatch(); + + if (updateFile.exists()) { + // Graduate patch file as active for asset manager. + if (activeFile.exists() && !activeFile.delete()) { + Log.w(TAG, "Could not delete file " + activeFile); + return null; + } + if (!updateFile.renameTo(activeFile)) { + Log.w(TAG, "Could not create file " + activeFile); + return null; + } + } + } - deleteFiles(); + JSONObject updateManifest = readUpdateManifest(); + if (!validateUpdateManifest(updateManifest)) { + updateManifest = null; + } - if (updateManifest != null) { - if (!extractUpdate(dataDir)) { + final String timestamp = checkTimestamp(dataDir, updateManifest); + if (timestamp == null) { return null; } - } - if (!extractAPK(dataDir)) { - return null; - } + deleteFiles(); - if (timestamp != null) { - try { - new File(dataDir, timestamp).createNewFile(); - } catch (IOException e) { - Log.w(TAG, "Failed to write resource timestamp"); + if (updateManifest != null) { + if (!extractUpdate(dataDir)) { + return null; + } } - } - return null; + if (!extractAPK(dataDir)) { + return null; + } + + if (timestamp != null) { + try { + new File(dataDir, timestamp).createNewFile(); + } catch (IOException e) { + Log.w(TAG, "Failed to write resource timestamp"); + } + } + + return null; + + } finally { + if (resourceUpdater != null) { + resourceUpdater.getInstallationLock().unlock(); + } + } } } @@ -201,7 +232,7 @@ private boolean extractUpdate(File dataDir) { return true; } - File updateFile = resourceUpdater.getPatch(); + File updateFile = resourceUpdater.getInstalledPatch(); if (!updateFile.exists()) { return true; } @@ -289,7 +320,7 @@ private String checkTimestamp(File dataDir, JSONObject updateManifest) { } else { ResourceUpdater resourceUpdater = FlutterMain.getResourceUpdater(); assert resourceUpdater != null; - File patchFile = resourceUpdater.getPatch(); + File patchFile = resourceUpdater.getInstalledPatch(); assert patchFile.exists(); if (patchNumber != null) { expectedTimestamp += "-" + patchNumber + "-" + patchFile.lastModified(); @@ -362,7 +393,7 @@ private JSONObject readUpdateManifest() { return null; } - File updateFile = resourceUpdater.getPatch(); + File updateFile = resourceUpdater.getInstalledPatch(); if (!updateFile.exists()) { return null; } diff --git a/shell/platform/android/io/flutter/view/ResourceUpdater.java b/shell/platform/android/io/flutter/view/ResourceUpdater.java index 5fef38f66f111..b6a0c39025451 100644 --- a/shell/platform/android/io/flutter/view/ResourceUpdater.java +++ b/shell/platform/android/io/flutter/view/ResourceUpdater.java @@ -15,6 +15,7 @@ import java.io.InputStream; import java.io.IOException; import java.io.OutputStream; +import java.lang.Math; import java.net.HttpURLConnection; import java.net.URI; import java.net.URISyntaxException; @@ -22,6 +23,8 @@ import java.util.Date; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; public final class ResourceUpdater { private static final String TAG = "ResourceUpdater"; @@ -58,12 +61,33 @@ enum InstallMode { IMMEDIATE } + /// Lock that prevents replacement of the install file by the downloader + /// while this file is being extracted, since these can happen in parallel. + Lock getInstallationLock() { + return installationLock; + } + + // Patch file that's fully installed and is ready to serve assets. + // This file represents the final stage in the installation process. + public File getInstalledPatch() { + return new File(context.getFilesDir().toString() + "/patch.zip"); + } + + // Patch file that's finished downloading and is ready to be installed. + // This is a separate file in order to prevent serving assets from patch + // that failed installing for any reason, such as mismatched APK version. + File getDownloadedPatch() { + return new File(getInstalledPatch().getPath() + ".install"); + } + private class DownloadTask extends AsyncTask { @Override protected Void doInBackground(String... unused) { try { URL unresolvedURL = new URL(buildUpdateDownloadURL()); - File localFile = getPatch(); + + // Download to transient file to avoid extracting incomplete download. + File localFile = new File(getInstalledPatch().getPath() + ".download"); long startMillis = new Date().getTime(); Log.i(TAG, "Checking for updates at " + unresolvedURL); @@ -71,7 +95,10 @@ protected Void doInBackground(String... unused) { HttpURLConnection connection = (HttpURLConnection)unresolvedURL.openConnection(); - long lastDownloadTime = localFile.lastModified(); + long lastDownloadTime = Math.max( + getDownloadedPatch().lastModified(), + getInstalledPatch().lastModified()); + if (lastDownloadTime != 0) { Log.i(TAG, "Active update timestamp " + lastDownloadTime); connection.setIfModifiedSince(lastDownloadTime); @@ -107,9 +134,29 @@ protected Void doInBackground(String... unused) { long totalMillis = new Date().getTime() - startMillis; Log.i(TAG, "Update downloaded in " + totalMillis / 100 / 10. + "s"); + } + } + + // Wait renaming the file if extraction is in progress. + installationLock.lock(); + + try { + File updateFile = getDownloadedPatch(); + // Graduate downloaded file as ready for installation. + if (updateFile.exists() && !updateFile.delete()) { + Log.w(TAG, "Could not delete file " + updateFile); + return null; + } + if (!localFile.renameTo(updateFile)) { + Log.w(TAG, "Could not create file " + updateFile); return null; } + + return null; + + } finally { + installationLock.unlock(); } } catch (IOException e) { @@ -121,6 +168,7 @@ protected Void doInBackground(String... unused) { private final Context context; private DownloadTask downloadTask; + private final Lock installationLock = new ReentrantLock(); public ResourceUpdater(Context context) { this.context = context; @@ -137,10 +185,6 @@ private String getAPKVersion() { } } - public File getPatch() { - return new File(context.getFilesDir().toString() + "/patch.zip"); - } - private String buildUpdateDownloadURL() { Bundle metaData; try {