From 6da2d0a2e1d046cd39270a46c3c77f8234f4cfaa Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 13 Apr 2012 12:34:18 -0700 Subject: Locking around downloads, and more dump info. Bug: 4997552 Change-Id: I3c612acb5145a7638c9345a376a99958851a0891 --- .../android/providers/downloads/DownloadInfo.java | 64 ++++--- .../providers/downloads/DownloadService.java | 193 +++++++++++---------- 2 files changed, 143 insertions(+), 114 deletions(-) (limited to 'src/com/android/providers') diff --git a/src/com/android/providers/downloads/DownloadInfo.java b/src/com/android/providers/downloads/DownloadInfo.java index 304d70fd..1e670e12 100644 --- a/src/com/android/providers/downloads/DownloadInfo.java +++ b/src/com/android/providers/downloads/DownloadInfo.java @@ -34,6 +34,8 @@ import android.text.TextUtils; import android.util.Log; import android.util.Pair; +import com.android.internal.util.IndentingPrintWriter; + import java.io.PrintWriter; import java.util.ArrayList; import java.util.Collection; @@ -480,29 +482,45 @@ public class DownloadInfo { return ContentUris.withAppendedId(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, mId); } - public void dump(PrintWriter writer) { - writer.println("DownloadInfo:"); - - writer.print(" mId="); writer.print(mId); - writer.print(" mLastMod="); writer.print(mLastMod); - writer.print(" mPackage="); writer.print(mPackage); - writer.print(" mUid="); writer.println(mUid); - - writer.print(" mUri="); writer.print(mUri); - writer.print(" mMimeType="); writer.print(mMimeType); - writer.print(" mCookies="); writer.print((mCookies != null) ? "yes" : "no"); - writer.print(" mReferer="); writer.println((mReferer != null) ? "yes" : "no"); - - writer.print(" mUserAgent="); writer.println(mUserAgent); - - writer.print(" mFileName="); writer.println(mFileName); - - writer.print(" mStatus="); writer.print(mStatus); - writer.print(" mCurrentBytes="); writer.print(mCurrentBytes); - writer.print(" mTotalBytes="); writer.println(mTotalBytes); - - writer.print(" mNumFailed="); writer.print(mNumFailed); - writer.print(" mRetryAfter="); writer.println(mRetryAfter); + public void dump(IndentingPrintWriter pw) { + pw.println("DownloadInfo:"); + pw.increaseIndent(); + + pw.printPair("mId", mId); + pw.printPair("mLastMod", mLastMod); + pw.printPair("mPackage", mPackage); + pw.printPair("mUid", mUid); + pw.println(); + + pw.printPair("mUri", mUri); + pw.println(); + + pw.printPair("mMimeType", mMimeType); + pw.printPair("mCookies", (mCookies != null) ? "yes" : "no"); + pw.printPair("mReferer", (mReferer != null) ? "yes" : "no"); + pw.printPair("mUserAgent", mUserAgent); + pw.println(); + + pw.printPair("mFileName", mFileName); + pw.printPair("mDestination", mDestination); + pw.println(); + + pw.printPair("mStatus", Downloads.Impl.statusToString(mStatus)); + pw.printPair("mCurrentBytes", mCurrentBytes); + pw.printPair("mTotalBytes", mTotalBytes); + pw.println(); + + pw.printPair("mNumFailed", mNumFailed); + pw.printPair("mRetryAfter", mRetryAfter); + pw.printPair("mETag", mETag); + pw.printPair("mIsPublicApi", mIsPublicApi); + pw.println(); + + pw.printPair("mAllowedNetworkTypes", mAllowedNetworkTypes); + pw.printPair("mAllowRoaming", mAllowRoaming); + pw.println(); + + pw.decreaseIndent(); } /** diff --git a/src/com/android/providers/downloads/DownloadService.java b/src/com/android/providers/downloads/DownloadService.java index 5fb46fb9..f77a3bf0 100644 --- a/src/com/android/providers/downloads/DownloadService.java +++ b/src/com/android/providers/downloads/DownloadService.java @@ -16,9 +16,6 @@ package com.android.providers.downloads; -import com.google.android.collect.Maps; -import com.google.common.annotations.VisibleForTesting; - import android.app.AlarmManager; import android.app.PendingIntent; import android.app.Service; @@ -40,14 +37,20 @@ import android.provider.Downloads; import android.text.TextUtils; import android.util.Log; +import com.android.internal.util.IndentingPrintWriter; +import com.google.android.collect.Maps; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; + import java.io.File; import java.io.FileDescriptor; import java.io.PrintWriter; +import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; - /** * Performs the background downloads requested by applications that use the Downloads provider. */ @@ -287,102 +290,104 @@ public class DownloadService extends Service { mPendingUpdate = false; } - long now = mSystemFacade.currentTimeMillis(); - boolean mustScan = false; - keepService = false; - wakeUp = Long.MAX_VALUE; - Set idsNoLongerInDatabase = new HashSet(mDownloads.keySet()); - - Cursor cursor = getContentResolver().query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, - null, null, null, null); - if (cursor == null) { - continue; - } - try { - DownloadInfo.Reader reader = - new DownloadInfo.Reader(getContentResolver(), cursor); - int idColumn = cursor.getColumnIndexOrThrow(Downloads.Impl._ID); - if (Constants.LOGVV) { - Log.i(Constants.TAG, "number of rows from downloads-db: " + - cursor.getCount()); + synchronized (mDownloads) { + long now = mSystemFacade.currentTimeMillis(); + boolean mustScan = false; + keepService = false; + wakeUp = Long.MAX_VALUE; + Set idsNoLongerInDatabase = new HashSet(mDownloads.keySet()); + + Cursor cursor = getContentResolver().query(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, + null, null, null, null); + if (cursor == null) { + continue; } - for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) { - long id = cursor.getLong(idColumn); - idsNoLongerInDatabase.remove(id); - DownloadInfo info = mDownloads.get(id); - if (info != null) { - updateDownload(reader, info, now); - } else { - info = insertDownload(reader, now); + try { + DownloadInfo.Reader reader = + new DownloadInfo.Reader(getContentResolver(), cursor); + int idColumn = cursor.getColumnIndexOrThrow(Downloads.Impl._ID); + if (Constants.LOGVV) { + Log.i(Constants.TAG, "number of rows from downloads-db: " + + cursor.getCount()); } + for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) { + long id = cursor.getLong(idColumn); + idsNoLongerInDatabase.remove(id); + DownloadInfo info = mDownloads.get(id); + if (info != null) { + updateDownload(reader, info, now); + } else { + info = insertDownloadLocked(reader, now); + } - if (info.shouldScanFile() && !scanFile(info, true, false)) { - mustScan = true; - keepService = true; - } - if (info.hasCompletionNotification()) { - keepService = true; - } - long next = info.nextAction(now); - if (next == 0) { - keepService = true; - } else if (next > 0 && next < wakeUp) { - wakeUp = next; + if (info.shouldScanFile() && !scanFile(info, true, false)) { + mustScan = true; + keepService = true; + } + if (info.hasCompletionNotification()) { + keepService = true; + } + long next = info.nextAction(now); + if (next == 0) { + keepService = true; + } else if (next > 0 && next < wakeUp) { + wakeUp = next; + } } + } finally { + cursor.close(); } - } finally { - cursor.close(); - } - for (Long id : idsNoLongerInDatabase) { - deleteDownload(id); - } + for (Long id : idsNoLongerInDatabase) { + deleteDownloadLocked(id); + } - // is there a need to start the DownloadService? yes, if there are rows to be - // deleted. - if (!mustScan) { - for (DownloadInfo info : mDownloads.values()) { - if (info.mDeleted && TextUtils.isEmpty(info.mMediaProviderUri)) { - mustScan = true; - keepService = true; - break; + // is there a need to start the DownloadService? yes, if there are rows to be + // deleted. + if (!mustScan) { + for (DownloadInfo info : mDownloads.values()) { + if (info.mDeleted && TextUtils.isEmpty(info.mMediaProviderUri)) { + mustScan = true; + keepService = true; + break; + } } } - } - mNotifier.updateNotification(mDownloads.values()); - if (mustScan) { - bindMediaScanner(); - } else { - mMediaScannerConnection.disconnectMediaScanner(); - } + mNotifier.updateNotification(mDownloads.values()); + if (mustScan) { + bindMediaScanner(); + } else { + mMediaScannerConnection.disconnectMediaScanner(); + } - // look for all rows with deleted flag set and delete the rows from the database - // permanently - for (DownloadInfo info : mDownloads.values()) { - if (info.mDeleted) { - // this row is to be deleted from the database. but does it have - // mediaProviderUri? - if (TextUtils.isEmpty(info.mMediaProviderUri)) { - if (info.shouldScanFile()) { - // initiate rescan of the file to - which will populate - // mediaProviderUri column in this row - if (!scanFile(info, false, true)) { - throw new IllegalStateException("scanFile failed!"); + // look for all rows with deleted flag set and delete the rows from the database + // permanently + for (DownloadInfo info : mDownloads.values()) { + if (info.mDeleted) { + // this row is to be deleted from the database. but does it have + // mediaProviderUri? + if (TextUtils.isEmpty(info.mMediaProviderUri)) { + if (info.shouldScanFile()) { + // initiate rescan of the file to - which will populate + // mediaProviderUri column in this row + if (!scanFile(info, false, true)) { + throw new IllegalStateException("scanFile failed!"); + } + continue; } - continue; + } else { + // yes it has mediaProviderUri column already filled in. + // delete it from MediaProvider database. + getContentResolver().delete(Uri.parse(info.mMediaProviderUri), null, + null); } - } else { - // yes it has mediaProviderUri column already filled in. - // delete it from MediaProvider database. - getContentResolver().delete(Uri.parse(info.mMediaProviderUri), null, - null); + // delete the file + deleteFileIfExists(info.mFileName); + // delete from the downloads db + getContentResolver().delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, + Downloads.Impl._ID + " = ? ", + new String[]{String.valueOf(info.mId)}); } - // delete the file - deleteFileIfExists(info.mFileName); - // delete from the downloads db - getContentResolver().delete(Downloads.Impl.ALL_DOWNLOADS_CONTENT_URI, - Downloads.Impl._ID + " = ? ", - new String[]{String.valueOf(info.mId)}); } } } @@ -424,7 +429,7 @@ public class DownloadService extends Service { * Keeps a local copy of the info about a download, and initiates the * download if appropriate. */ - private DownloadInfo insertDownload(DownloadInfo.Reader reader, long now) { + private DownloadInfo insertDownloadLocked(DownloadInfo.Reader reader, long now) { DownloadInfo info = reader.newDownloadInfo(this, mSystemFacade); mDownloads.put(info.mId, info); @@ -466,7 +471,7 @@ public class DownloadService extends Service { /** * Removes the local copy of the info about a download. */ - private void deleteDownload(long id) { + private void deleteDownloadLocked(long id) { DownloadInfo info = mDownloads.get(id); if (info.shouldScanFile()) { scanFile(info, false, false); @@ -561,8 +566,14 @@ public class DownloadService extends Service { @Override protected void dump(FileDescriptor fd, PrintWriter writer, String[] args) { - for (DownloadInfo info : mDownloads.values()) { - info.dump(writer); + final IndentingPrintWriter pw = new IndentingPrintWriter(writer, " "); + synchronized (mDownloads) { + final List ids = Lists.newArrayList(mDownloads.keySet()); + Collections.sort(ids); + for (Long id : ids) { + final DownloadInfo info = mDownloads.get(id); + info.dump(pw); + } } } } -- cgit v1.2.3