1. Load the url ftp://ftp.mozilla.org/welcome.msg 2. Press reload You get an empty directory Whats happening is that this is being placed into the cache, so when we pull it out, we try to treat is as a directory. Parsing fails, so we error out and remove the entry. This means that every second load fails. Brad is orange because of this. This happened somewhere arround 16:15-18:15 on 23rd of Jan. There were cache checkins arround then, but no ftp ones. I'll debug this later on, but the entry is in about:cache.

I was wrong in earlier defining the window. The last good build for 'brad' was pulled at about 16:15pm 01/23. Then there were two cycles of red, and then an orange build, pulled at about 18:42 01/23. The orange build fails on ftp://ftp.mozilla.org/welcome.msg while running a second bloaturls.txt sequence.

times stamps in logs are: last succesful pulled 16:21 PST 01/23, and the orange build pulled at 18:43 PST 01/23, with two red cycles in between.

cvs update -j1.19 -j1.18 mozilla/netwerk/cache/src/nsCacheMetaData.cpp "fixes" this in a current build on Linux (the above from bug 119085). With that backed out, ftp://ftp.mozilla.org/welcome.msg loads cleanly every time.

The patch jrgm found was to allow entries with no data to be consdiered valid. Before that change, the entry was ignored, which is why we didn't notice this. The bug here is that we need to not markValid directory entries, so we set the cache descriptor we have to null. I don't know if the call to Doom() is needed, but it won't hurt. gordon/darin/dougt: Does this look right? It fixes the problem.

this seems fine to me. of course, dooming the cache entry causes it to be deleted from the cache directory. is that what you want? should it have been created in the first place if it is only going to be doomed later on?

Comment on attachment 67067 [details] [diff] [review] patch sr=darin

Adding orange tinderbox people.

Yeah. The problem is that we can't know that its a directory until we've tried it. So we always need to try to read the entry, because we may write to it. Theres also nothing wrong in theory with us caching files less than a certain size, since we can know the size in advance. Since we never mark it valid (any more), dooming probably doesn't do much, though.

closing a cache entry that has not been marked valid will mark it valid, so dooming the cache entry before closing it does indeed make a difference.

OK, good. Thats what I want, and thats what the patch does. (Its never actually explicitly closed, anyway)

i should have made that clearer... no need to explicitly close the descriptor. the final release is equivalent to an explicit close.

Yeah, I thought so. So, how about a review? dougt? gordon?

Checked in.

*** Bug 128548 has been marked as a duplicate of this bug. ***

VERIFIED: general behavior has appeared correct since then.