948570 - HTTP cache v2: Assertion failure: data.mCacheSize == mIndexStats.Size(), CacheIndex.cpp:850 with the index on win

Status: RESOLVED DUPLICATE of bug 923016

(Core :: Networking: Cache, defect)

Description

[Honza Bambas (:mayhemer)]

https://tbpl.mozilla.org/?tree=Gum&rev=ec2fcd5a61ae

On all win platforms.

There is also
Assertion failure: isEmpty(), at c:\builds\moz2_slave\gum-w32-d-00000000000000000000\build\obj-firefox\dist\include\mozilla/LinkedList.h:305 
https://tbpl.mozilla.org/php/getParsedLog.php?id=31701118&tree=Gum

Not sure it's related.

Comment 1

[Honza Bambas (:mayhemer)]

I've just hit this locally.  Called from CacheIndex::OnFileRenamed with aSucceeded=false since aResult=NS_ERROR_FILE_IS_LOCKED.

When looking at the code...

  ApplyIndexChangesHelper data;
  data.mIndexStats = &mIndexStats;
  data.mCacheSize = 0;

  if (aSucceeded) {
    mIndex.EnumerateEntries(&CacheIndex::ApplyIndexChanges, &data);
    mIndexOnDiskIsValid = true;
  }

  MOZ_ASSERT(data.mCacheSize == mIndexStats.Size());

...should this assert be called when !aSucceeded?

Since:

data.mCacheSize = 0 (obviously)
mIndexStats.Size() = 3001 (not sure what this number means at all, no comments on the memeber or in the code..)

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: log snipppet

What is the most curious is that we are renaming the index file (index.tmp).  I'm saving the whole log (until the assert has happen) if interested.

Comment 3

[Honza Bambas (:mayhemer)]

Attachment: v1

According how I understand the code, it seems like the clear solution is to move this assert to a place where the tested values make sense.

Comment 4

[Michal Novotny [:michal]]

Comment on attachment 8346179 [details] [diff] [review]
v1

Right, the assert should check the state after enumeration. Please check the patch in.

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 8346179 [details] [diff] [review]
v1

Landed on gum: https://hg.mozilla.org/projects/gum/rev/9a67a5eb528a

Comment 6

[Michal Novotny [:michal]]

This patch will be part of the final CacheIndex patch.