LRUCacheEvictor spins, causing it to lock the SimpleCache

[davidliu]

Issue description

LeastRecentlyUsedCacheEvictor.java:evictCache() spins forever, because it tries to remove an item that's no longer contained inside the cache. That is, the LRUCacheEvictor has spans that it tries to evict, but those spans are not inside CachedContent.

Since the eviction happens from within a synchronized thread, any other threads also block until this evict completes (never). Thus, all loading is blocked forever until the app dies.

Reproduction steps

Unknown. I have an app that has many videos in a recyclerview, but I'm unsure how this particular issue arises. The only obvious information is that the cache must be filled and then try to load a new video that goes into the eviction death loop.

Version of ExoPlayer being used

2.5.1

Device(s) and version(s) of Android being used

Various devices, on android 6 and 7.

A full bug report captured from the device

bugreport-NRD90U-2017-09-12-22-30-48.zip

stacktrace.txt

[davidliu]

What should happen is that there should be some sort of error fall back for the LeastRecentlyUsedCacheEvictor, and if removal fails, then it should move onto the next piece to remove instead of constantly trying the same span.

I believe that what happens is that during the cache eviction process, if there's a IOException in index.store(), then that prevents the CacheEvictor from getting notified that a span got removed, even though the CacheContentIndex has already removed it.

[ojw28]

@erdemguven - I guess notifySpanRemoved should be called in a finally block in SimpleCache.removeSpan, to ensure it's called even if the index.store call fails?

[bnap00]

facing simillar issue.

Exception java.lang.NullPointerException: Attempt to invoke virtual method 'boolean wm.a(com.google.android.exoplayer2.upstream.cache.CacheSpan)' on a null object reference
com.google.android.exoplayer2.upstream.cache.SimpleCache.removeSpan (SimpleCache.java:289)
com.google.android.exoplayer2.upstream.cache.SimpleCache.removeSpan (SimpleCache.java:300)
com.google.android.exoplayer2.upstream.cache.LeastRecentlyUsedCacheEvictor.evictCache (LeastRecentlyUsedCacheEvictor.java:79)
com.google.android.exoplayer2.upstream.cache.LeastRecentlyUsedCacheEvictor.onSpanAdded (LeastRecentlyUsedCacheEvictor.java:51)
com.google.android.exoplayer2.upstream.cache.SimpleCache.notifySpanAdded (SimpleCache.java:341)
com.google.android.exoplayer2.upstream.cache.SimpleCache.addSpan (SimpleCache.java:284)
com.google.android.exoplayer2.upstream.cache.SimpleCache.initialize (SimpleCache.java:266)
com.google.android.exoplayer2.upstream.cache.SimpleCache.access$000 (SimpleCache.java:33)
com.google.android.exoplayer2.upstream.cache.SimpleCache.access$102 (SimpleCache.java:33)
com.google.android.exoplayer2.upstream.cache.SimpleCache$1.run (SimpleCache.java:77)

[ojw28]

This is fixed in dev-v2, and will be in r2.5.3, which will be released some time this week.

[davidliu]

Issue still occurs on r2.5.3. It still spins, trying to remove a span that isn't there. Issue should probably be reopened.

[ojw28]

I don't really see how this can happen, so I think you need to figure out a better way to analyze what's going on if we're to make any progress here. Given our own apps don't experience this problem, I suspect it's something wrong with your application code (possibly because certain things aren't well enough documented on the ExoPlayer side). Some common pitfalls that I could see causing this issue:

• Passing the same LeastRecentlyUsedCacheEvictor instance into multiple SimpleCache instances. Each evictor tracks the state of the cache that it corresponds to, so passing the same instance to multiple cache instances will confuse it.
• Instantiating multiple SimpleCache instances that point to the same directory. This is not a safe thing to do. Typically applications instantiate a single SimpleCache instance and keep it somewhere globally accessible. It is safe to have multiple players using the same SimpleCache.

[davidliu]

Thanks for the follow up. After some analysis on our end, it turned out to be exactly what you thought, with multiple SimpleCache instances occurring. Hasn't reoccurred after fixing that issue.