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

RESOLVED FIXED in Firefox 57

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nbp, Assigned: michal)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment)

Reporter

Description

2 years ago
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

Updated

2 years ago
Assignee: nobody → michal.novotny
Flags: needinfo?(michal.novotny)
Assignee

Updated

2 years ago
Whiteboard: [necko-active]
Assignee

Comment 1

2 years ago
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.
Assignee

Comment 2

2 years ago
Posted 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..
Assignee

Comment 4

2 years ago
(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+

Comment 5

2 years ago
Pushed by mnovotny@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95536a26dee7
Assertion failure: !entry || !entry->IsFresh() in CacheIndex::UpdateIndex, r=honzab

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/95536a26dee7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.