commit 2fa007ef678b2283d47d007aa3dc91af683cc52c
author Jeff Sharkey <jsharkey@android.com> Wed Oct 24 17:18:32 2012 -0700
committer Jeff Sharkey <jsharkey@android.com> Wed Oct 24 17:33:06 2012 -0700
tree 90b289d03a4aded4f7da35dfaee40c09a1dc27f8
parent 169ab531ca6bb2f43ed18fe2b03f66c808f12b81

Better sanity checking for finished downloads.

Downloads in the RUNNING state are considered ready to start so that
downloads are correctly resumed when the process crashes. However,
this causes a race condition while UpdateThread is processing a
Cursor when a DownloadThread finishes.

With this change, DownloadThread now skips requests for downloads
already marked as finished. Apps listening for the DOWNLOAD_COMPLETE
broadcast will no longer see data mutated by the second thread, and
will not see the broadcast duplicated.

Bug: 6948938, 6970458, 6818900
Change-Id: I35deac3cedbfe7f50091fab5818d85594dba558c

src/com/android/providers/downloads/DownloadInfo.java

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index b6d57a0..5172b69 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -36,7 +36,6 @@
 
 import com.android.internal.util.IndentingPrintWriter;
 
-import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -576,4 +575,24 @@
                 StorageManager.getInstance(mContext));
         mSystemFacade.startThread(downloader);
     }
+
+    /**
+     * Query and return status of requested download.
+     */
+    public static int queryDownloadStatus(ContentResolver resolver, long id) {
+        final Cursor cursor = resolver.query(
+                ContentUris.withAppendedId(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, id),
+                new String[] { Downloads.Impl.COLUMN_STATUS }, null, null, null);
+        try {
+            if (cursor.moveToFirst()) {
+                return cursor.getInt(0);
+            } else {
+                // TODO: increase strictness of value returned for unknown
+                // downloads; this is safe default for now.
+                return Downloads.Impl.STATUS_PENDING;
+            }
+        } finally {
+            cursor.close();
+        }
+    }
 }

src/com/android/providers/downloads/DownloadThread.java

diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 37db32b..e74d5c7 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -130,6 +130,21 @@
     @Override
     public void run() {
         Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
+        try {
+            runInternal();
+        } finally {
+            DownloadHandler.getInstance().dequeueDownload(mInfo.mId);
+        }
+    }
+
+    private void runInternal() {
+        // Skip when download already marked as finished; this download was
+        // probably started again while racing with UpdateThread.
+        if (DownloadInfo.queryDownloadStatus(mContext.getContentResolver(), mInfo.mId)
+                == Downloads.Impl.STATUS_SUCCESS) {
+            Log.d(TAG, "Download " + mInfo.mId + " already finished; skipping");
+            return;
+        }
 
         State state = new State(mInfo);
         AndroidHttpClient client = null;
@@ -210,7 +225,6 @@
             notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter,
                                     state.mGotData, state.mFilename,
                                     state.mNewUri, state.mMimeType, errorMsg);
-            DownloadHandler.getInstance().dequeueDownload(mInfo.mId);
 
             netPolicy.unregisterListener(mPolicyListener);