Cache remove bug

[Caij]

Version

Media3 1.4.0

More version details

1. SimpleCache.initialize
   initialize -> loadDirectory -> SimpleCacheSpan create -> addSpan -> notifySpanAdded -> LeastRecentlyUsedCacheEvictor.onSpanAdded -> leastRecentlyUsed.add(span)

2. ContentIndex.removeEmpty
   SimpleCache.initialize -> ContentIndex.removeEmpty -> if have empty content, This content will be deleted.

3. LeastRecentlyUsedCacheEvictor.evictCache， the Cache removeSpanInternal has been return, so while (currentSize + requiredSpace > maxBytes && !leastRecentlyUsed.isEmpty()) Unable to end.
   eg: span file ha deleted, ContentIndex emoty

 private void evictCache(Cache cache, long requiredSpace) {
    while (currentSize + requiredSpace > maxBytes && !leastRecentlyUsed.isEmpty()) { // Unable to end
      cache.removeSpan(leastRecentlyUsed.first());
    }
  }

 private void removeSpanInternal(CacheSpan span) {
    @Nullable CachedContent cachedContent = contentIndex.get(span.key);
    if (cachedContent == null || !cachedContent.removeSpan(span)) {
      return;
    }
    totalSpace -= span.length;
    if (fileIndex != null) {
      String fileName = Assertions.checkNotNull(span.file).getName();
      try {
        fileIndex.remove(fileName);
      } catch (IOException e) {
        // This will leave a stale entry in the file index. It will be removed next time the cache
        // is initialized.
        Log.w(TAG, "Failed to remove file index entry for: " + fileName);
      }
    }
    contentIndex.maybeRemove(cachedContent.key);
    notifySpanRemoved(span);
  }

  public void onSpanRemoved(Cache cache, CacheSpan span) {
    leastRecentlyUsed.remove(span);
    currentSize -= span.length;
  }

Devices that reproduce the issue

any

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

No

Reproduction steps

1 play media
2 When the cache is full

Expected result

the cache remove success

Actual result

the cache remove fail

Media

none

Bug Report

• You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.

[tianyif]

Hi @Caij,

Could you please send the bug report again as I didn't see it in our mailbox? Also, could you please provide a detailed steps that can reliably reproduce the issue? I couldn't repro the issue by simply playing/downloading media until the cache exceeds the maxBytes that I set to the evictor.

By just looking at the mentioned logic, I didn't see how the loop

media/libraries/datasource/src/main/java/androidx/media3/datasource/cache/LeastRecentlyUsedCacheEvictor.java

Lines 73 to 75 in df887a9

	while (currentSize + requiredSpace > maxBytes && !leastRecentlyUsed.isEmpty()) {
	cache.removeSpan(leastRecentlyUsed.first());
	}

can fail to end due to the removeEmpty in the initialize() method.

As suggested from its name, CachedContentIndex.removeEmpty will be removing the empty CachedContent without the spans, thus it's unlikely to see how this early return hit

media/libraries/datasource/src/main/java/androidx/media3/datasource/cache/SimpleCache.java

Lines 700 to 702 in df887a9

	if (cachedContent == null || !cachedContent.removeSpan(span)) {
	return;
	}

, as the spans tracked by LeastRecentlyUsedCacheEvictor should already be added

media/libraries/datasource/src/main/java/androidx/media3/datasource/cache/SimpleCache.java

Lines 693 to 695 in df887a9

	contentIndex.getOrAdd(span.key).addSpan(span);
	totalSpace += span.length;
	notifySpanAdded(span);

.

There might be some corner case that I overlooked, but I think a real repro can only help to investigate here.

[Caij]

There is another way to reproduce this issue:

1. Set the cache value to be smaller and then play multiple media files until the cache is full.
2. Use the Android Studio file manager to go to app/cache and delete the cached files.
3. At this point, it will return, but the loop cannot exit.
   while (currentSize + requiredSpace > maxBytes && !leastRecentlyUsed.isEmpty()) {
   cache.removeSpan(leastRecentlyUsed.first());
   }
   if (cachedContent == null || !cachedContent.removeSpan(span)) {
   return;
   }