Closed Bug 1100403 Opened 5 years ago Closed 4 years ago

crash in java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)

Categories

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

36 Branch
All
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox35 --- affected
firefox36 --- affected
firefox44 --- fixed
firefox45 --- fixed
firefox46 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: aaronmt, Assigned: ahunt)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-ad72a320-ac58-4032-8fe5-a41412141116.
=============================================================

ava.lang.NullPointerException
	at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java:67)
	at org.mozilla.gecko.home.BrowserSearch$4.run(BrowserSearch.java:355)
	at android.os.Handler.handleCallback(Handler.java:730)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:5175)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:525)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
	at dalvik.system.NativeStart.main(Native Method)
This might have been fixed with Bug 1186683 as well.
Crash Signature: [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)] → [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)] [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500]
Crash Signature: [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)] [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500] → [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)] [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500] java.lang.NullPointerException: at org.mozilla.gecko.home.…
Crash Signature: [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)] [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500] java.lang.NullPointerException: at org.mozilla.gecko.home.… → [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500(BrowserSearch.java)] [@ java.lang.NullPointerException: at org.mozilla.gecko.home.BrowserSearch.access$500] [@ java.lang.NullPointerException: at org.mozilla.gecko.hom…
Crash Signature: org.mozilla.gecko.home.BrowserSearch.access$1900(BrowserSearch.java)] → org.mozilla.gecko.home.BrowserSearch.access$1900(BrowserSearch.java)] [@ java.lang.NullPointerException: Attempt to invoke virtual method ''boolean java.lang.String.startsWith(java.lang.String)'' on a null object reference at org.mozilla.gecko.home.Brow…
This crashed 12 times in 28 days – it's probably not worth looking deeply into this bug over the other top crashers we have.
Also, crashes occurred in 44, where bug 1186683 is landed, so I don't think it fixed this bug.
(In reply to Michael Comella (:mcomella) from comment #2)
> This crashed 12 times in 28 days – it's probably not worth looking deeply
> into this bug over the other top crashers we have.

I saw multiple similar signatures related to BrowserSearch.access. With these reports combined, this is crashing a lot more often than 12 times in the last 28 days.
(In reply to Michael Comella (:mcomella) from comment #3)
> Also, crashes occurred in 44, where bug 1186683 is landed, so I don't think
> it fixed this bug.

I wonder if bug 1186683 even made this crash occur more often, since most of these new stacks appear to happen only in 44.
Looking into this a bit, I wonder if mAdapter is null here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#1127

It only gets set in onActivityCreated:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#388

And if we're destroying/creating the fragment more often, I suspect we can get into race issues with loaders.
Yeah, two loaders racing -- one will reset mAdapter to null. Shared mutable state is the enemy :/
liuche or ahunt, can one of you write a patch here?
Flags: needinfo?(liuche)
Flags: needinfo?(ahunt)
(In reply to :Margaret Leibovic from comment #8)
> liuche or ahunt, can one of you write a patch here?

Looking into it right now!
Assignee: nobody → ahunt
Flags: needinfo?(liuche)
Flags: needinfo?(ahunt)
I've got one patch which would prevent us from using the adapter if null, however the last trace that was linked seems to be a separate issue ("java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.lang.String.startsWith(java.lang.String)' on a null object reference") - I wonder it that's in:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#546

or

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#564

Trace:
https://crash-stats.mozilla.com/report/index/aa12bdb8-1dca-41a2-a73b-626fd2160115
Comment on attachment 8708465 [details]
MozReview Request: Bug 1100403 - Don't read mAdapter if null r?margaret

https://reviewboard.mozilla.org/r/31063/#review27879

Thanks for the quick fix. Let's also request uplift for this one and see if we can get it into the next beta.
Attachment #8708465 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8708465 [details]
MozReview Request: Bug 1100403 - Don't read mAdapter if null r?margaret

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: 30th top crasher in current beta (44.0b8)
[Describe test coverage new/current, TreeHerder]: no tests - not reproducable locally.
[Risks and why]: low-risk: add null check where object was previously accessed directly.
[String/UUID change made/needed]: None.

(Separate patch required for beta due to path changes)
Attachment #8708465 - Flags: approval-mozilla-aurora?
Separate patch for beta due to path changes:

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: 30th top crasher in current beta (44.0b8)
[Describe test coverage new/current, TreeHerder]: no tests - not reproducable locally.
[Risks and why]: low-risk: add null check where object was previously accessed directly.
[String/UUID change made/needed]: None.

(Separate patch required for beta due to path changes)
Attachment #8708500 - Flags: approval-mozilla-beta?
I'm in favor of a band-aid to avoid a NPE, but this is the symptom of a more significant lifecycle problem. The loader should capture its adapter, or otherwise untie this knot.

Please file a follow-up to do this right -- more 'final', please!


(In reply to Andrzej Hunt :ahunt from comment #10)

> however the last trace that was linked seems to be a separate issue
> ("java.lang.NullPointerException: Attempt to invoke virtual method 'boolean
> java.lang.String.startsWith(java.lang.String)' on a null object reference")

This kinda doesn't make sense. The DB URL must be non-null, we null-check host, stripCommonSubdomains doesn't return null, so perhaps this is inlined code?
https://hg.mozilla.org/mozilla-central/rev/80e55e78a44c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment on attachment 8708500 [details] [diff] [review]
1100403_backport_beta.patch

Fennec crash fix, makes sense to uplift to Beta44 as Fennec stability remains a big concern for this cycle.
Attachment #8708500 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8708465 [details]
MozReview Request: Bug 1100403 - Don't read mAdapter if null r?margaret

Just like with beta.
Attachment #8708465 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Will verify this after checking in crash-stats that this crash signature will not be present.
(In reply to Richard Newman [:rnewman] from comment #16)
> I'm in favor of a band-aid to avoid a NPE, but this is the symptom of a more
> significant lifecycle problem. The loader should capture its adapter, or
> otherwise untie this knot.
> 
> Please file a follow-up to do this right -- more 'final', please!

Done at bug 1240797.

> 
> 
> (In reply to Andrzej Hunt :ahunt from comment #10)
> 
> > however the last trace that was linked seems to be a separate issue
> > ("java.lang.NullPointerException: Attempt to invoke virtual method 'boolean
> > java.lang.String.startsWith(java.lang.String)' on a null object reference")
> 
> This kinda doesn't make sense. The DB URL must be non-null, we null-check
> host, stripCommonSubdomains doesn't return null, so perhaps this is inlined
> code?

I really have no idea what's happening here - however it doesn't seem like a common cause of crashing, so probably not worth prioritising right now?
It looks like this crash is still happening on 44:
https://crash-stats.mozilla.com/report/index/01f0d5cf-1011-48c7-966e-001272160127

Either that, or it only solved some of the signatures here. Bug 1232608 has a similar signature, we could use that bug to investigate the remaining problems in BrowserSearch.
You need to log in before you can comment on or make changes to this bug.