Skip to content
This repository was archived by the owner on Jul 21, 2023. It is now read-only.
This repository was archived by the owner on Jul 21, 2023. It is now read-only.

Silent commit failure due to ignore renameTo return value #70

@TWiStErRob

Description

@TWiStErRob
DiskLruCache.Editor editor = getDiskCache().edit(key);
try {
    File file = editor.getFile(0);
    new FileOutputStream(file).write(data);
    editor.commit();
} finally {
    editor.abortUnlessCommitted();
}

Now, commit calls completeEdit which has a line dirty.renameTo(clean);
File.renameTo has a return value which is discarded. This may fail silently due to a clumsy way of writing the file (as seen above) and think that the commit was commited. However later when getDiskCache.get(key) is called it sees that the file is missing for the wrong reason.

The reason for renameTo failing is because there still a handle open for it in the process and the OS (Windows) may prevent the rename and return false. (side note: the strange thing is that in the past I've been able to rename running/and-locked .exe files without any problems from non-Java apps)

The line was mentioned in #67 where if you check the return value the operation will be still atomic and should throw an IOException as you do in 4c31913 which is a fix for #32.

Also there are other 3 File.delete calls which are not checked, mostly related to backup files but still worth checking the code (and intentionally ignoring it with a comment maybe).

This issue was discovered when trying to build: https://github.com/sjudd/glide/tree/3.0a

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions