Closed
Bug 790535
Opened 12 years ago
Closed 12 years ago
java.lang.NullPointerException: at org.mozilla.gecko.ScreenshotHandler$1.run(GeckoAppShell.java)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 18
People
(Reporter: scoobidiver, Assigned: kats)
Details
(Keywords: crash, Whiteboard: [native-crash])
Crash Data
Attachments
(1 file)
1.29 KB,
patch
|
cpeterson
:
review+
|
Details | Diff | Splinter Review |
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
Comment 1•12 years ago
|
||
kats, can LayerView.getRenderer() return null? https://hg.mozilla.org/mozilla-central/annotate/d260fcec71ce/mobile/android/base/GeckoAppShell.java#l2572
Assignee | ||
Comment 2•12 years ago
|
||
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.
Assignee | ||
Comment 3•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nobody → bugmail.mozilla
Comment 4•12 years ago
|
||
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+
Assignee | ||
Comment 5•12 years ago
|
||
(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?
Comment 6•12 years ago
|
||
The ScreenshotHandler notifyScreenShot() method has multiple synchronized blocks and (as this patch points out) is accessing LayerView and LayerRenderer from two threads without synchronization.
Assignee | ||
Comment 7•12 years ago
|
||
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•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/36d865c2f094
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•