Open Bug 1055573 Opened 10 years ago Updated 2 years ago

cache2: opening entry with OPEN_READONLY flag creates an empty cache file on the disk

Categories

(Core :: Networking: Cache, defect, P3)

defect

Tracking

()

People

(Reporter: michal, Unassigned)

Details

(Whiteboard: [necko-backlog])

Attachments

(1 file)

Attached patch readonly.patchSplinter Review
When the entry is opened with OPEN_READONLY, CacheEntry initializes CacheFile which fails to load the entry from disk, so it initializes itself as a new one. CacheEntry checks that the CacheFile is new and returns NS_ERROR_CACHE_KEY_NOT_FOUND correctly. But such entry is present in the cache index and is also later written to the disk because it isn't doomed. The correct behavior here would be to not create a new CacheFileHandle when the file does not exist. I've created a patch that adds aReadOnly argument to CacheFile::Init() but test test_cache2-02-open-non-existing.js fails. I'm not sure what needs to be changed in the upper layer to make the test working.
Attachment #8475192 - Flags: feedback?(honzab.moz)
The entry opens twice as R/O. This means the entry is loaded (asynchronously) with file status == failure. Then we open it normally and don't reload the file, so the entry is not writable (file status left at failure). But logically the entry should now work and be given the consumer as a new empty and writable entry. Can you rather create the file on demand when needed? You can well remember the file doesn't exist and just truncate, I can give you a call to do it actually. Your code is already capable of usage-before-init in the open-truncate mode. I have a patch on the CacheEntry level, but I cannot say I really like it, at least because it can loop or assert and is a change to an otherwise well working code.
Comment on attachment 8475192 [details] [diff] [review] readonly.patch Feedback given in a previous comment.
Attachment #8475192 - Flags: feedback?(honzab.moz)
Whiteboard: [necko-backlog]
Priority: -- → P1
Priority: P1 → P3

Unassigning Michal to move bugs back to triage pool.

Assignee: michal.novotny → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: