From 2fa007ef678b2283d47d007aa3dc91af683cc52c Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Wed, 24 Oct 2012 17:18:32 -0700
Subject: 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
---
 .../android/providers/downloads/DownloadInfo.java   | 21 ++++++++++++++++++++-
 .../android/providers/downloads/DownloadThread.java | 16 +++++++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index b6d57a05..5172b696 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -36,7 +36,6 @@ import android.util.Pair;
 
 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 @@ public class DownloadInfo {
                 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();
+        }
+    }
 }
diff --git a/src/com/android/providers/downloads/DownloadThread.java b/src/com/android/providers/downloads/DownloadThread.java
index 37db32b1..e74d5c72 100644
--- a/src/com/android/providers/downloads/DownloadThread.java
+++ b/src/com/android/providers/downloads/DownloadThread.java
@@ -130,6 +130,21 @@ public class DownloadThread extends Thread {
     @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 @@ public class DownloadThread extends Thread {
             notifyDownloadCompleted(finalStatus, state.mCountRetry, state.mRetryAfter,
                                     state.mGotData, state.mFilename,
                                     state.mNewUri, state.mMimeType, errorMsg);
-            DownloadHandler.getInstance().dequeueDownload(mInfo.mId);
 
             netPolicy.unregisterListener(mPolicyListener);
 
-- 
cgit v1.2.3