loliball/media3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cleanly detach DownloadThreads on release

Browse Source 
- Make a best effort to avoid posting MSG_DOWNLOAD_THREAD_STOPPED
  if the internal thread has already quit. Doing so is harmless,
  but causes an error in Logcat. We used to generate this warning
  quite a bit in ExoPlayerImplInternal as well, and we got quite
  a few issues filed about it, so best to avoid it if possible.
- Null out the back reference DownloadThread holds to its manager
  on release. This avoids a potential issue where a download thread
  can prevent GC of its manager if it doesn't cancel quickly.
  There's similar logic (with a similar comment) in Loader.LoadTask.

PiperOrigin-RevId: 243365143
This commit is contained in:
olly 2019-04-13 01:44:55 +01:00 committed by Oliver Woodman
parent 97acc681d1
commit 2e3eac25b7
1 changed files with 33 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java
View File

@ -718,9 +718,11 @@ public final class DownloadManager {
} }
private void releaseInternal() { private void releaseInternal() {
for (String downloadId : downloadThreads.keySet()) { for (DownloadThread downloadThread : downloadThreads.values()) {
stopDownloadThreadInternal(downloadId); downloadThread.cancel(/* released= */ true);
} }
downloadThreads.clear();
downloadInternals.clear();
internalThread.quit(); internalThread.quit();
synchronized (releaseLock) { synchronized (releaseLock) {
released = true; released = true;
@ -769,7 +771,9 @@ public final class DownloadManager {
} }
simultaneousDownloads++; simultaneousDownloads++;
} }
DownloadThread downloadThread = new DownloadThread(action, isRemove); Downloader downloader = downloaderFactory.createDownloader(action);
DownloadThread downloadThread =
new DownloadThread(action, downloader, isRemove, minRetryCount, internalHandler);
downloadThreads.put(downloadId, downloadThread); downloadThreads.put(downloadId, downloadThread);
downloadInternal.setCounters(downloadThread.downloader.getCounters()); downloadInternal.setCounters(downloadThread.downloader.getCounters());
downloadThread.start(); downloadThread.start();
@ -780,7 +784,7 @@ public final class DownloadManager {
private boolean stopDownloadThreadInternal(String downloadId) { private boolean stopDownloadThreadInternal(String downloadId) {
DownloadThread downloadThread = downloadThreads.get(downloadId); DownloadThread downloadThread = downloadThreads.get(downloadId);
if (downloadThread != null && !downloadThread.isRemove) { if (downloadThread != null && !downloadThread.isRemove) {
downloadThread.cancel(); downloadThread.cancel(/* released= */ false);
logd("Download is cancelled", downloadThread.action); logd("Download is cancelled", downloadThread.action);
return true; return true;
} }
@ -1023,22 +1027,38 @@ public final class DownloadManager {
} }
} }
private class DownloadThread extends Thread { private static class DownloadThread extends Thread {
private final DownloadAction action; private final DownloadAction action;
private final boolean isRemove;
private final Downloader downloader; private final Downloader downloader;
private final boolean isRemove;
private final int minRetryCount;
private volatile Handler onStoppedHandler;
private volatile boolean isCanceled; private volatile boolean isCanceled;
private Throwable finalError; private Throwable finalError;
private DownloadThread(DownloadAction action, boolean isRemove) { private DownloadThread(
DownloadAction action,
Downloader downloader,
boolean isRemove,
int minRetryCount,
Handler onStoppedHandler) {
this.action = action; this.action = action;
this.isRemove = isRemove; this.isRemove = isRemove;
downloader = downloaderFactory.createDownloader(action); this.downloader = downloader;
this.minRetryCount = minRetryCount;
this.onStoppedHandler = onStoppedHandler;
} }
public void cancel() { public void cancel(boolean released) {
if (released) {
// Download threads are GC roots for as long as they're running. The time taken for
// cancellation to complete depends on the implementation of the downloader being used. We
// null the handler reference here so that it doesn't prevent garbage collection of the
// download manager whilst cancellation is ongoing.
onStoppedHandler = null;
}
isCanceled = true; isCanceled = true;
downloader.cancel(); downloader.cancel();
interrupt(); interrupt();
@ -1079,7 +1099,10 @@ public final class DownloadManager {
} catch (Throwable e) { } catch (Throwable e) {
finalError = e; finalError = e;
} }
internalHandler.obtainMessage(MSG_DOWNLOAD_THREAD_STOPPED, this).sendToTarget(); Handler onStoppedHandler = this.onStoppedHandler;
if (onStoppedHandler != null) {
onStoppedHandler.obtainMessage(MSG_DOWNLOAD_THREAD_STOPPED, this).sendToTarget();
}
} }
private int getRetryDelayMillis(int errorCount) { private int getRetryDelayMillis(int errorCount) {