Closed Bug 929025 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox for Android :: General, defect, critical)

27 Branch
All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 27
Tracking Status
firefox26 --- unaffected
firefox27 + fixed

People

(Reporter: aaronmt, Assigned: ckitching)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

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
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.)
(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.
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+
(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
https://hg.mozilla.org/mozilla-central/rev/59608352c11e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.