Skip to content

DownloadManager task sync creates too many concurrently-running threads for moderate amounts of STATE_RESTARTING or STATE_REMOVING downloads in index #10458

Closed

Description

ExoPlayer Version

2.18.0

Devices that reproduce the issue

Pixel 2 emulator API 31
TCL 10 SE TCL UI 3.0.7.K2V (API 30)

Devices that do not reproduce the issue

None found

Reproducible in the demo app?

Yes

Reproduction steps

Simple setup:

  1. Check out https://github.com/stoyicker/mwe-exoplayer-downloadmanager-oom/tree/7d36b7c2e3cff7956b70de9700d192ca5874acec It is an empty app using exoplayer 2.18.0 set up to create a downloadmanager with an index that will get problematic downloads.
  2. Choose an appropriate number of downloads with problematic state to add to the index. For this setup, my emulator would crash with 1.5k and my device with 2k. Using values too low for your setup will result in the expected result.
  3. Run the app.
  4. When the app shows the black screen stating it's ready, tap it. This will start adding downloads with one of the states mentioned in the title to the index to simulate the problematic situation, and when it's done it'll change the screen color to red and proceed to create a DownloadManager with this index.

Demo app setup:

  1. Check out https://github.com/stoyicker/ExoPlayer/tree/a2e23ae8f700556c9c6cc6f08e377fb81a2621ae to recreate scenario. It is the tag for 2.18.0 with a customized download index that will get problematic downloads added.
  2. Choose an appropriate number of downloads with problematic state to add to the index. For this setup, my emulator would crash with 4k (although I noticed that smaller amounts would print the exception as a log instead of crashing, but I couldn't pinpoint from where) and haven't tested my device. Using values too low for your setup will result in the expected result.
  3. Run the app.
  4. Wait for the list of demos to render, which will mean that the index has been filled with the requested amount of downloads.
  5. Wait a few seconds after the list of demos renders. Less than 30s should do.

Make sure you always uninstall the app or clear its data when trying anew to avoid picking up the old index. Also note that this applies not only on initialization, but on any calls that trigger a call to syncTasks with the appropriate amount of downloads in the problematic states.

Expected result

Simple setup:

When the DownloadManager is initialized (as in, the activeTask map filled because of the syncTasks call in initialize has been emptied, not when the callback has been invoked), the screen will change to blue, so it is expected that after a few seconds this happens.

Demo app setup:

The app continues to run after 30s of wait (starting after the list has been drawn).

Actual result

Simple setup:

You never get to the blue screen. Instead, the app crashes with the exception below.

Demo app setup:

It crashes because of the exception below.

Example stacktrace:

java.lang.OutOfMemoryError: Failed to allocate a 131088 byte allocation with 80744 free bytes and 78KB until OOM, target footprint 536870912, growth limit 536870912
        at com.google.android.exoplayer2.upstream.cache.CacheWriter.<init>(CacheWriter.java:78)
        at com.google.android.exoplayer2.offline.ProgressiveDownloader.<init>(ProgressiveDownloader.java:82)
        at com.google.android.exoplayer2.offline.DefaultDownloaderFactory.createDownloader(DefaultDownloaderFactory.java:83)
        at com.google.android.exoplayer2.offline.DownloadManager$InternalHandler.syncRemovingDownload(DownloadManager.java:1062)
        at com.google.android.exoplayer2.offline.DownloadManager$InternalHandler.syncTasks(DownloadManager.java:986)
        at com.google.android.exoplayer2.offline.DownloadManager$InternalHandler.initialize(DownloadManager.java:816)
        at com.google.android.exoplayer2.offline.DownloadManager$InternalHandler.handleMessage(DownloadManager.java:737)

I'm fairly certain this is due to the amount of tasks started from syncRemovingDownload not being capped, for example such as the the ones from syncQueuedDownload which are capped by the amount of parallel downloads. I was thinking of drafting a solution where Task is changed to extend Runnable instead of Thread and then a new setter is introduced, akin to that for max parallel downloads, to control the amount of threads in an executor that we submit tasks to, but wanted to run the issue by you before. Note that I understand that 2k may seem a large amount of items to have removing, but I don't think it is a justification to consider this a corner-case given that not only does it reproduces consistently in an unrealistically simple app, but also in the demo app (which sets largeHeap to true) with a not-much-higher-number.

Media

N/A

Bug Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions