Closed Bug 1499415 Opened Last year Closed 11 months ago

Crash in java.lang.NullPointerException: at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java)

Categories

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

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 65
Tracking Status
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: marcia, Assigned: petru)

Details

(Keywords: crash, regression, Whiteboard: [priority:low])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-b37bc271-142a-43b9-b5af-9e5240181016.
=============================================================

This signature has been seen before, most recently in Bug 869827: https://bit.ly/2pXoTzY. Overall this is the #10 crash in 62.0.3. Filing because it needs a new bug to see if we can do anything about the Null Pointer Exception being called below. Only 1 crash in 64 in the last 7 days, and visible in 63 beta but in low volume.

java.lang.NullPointerException: Attempt to invoke virtual method 'boolean org.mozilla.gecko.extensions.ExtensionPermissionsHelper.getShowUpdateIcon()' on a null object reference
	at org.mozilla.gecko.BrowserApp.onPrepareOptionsMenu(BrowserApp.java:3904)
	at org.mozilla.gecko.GeckoApp.invalidateOptionsMenu(GeckoApp.java:421)
	at org.mozilla.gecko.BrowserApp.refreshChrome(BrowserApp.java:1884)
	at org.mozilla.gecko.GeckoApp.onLocaleReady(GeckoApp.java:1330)
	at org.mozilla.gecko.BrowserApp.onLocaleReady(BrowserApp.java:2975)
	at org.mozilla.gecko.GeckoApp$10$1.run(GeckoApp.java:1262)
	at android.os.Handler.handleCallback(Handler.java:815)
	at android.os.Handler.dispatchMessage(Handler.java:104)
	at android.os.Looper.loop(Looper.java:207)
	at android.app.ActivityThread.main(ActivityThread.java:5728)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:888)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:749)
Susheel, do you have someone who can take a look at this crash? Thanks.
Flags: needinfo?(sdaswani)
Looks pretty low volume, so will put on our low priority list.
Flags: needinfo?(sdaswani)
Whiteboard: [priority:low]
It's the #10 topcrash on release, but doesn't show up in high volume on other channels. 
Since 63 release is next week, marking as wontfix.
Priority: -- → P1
Assignee: nobody → petru.lingurar
Status: NEW → ASSIGNED
`mExtensionPermissionsHelper` is initialized in BrowserApp's onCreate() and
nullified in it's onDestroy().
GeckoApp's onCreate() can finish after BrowserApp's onDestroy() because it uses
background initialization.
`mExtensionPermissionsHelper` will be checked for null before using it and
if null it's safe to ignore it's method's results as it only can be null if the
app was already closed.
Looking through the code looks like this might happen when the app is opened and closed immediately after.
GeckoApp's onCreate() uses background threads to complete it's initialization and from one such thread this crash originates [1].

When the app is closed BroserApp.java nullifies that ExtensionPermissionHelper instance[2].
But the background initialization of GeckoApp might still be running and eventually [3] a call to onPrepareOptionsMenu in BrowserApp will be made which will try to use that already nullified ExtensionPermissionHelper instance.

Because `mExtensionPermissionsHelper` is only nullified in BrowserApp's onDestroy() the safe and simple solution would be to just add a null check before trying to execute something on it.


[1] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1211
[2] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#1491
[3] https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1264
Keywords: checkin-needed
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d2294c830e11
Check mExtensionPermissionsHelper for null before using it; r=sdaswani
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d2294c830e11
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Please request Beta approval on this when you get a chance.
Flags: needinfo?(petru.lingurar)
Comment on attachment 9022623 [details]
Bug 1499415 - Check mExtensionPermissionsHelper for null before using it; r?sdaswani

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: App crash

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Simple null check added

String changes made/needed:
Flags: needinfo?(petru.lingurar)
Attachment #9022623 - Flags: approval-mozilla-beta?
Comment on attachment 9022623 [details]
Bug 1499415 - Check mExtensionPermissionsHelper for null before using it; r?sdaswani

fennec crash fix, approved for 64.0b9
Attachment #9022623 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.