750195 - java.lang.NullPointerException: at org.mozilla.gecko.GeckoApp.onDestroy(GeckoApp.java)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

There are two crashes in 15.0a1/20120429: bp-bf627e84-e034-4916-b219-d95ca2120430 and bp-c368944b-2c22-4cfa-a394-866192120429.

java.lang.NullPointerException
	at org.mozilla.gecko.GeckoApp.onDestroy(GeckoApp.java:2113)
	at android.app.Activity.performDestroy(Activity.java:4629)
	at android.app.Instrumentation.callActivityOnDestroy(Instrumentation.java:1079)
	at android.app.ActivityThread.performDestroyActivity(ActivityThread.java:3099)
	at android.app.ActivityThread.handleDestroyActivity(ActivityThread.java:3130)
	at android.app.ActivityThread.access$1200(ActivityThread.java:123)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1180)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4424)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:784)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:551)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.GeckoApp.onDestroy%28GeckoApp.java%29

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

Looking at the code, I see mFavicons and mBatteryReceiver used without null checks. Both are created in initialize() and could race with onDestroy in some cases. Other items created in initialize() do have null checks in various places, so this patch seems prudent.

I don't see a way around the checks. Thoughts?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Adding Kats and Brad for more input.

Comment 3

[Brian Nicholson (:bnicholson)]

Comment on attachment 619565 [details] [diff] [review]
patch

This looks correct to me.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Adding Kats and Brad for more input.

I don't understand why they would race. Shouldn't both of those functions be running on the Android UI thread?

Comment 5

[Brian Nicholson (:bnicholson)]

(In reply to Kartikaya Gupta (:kats) from comment #4)
> I don't understand why they would race. Shouldn't both of those functions be
> running on the Android UI thread?

By "race", I think he just means that onDestroy() incorrectly assumes that initialize() is always called beforehand, and not that there's two competing, simultaneously executing threads.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

(In reply to Brian Nicholson (:bnicholson) from comment #5)
> By "race", I think he just means that onDestroy() incorrectly assumes that
> initialize() is always called beforehand, and not that there's two
> competing, simultaneously executing threads.

Ah, that makes sense. Carry on then :)

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc56a41ebfb8

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

noming. potentially fixes the crash

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 619565 [details] [diff] [review]
patch

[Triage Comment] drivers approved

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ffc8cf166c6

Comment 11

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/bc56a41ebfb8

Comment 12

[Cristian Nicolae (:xti)]

This crash doesn't occur anymore on the recent Nightly and Aurora builds. Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-29)
Firefox 14.0a2 (2012-05-29)

Device: Galaxy Nexus
OS: Android 4.0.2