From 18b3a058062793291acc85d43d951a78e2f132fc Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 4 May 2016 13:48:27 +0200 Subject: [PATCH] always cancel APK download progress reports Before, these were not being reliably canceled before the final COMPLETE or INTERRUPTED notification went out. This moves closing the progress Timer to a finally block after the Timer is setup, which should hopefully guarantee the progress reports are always stopped. This prevents any more progress reports from being sent, in case there is one pending anywhere. downloaderProgressListener needs to be volatile to ensure that the two threads are seeing the same values. This was an omission on my part when putting together the DownloaderService in !248 --- .../org/fdroid/fdroid/net/Downloader.java | 66 +++++++++---------- .../fdroid/fdroid/net/DownloaderService.java | 24 ++++--- 2 files changed, 44 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java index 86e7f2685..7981c2571 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/Downloader.java +++ b/app/src/main/java/org/fdroid/fdroid/net/Downloader.java @@ -30,7 +30,6 @@ public abstract class Downloader { private volatile boolean cancelled = false; private volatile int bytesRead; private volatile int totalBytes; - private Timer timer; public final File outputFile; @@ -49,7 +48,7 @@ public abstract class Downloader { /** * For sending download progress, should only be called in {@link #progressTask} */ - private DownloaderProgressListener downloaderProgressListener; + private volatile DownloaderProgressListener downloaderProgressListener; protected abstract InputStream getDownloadersInputStream() throws IOException; @@ -131,9 +130,6 @@ public abstract class Downloader { private void throwExceptionIfInterrupted() throws InterruptedException { if (cancelled) { Utils.debugLog(TAG, "Received interrupt, cancelling download"); - if (timer != null) { - timer.cancel(); - } throw new InterruptedException(); } } @@ -151,40 +147,44 @@ public abstract class Downloader { * progress counter. */ private void copyInputToOutputStream(InputStream input, int bufferSize, OutputStream output) throws IOException, InterruptedException { - bytesRead = 0; - totalBytes = totalDownloadSize(); - byte[] buffer = new byte[bufferSize]; + Timer timer = new Timer(); + try { + bytesRead = 0; + totalBytes = totalDownloadSize(); + byte[] buffer = new byte[bufferSize]; - timer = new Timer(); - timer.scheduleAtFixedRate(progressTask, 0, 100); - - // Getting the total download size could potentially take time, depending on how - // it is implemented, so we may as well check this before we proceed. - throwExceptionIfInterrupted(); - - while (true) { - - int count; - if (input.available() > 0) { - int readLength = Math.min(input.available(), buffer.length); - count = input.read(buffer, 0, readLength); - } else { - count = input.read(buffer); - } + timer.scheduleAtFixedRate(progressTask, 0, 100); + // Getting the total download size could potentially take time, depending on how + // it is implemented, so we may as well check this before we proceed. throwExceptionIfInterrupted(); - if (count == -1) { - Utils.debugLog(TAG, "Finished downloading from stream"); - break; + while (true) { + + int count; + if (input.available() > 0) { + int readLength = Math.min(input.available(), buffer.length); + count = input.read(buffer, 0, readLength); + } else { + count = input.read(buffer); + } + + throwExceptionIfInterrupted(); + + if (count == -1) { + Utils.debugLog(TAG, "Finished downloading from stream"); + break; + } + bytesRead += count; + output.write(buffer, 0, count); } - bytesRead += count; - output.write(buffer, 0, count); + } finally { + downloaderProgressListener = null; + timer.cancel(); + timer.purge(); + output.flush(); + output.close(); } - timer.cancel(); - timer.purge(); - output.flush(); - output.close(); } /** diff --git a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java index ade4f30de..d7ddc2398 100644 --- a/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java +++ b/app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java @@ -280,20 +280,18 @@ public class DownloaderService extends Service { downloader.setListener(new Downloader.DownloaderProgressListener() { @Override public void sendProgress(URL sourceUrl, int bytesRead, int totalBytes) { - if (isActive(uri.toString())) { - Intent intent = new Intent(Downloader.ACTION_PROGRESS); - intent.setData(uri); - intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); - intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); - localBroadcastManager.sendBroadcast(intent); + Intent intent = new Intent(Downloader.ACTION_PROGRESS); + intent.setData(uri); + intent.putExtra(Downloader.EXTRA_BYTES_READ, bytesRead); + intent.putExtra(Downloader.EXTRA_TOTAL_BYTES, totalBytes); + localBroadcastManager.sendBroadcast(intent); - if (Preferences.get().isUpdateNotificationEnabled()) { - NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); - Notification notification = createNotification(uri.toString(), packageName) - .setProgress(totalBytes, bytesRead, false) - .build(); - nm.notify(NOTIFY_DOWNLOADING, notification); - } + if (Preferences.get().isUpdateNotificationEnabled()) { + NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); + Notification notification = createNotification(uri.toString(), packageName) + .setProgress(totalBytes, bytesRead, false) + .build(); + nm.notify(NOTIFY_DOWNLOADING, notification); } } });