1369051 - Assertion failure: !entry || !entry->IsFresh(), at /home/nicolas/mozilla/contrib-push/netwerk/cache2/CacheIndex.cpp:3129

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Nicolas B. Pierron [:nbp]]

Log file produced with MOZ_LOG='ScriptLoader:5,cache2:5,timestamp,sync' :
https://drive.google.com/file/d/0B-1Yv9nwEU69YVl1Z1ZYdG50bk0/view?usp=sharing

I was able to reproduce this issue with the following preferences:
  - dom.script_loader.bytecode_cache.enabled = true
  - dom.script_loader.bytecode_cache.eager = true (before Bug 1368675)
  - dom.script_loader.bytecode_cache.strategy = -1 (after Bug 1368675)

And by restoring a sessions with the following tabs opened, and visiting all the tabs, one by one with Shift+Tab quickly:
  - https://blog.nightly.mozilla.org/
  - https://matthew.noorenberghe.com/blog/improved-form-history
  - https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-autocomplete
  - https://luke-chang.github.io/autofill-demo/basic.html
  - https://www.macys.com/
  - https://checkout.shopify.com/1607500/checkouts/e9387ea64c906ec5c9e35a30d9ecd57f?cookies_blocked=1&no_cookies_from_redirect=1
  - https://www.mozilla.org/en-US/firefox/channel/desktop/#nightly
  - https://luke-chang.github.io/autofill-demo/basic.html
  - https://www.macys.com/
  - https://wiki.mozilla.org/Firefox/Features/Form_Autofill
  - https://blog.nightly.mozilla.org/2017/04/24/release-notes-for-nightly/
  - https://blog.nightly.mozilla.org/2016/10/11/found-a-regression-in-firefox-give-us-details-with-mozregression/
  - https://blog.nightly.mozilla.org/2016/07/13/crash-investigation-in-firefox-nightly/
  - https://bugzilla.mozilla.org/show_bug.cgi?id=1365998

This is not really a minimal STR, but it reproduced this crash twice on my laptop.

I have not isolated if the fact of having multiple tabs opened on the same page mattered or not at the moment, nor have I tried to reduce the set of website needed to trigger this issue.

Comment 1

[Michal Novotny [:michal]]

We get the files enumerator at the beginning of the update process. If some file is deleted after this point the enumerator might still return the file name. The solution here is to check if the file really exists when we get it from the enumerator.

Comment 2

[Michal Novotny [:michal]]

Attachment: fix

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8894967 [details] [diff] [review]
fix

Review of attachment 8894967 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/cache2/CacheIndex.cpp
@@ +3066,5 @@
> +    if (!fileExists) {
> +      LOG(("CacheIndex::UpdateIndex() - File returned by the iterator was "
> +           "removed in the meantime [name=%s]", leaf.get()));
> +      continue;
> +    }

any reason why this should not be above the file->GetNativeLeafName(leaf); line? (applies to BuildIndex() as well)

what happens if the file is deleted between the file->Exists(&fileExists); check and the assertion check?

I'm not sure I understand how this fixes the problem well enough to review it..

Comment 4

[Michal Novotny [:michal]]

(In reply to Honza Bambas (:mayhemer) from comment #3)
> Comment on attachment 8894967 [details] [diff] [review]
> fix
> 
> Review of attachment 8894967 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/cache2/CacheIndex.cpp
> @@ +3066,5 @@
> > +    if (!fileExists) {
> > +      LOG(("CacheIndex::UpdateIndex() - File returned by the iterator was "
> > +           "removed in the meantime [name=%s]", leaf.get()));
> > +      continue;
> > +    }
> 
> any reason why this should not be above the file->GetNativeLeafName(leaf);
> line? (applies to BuildIndex() as well)

Because the leaf name is logged.

> what happens if the file is deleted between the file->Exists(&fileExists);
> check and the assertion check?

It cannot be deleted by Firefox and it's fine if it's deleted by other process.

The problem here wasn't that some file was missing, but that the file existed when we were sure we deleted it. But in fact it didn't exist, it was just an old information from the enumerator.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95536a26dee7
Assertion failure: !entry || !entry->IsFresh() in CacheIndex::UpdateIndex, r=honzab

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/95536a26dee7