summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSteve Howard <showard@google.com>2010-07-20 12:04:02 -0700
committerSteve Howard <showard@google.com>2010-07-20 12:04:02 -0700
commit5180de23e77139dd6971dfd48269242e3e3562d9 (patch)
treeebcf27563c2eb44e821437b459f99f3b150e1454
parent071bd7acb3185f4f1e807855605c5e6018e9742f (diff)
downloadandroid_packages_providers_DownloadProvider-5180de23e77139dd6971dfd48269242e3e3562d9.tar.gz
android_packages_providers_DownloadProvider-5180de23e77139dd6971dfd48269242e3e3562d9.tar.bz2
android_packages_providers_DownloadProvider-5180de23e77139dd6971dfd48269242e3e3562d9.zip
Fix bug with deciding when to restart a download.
In previous refactoring I had combined the code for starting a download (from DownloadService.insertDownload()) and restarting a download (from DownloadService.updateDownload()). I'd missed the fact that the former checks DownloadInfo.isReadyToStart() while the latter checks DownloadInfo.isReadyToRestart(). This cause a race condition where multiple startService() calls during a download would cause the service to try to spawn a second thread for the same running download. I've fixed the bug and added a test case to exercise this. Change-Id: Ia968331a030137daac7c8b5792a01e3e19065b38
-rw-r--r--src/com/android/providers/downloads/DownloadInfo.java36
-rw-r--r--src/com/android/providers/downloads/DownloadService.java8
-rw-r--r--tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java13
3 files changed, 36 insertions, 21 deletions
diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java
index 5cd50c92..153e0454 100644
--- a/src/com/android/providers/downloads/DownloadInfo.java
+++ b/src/com/android/providers/downloads/DownloadInfo.java
@@ -289,25 +289,23 @@ public class DownloadInfo {
return mTotalBytes <= maxBytesOverMobile;
}
- void startIfReady(long now) {
- if (isReadyToStart(now)) {
- if (Constants.LOGV) {
- Log.v(Constants.TAG, "Service spawning thread to handle download " + mId);
- }
- if (mHasActiveThread) {
- throw new IllegalStateException("Multiple threads on same download");
- }
- if (mStatus != Impl.STATUS_RUNNING) {
- mStatus = Impl.STATUS_RUNNING;
- ContentValues values = new ContentValues();
- values.put(Impl.COLUMN_STATUS, mStatus);
- mContext.getContentResolver().update(
- ContentUris.withAppendedId(Impl.CONTENT_URI, mId),
- values, null, null);
- }
- DownloadThread downloader = new DownloadThread(mContext, mSystemFacade, this);
- mHasActiveThread = true;
- downloader.start();
+ void start(long now) {
+ if (Constants.LOGV) {
+ Log.v(Constants.TAG, "Service spawning thread to handle download " + mId);
+ }
+ if (mHasActiveThread) {
+ throw new IllegalStateException("Multiple threads on same download");
+ }
+ if (mStatus != Impl.STATUS_RUNNING) {
+ mStatus = Impl.STATUS_RUNNING;
+ ContentValues values = new ContentValues();
+ values.put(Impl.COLUMN_STATUS, mStatus);
+ mContext.getContentResolver().update(
+ ContentUris.withAppendedId(Impl.CONTENT_URI, mId),
+ values, null, null);
}
+ DownloadThread downloader = new DownloadThread(mContext, mSystemFacade, this);
+ mHasActiveThread = true;
+ downloader.start();
}
}
diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java
index e474d4d7..f870954b 100644
--- a/src/com/android/providers/downloads/DownloadService.java
+++ b/src/com/android/providers/downloads/DownloadService.java
@@ -614,7 +614,9 @@ public class DownloadService extends Service {
}
}
- info.startIfReady(now);
+ if (info.isReadyToStart(now)) {
+ info.start(now);
+ }
}
/**
@@ -674,7 +676,9 @@ public class DownloadService extends Service {
info.mMediaScanned =
cursor.getInt(cursor.getColumnIndexOrThrow(Constants.MEDIA_SCANNED)) == 1;
- info.startIfReady(now);
+ if (info.isReadyToRestart(now)) {
+ info.start(now);
+ }
}
/**
diff --git a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
index a9810fc1..e34c66e6 100644
--- a/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
+++ b/tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java
@@ -309,6 +309,19 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
enqueueResponse(HTTP_OK, FILE_CONTENT);
download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
}
+
+ /**
+ * Test for race conditions when the service is flooded with startService() calls while running
+ * a download.
+ */
+ public void testFloodServiceWithStarts() throws Exception {
+ enqueueResponse(HTTP_OK, FILE_CONTENT);
+ Download download = enqueueRequest(getRequest());
+ while (download.getStatus() != DownloadManager.STATUS_SUCCESSFUL) {
+ startService(null);
+ Thread.sleep(10);
+ }
+ }
private DownloadManager.Request getRequest() throws MalformedURLException {
return getRequest(getServerUri(REQUEST_PATH));