Closed Bug 1369051 Opened 4 years ago Closed 3 years ago

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

Categories

(Core :: Networking: Cache, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: nbp, Assigned: michal)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file)

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.
Flags: needinfo?(michal.novotny)
Assignee: nobody → michal.novotny
Flags: needinfo?(michal.novotny)
Whiteboard: [necko-active]
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.
Attached patch fixSplinter Review
Attachment #8894967 - Flags: review?(honzab.moz)
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..
(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.
Attachment #8894967 - Flags: review?(honzab.moz) → review+
Pushed by mnovotny@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95536a26dee7
Assertion failure: !entry || !entry->IsFresh() in CacheIndex::UpdateIndex, r=honzab
https://hg.mozilla.org/mozilla-central/rev/95536a26dee7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.