This comes from bug #588804, comment #60. If something bad happens when writing or flushing data or metadata for a cache-entry the entry should be doomed and the cache should be cleaned up.

Assigned, added dependency and requesting blocking2.0

New patch fixing one more problem. If writing of metadata fails then the eviction rank in record and cachemap could get out of sync, which causes an assertion in nsDiskCacheMap::DeleteRecord().

Comment on attachment 483876 [details] [diff] [review] fix Looks good. The added comment in WriteDiskCacheEntry() is a little confusing - please be more specific about which failure is acceptable. Also: have you checked the other two cache-devices for similar issues? I guess it would be hard to write a testcase for this but if possible, please give it a shot.

(In reply to comment #3) > Looks good. The added comment in WriteDiskCacheEntry() is a little confusing - > please be more specific about which failure is acceptable. Also: have you > checked the other two cache-devices for similar issues? It isn't acceptable, we just need to fail after the call to UpdateRecord(). I've changed the comment a bit. I checked both and didn't find similar problem there. > I guess it would be hard to write a testcase for this but if possible, please > give it a shot. Actually it shouldn't be too hard to write the test now, but after fixing bug #604897 it wouldn't work anymore.

Comment on attachment 483999 [details] [diff] [review] patch v3 Looks fine, assuming the other devices are not subject tho similar issues. > // flush buffer to block files > if (mStreamEnd > 0) { > rv = cacheMap->WriteDataCacheBlocks(mBinding, mBuffer, mBufEnd); > if (NS_FAILED(rv)) { > NS_WARNING("WriteDataCacheBlocks() failed."); >- return rv; // XXX doom cache entry? >- >+ nsCacheService::DoomEntry(mBinding->mCacheEntry); >+ mBufDirty = PR_FALSE; >+ return rv; > } > } > > mBufDirty = PR_FALSE; Clear |mBufDirty| before the if-block? r=bjarne with this.

Blocking.

Comment on attachment 484356 [details] [diff] [review] patch v4 I'm not a superreviewer. Reassign to bz or biesi?

Comment on attachment 484356 [details] [diff] [review] patch v4 sr=me

Checked in. http://hg.mozilla.org/mozilla-central/rev/5ae431772486