If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

crash in java.lang.NullPointerException: at org.mozilla.gecko.favicons.cache.FaviconCache.cullIfRequired(FaviconCache.java)

RESOLVED FIXED in Firefox 27

Status

()

Firefox for Android
General
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aaronmt, Assigned: ckitching)

Tracking

({crash})

27 Branch
Firefox 27
All
Android
crash
Points:
---

Firefox Tracking Flags

(firefox26 unaffected, firefox27+ fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
This bug was filed from the Socorro interface and is 
report bp-cc9f957d-eeeb-4fe4-963d-db82c2131021.
=============================================================

java.lang.NullPointerException
	at org.mozilla.gecko.favicons.cache.FaviconCache.cullIfRequired(FaviconCache.java:617)
	at org.mozilla.gecko.favicons.cache.FaviconCache.putSingleFavicon(FaviconCache.java:537)
	at org.mozilla.gecko.favicons.Favicons.putFaviconInMemCache(Favicons.java:250)
	at org.mozilla.gecko.favicons.LoadFaviconTask.onPostExecute(LoadFaviconTask.java:320)

* Samsung Galaxy Nexus	2
* LGE Nexus 4
(Reporter)

Updated

4 years ago
tracking-firefox27: --- → ?
(Assignee)

Comment 1

4 years ago
Created attachment 820191 [details] [diff] [review]
Reset cache fullness counter when evictAll is called.

This appears to be caused by the favicon cache trying to remove elements to reclaim space when there are no elements to remove. (ie. mOrdering is empty).

It also appears that a call to evictAll doesn't lead to mCurrentSize being reset - this would lead to the next cull when full attempting to remove nonexistent elements to free up space that is already freed, resulting in this crash.

This crash might also be caused by another, undetected problem causesing mCurrentSize to become inconsistent with the actual cache contents.

Still, the evictAll problem seems like a likely candidate for causing this problem. Have a patch.
Assignee: nobody → chriskitching
Status: NEW → ASSIGNED
Attachment #820191 - Flags: review?(rnewman)
Comment on attachment 820191 [details] [diff] [review]
Reset cache fullness counter when evictAll is called.

Do we not need to take a lock here? (Reviewing from phone, else I'd pull up the source.)
(Assignee)

Comment 3

4 years ago
(In reply to Richard Newman [:rnewman] from comment #2)
> Comment on attachment 820191 [details] [diff] [review]
> Reset cache fullness counter when evictAll is called.
> 
> Do we not need to take a lock here? (Reviewing from phone, else I'd pull up
> the source.)

Yes - the appropriate lock was already taken in earlier code. The patch adds.an extra line of work while the lock is held.

Updated

4 years ago
status-firefox26: --- → unaffected
tracking-firefox27: ? → +
Comment on attachment 820191 [details] [diff] [review]
Reset cache fullness counter when evictAll is called.

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

Well, at least we know our eviction code is being called!
Attachment #820191 - Flags: review?(rnewman) → review+
Keywords: steps-wanted → checkin-needed
(Assignee)

Comment 5

4 years ago
(In reply to Richard Newman [:rnewman] from comment #4)
> Well, at least we know our eviction code is being called!

That, or my hypothesis is incorrect. :P

Comment 6

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/59608352c11e
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/59608352c11e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox27: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.