java.lang.NullPointerException: at org.mozilla.gecko.ScreenshotHandler$1.run(GeckoAppShell.java)

RESOLVED FIXED in Firefox 18

Status

()

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

People

(Reporter: Scoobidiver (away), Assigned: kats)

Tracking

({crash})

Trunk
Firefox 18
ARM
Android
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [native-crash], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
There's one crash in 18.0a1/20120911140351: bp-add6ebf5-c7c8-402c-b408-38ee12120911.

java.lang.NullPointerException
	at org.mozilla.gecko.ScreenshotHandler$1.run(GeckoAppShell.java:2572)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:130)
	at org.mozilla.gecko.util.GeckoBackgroundThread.run(GeckoBackgroundThread.java:31)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.ScreenshotHandler%241.run%28GeckoAppShell.java%29
kats, can LayerView.getRenderer() return null?

https://hg.mozilla.org/mozilla-central/annotate/d260fcec71ce/mobile/android/base/GeckoAppShell.java#l2572
Looks like it can, but there's a very narrow window in which it can happen. I'll attach a patch that should close that window.
Created attachment 660530 [details] [diff] [review]
Patch

This just makes sure that by the time mLayerView becomes mLayerView, it is fully initialized and ready for action. (i.e. there should be point where mLayerView.getRenderer() returns null now).
Attachment #660530 - Flags: review?(cpeterson)
Assignee: nobody → bugmail.mozilla
Comment on attachment 660530 [details] [diff] [review]
Patch

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

LGTM, though the thread-safety of the ScreenshotHandler run() method itself looks pretty fragile.
Attachment #660530 - Flags: review?(cpeterson) → review+
(In reply to Chris Peterson (:cpeterson) from comment #4)
> LGTM, though the thread-safety of the ScreenshotHandler run() method itself
> looks pretty fragile.

In what way?
The ScreenshotHandler notifyScreenShot() method has multiple synchronized blocks and (as this patch points out) is accessing LayerView and LayerRenderer from two threads without synchronization.
Hmm it looks ok to me (but I'm biased since I wrote it). I'll land this patch for now and if I have reason to touch that code again I'll consider improving it somehow.

https://hg.mozilla.org/integration/mozilla-inbound/rev/36d865c2f094

Comment 8

6 years ago
https://hg.mozilla.org/mozilla-central/rev/36d865c2f094
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
You need to log in before you can comment on or make changes to this bug.