coil-kt / coil

Image loading for Android and Compose Multiplatform.
https://coil-kt.github.io/coil/
Apache License 2.0
10.81k stars 662 forks source link

DiskCache should delete files that no longer exist during initialization. #2667

Open panpf opened 2 days ago

panpf commented 2 days ago

Describe the bug I used the DiskLruCache in my project, and I found a problem.

After calling evictAll(), all entities in memory and files on disk have indeed been deleted, and size() also returns 0. However, when the App is killed from the recent tasks and reopened (the test ensured that no download behavior was triggered after restart), the result returned by DiskLruCache's size() is greater than 0.

I pulled the journal files before and after the restart and compared them and found that after the App was restarted, there were still CLEAN records of all or part of the files.

I repeatedly studied the source code of DiskLruCache and found that flush() was not executed in evictAll() or flush() was triggered only in the middle of the loop execution of removeEntry(). The REMOVE record in the journal was incomplete when the app was restarted. Then, when DiskLruCache was initialized after the App was restarted, the Entry was added to lruEntries without checking whether the Entry file existed, which eventually caused this problem.

To Reproduce Any API LEVEL can be reproduced on the Android emulator

panpf commented 2 days ago

My modification suggestions:

fun initialize() = synchronized(lock) {
    if (initialized) return

    // If a temporary file exists, delete it.
    fileSystem.delete(journalFileTmp)

    // If a backup file exists, use it instead.
    if (fileSystem.exists(journalFileBackup)) {
        // If journal file also exists just delete backup file.
        if (fileSystem.exists(journalFile)) {
            fileSystem.delete(journalFileBackup)
        } else {
            fileSystem.atomicMove(journalFileBackup, journalFile)
        }
    }

    // Prefer to pick up where we left off.
    if (fileSystem.exists(journalFile)) {
        try {
            readJournal()
            processJournal()
            filterFileNotExistEntry()
            initialized = true
            return
        } catch (_: IOException) {
            // The journal is corrupt.
        }

        // The cache is corrupted; attempt to delete the contents of the directory.
        // This can throw and we'll let that propagate out as it likely means there
        // is a severe filesystem problem.
        try {
            delete()
        } finally {
            closed = false
        }
    }

    writeJournal()
    initialized = true
}

private fun filterFileNotExistEntry() {
    for (entry in lruEntries.values.toTypedArray()) {
        if (entry.readable && entry.currentEditor == null && !entry.zombie) {
            val allExist = entry.cleanFiles.all { fileSystem.exists(it) }
            if (!allExist) {
                removeEntry(entry)
            }
        }
    }
}