Closed
Bug 1372777
Opened 7 years ago
Closed 7 years ago
Crash in java.lang.NullPointerException: Null native pointer at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method), mainly about nsWindow::LayerViewSupport
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec+, firefox54 unaffected, firefox55 fixed, firefox56 fixed)
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
fennec | + | --- |
firefox54 | --- | unaffected |
firefox55 | --- | fixed |
firefox56 | --- | fixed |
People
(Reporter: ting, Assigned: rbarker)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
esawin
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
This bug was filed from the Socorro interface and is report bp-75449190-1f05-43a6-8348-d1cf60170613. ============================================================= Top #2 of Nightly 20170611100331 on Android, 17 reports from 15 installations. There are 359 reports in the past week.
Reporter | ||
Comment 1•7 years ago
|
||
The frame 0 is mainly: mozilla::jni::detail::ProxyNativeCall<nsWindow::LayerViewSupport, mozilla::java::LayerView::Compositor, false, false, const mozilla::jni::Ref<mozilla::jni::Object, _jobject*>&, const mozilla::jni::Ref<mozilla::jni::Object, _jobject*>&>::operator() and bp-c554f229-b9d6-406b-b283-657550170607 has: nsWindow::LayerViewSupport::SyncResumeResizeCompositor(mozilla::jni::LocalRef<mozilla::java::LayerView::Compositor> const&, int, int, mozilla::jni::Ref<mozilla::jni::Object, _jobject*> const&)::OnResumedEvent::Run() in frame 0.
Summary: Crash in java.lang.NullPointerException: Null native pointer at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method) → Crash in java.lang.NullPointerException: Null native pointer at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method), mainly about nsWindow::LayerViewSupport
Randall can you investigate?
Assignee: nobody → rbarker
tracking-fennec: --- → +
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
status-firefox56:
--- → affected
Flags: needinfo?(rbarker)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(rbarker)
Assignee | ||
Comment 4•7 years ago
|
||
It looks to me like a Java object that has a native pointer attached to it is being dispatched to a different thread. It would seem in the time it takes to dispatch to the other thread, the native pointer is being detached from the java object so a null pointer gets returned in the dispatch. I'll put some if checks to prevent this and see if that fixes the issue.
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8879272 [details] Bug 1372777 - Check that the native pointer has not been detached from the java object when dispatched to different thread https://reviewboard.mozilla.org/r/150562/#review155764
Attachment #8879272 -
Flags: review?(esawin) → review+
Pushed by rbarker@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8e1de9b86753 Check that the native pointer has not been detached from the java object when dispatched to different thread r=esawin
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8e1de9b86753
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8879272 [details] Bug 1372777 - Check that the native pointer has not been detached from the java object when dispatched to different thread https://reviewboard.mozilla.org/r/150562/#review157374 ::: widget/android/jni/Natives.h:407 (Diff revision 1) > template<bool Static, bool ThisArg, size_t... Indices> > typename mozilla::EnableIf<!Static && ThisArg, void>::Type > Call(const typename Owner::LocalRef& inst, > mozilla::IndexSequence<Indices...>) const > { > - Impl* const impl = NativePtr<Impl>::Get(inst); > + if (Impl* const impl = NativePtr<Impl>::Get(inst)) { This was designed to crash if we don't have a valid pointer here. Instead of ignoring null pointers, we should take care to free pointers after making sure they are no longer used.
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to (pto) Jim Chen [:jchen] [:darchons] from comment #9) > Comment on attachment 8879272 [details] > Bug 1372777 - Check that the native pointer has not been detached from the > java object when dispatched to different thread > > https://reviewboard.mozilla.org/r/150562/#review157374 > > ::: widget/android/jni/Natives.h:407 > (Diff revision 1) > > template<bool Static, bool ThisArg, size_t... Indices> > > typename mozilla::EnableIf<!Static && ThisArg, void>::Type > > Call(const typename Owner::LocalRef& inst, > > mozilla::IndexSequence<Indices...>) const > > { > > - Impl* const impl = NativePtr<Impl>::Get(inst); > > + if (Impl* const impl = NativePtr<Impl>::Get(inst)) { > > This was designed to crash if we don't have a valid pointer here. Instead of > ignoring null pointers, we should take care to free pointers after making > sure they are no longer used. I think the pointer was valid when it was dispatched. But between the dispatch to a different thread and actually being invoked it seems the pointer gets removed. I can't reproduce so I had to guess.
Comment 11•7 years ago
|
||
Please request Beta approval on this when you get a chance.
Flags: needinfo?(rbarker)
Assignee | ||
Comment 12•7 years ago
|
||
Jim, did you have any comments on this patch before I request uplift to beta?
Flags: needinfo?(nchen)
Comment 13•7 years ago
|
||
I think it's okay to uplift the current patch to beta. I'll probably work on this more for nightly.
Flags: needinfo?(nchen)
Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8879272 [details] Bug 1372777 - Check that the native pointer has not been detached from the java object when dispatched to different thread Approval Request Comment [Feature/Bug causing the regression]: Intermittent bug, unclear what caused it to start manifesting. [User impact if declined]: Possible random crashes when shutting down. [Is this code covered by automated tests?]: Almost all tests go through this code but there are no explicit tests. [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: just adds a nullptr check to three different places in code. [String changes made/needed]: none.
Flags: needinfo?(rbarker)
Attachment #8879272 -
Flags: approval-mozilla-beta?
Comment 15•7 years ago
|
||
Comment on attachment 8879272 [details] Bug 1372777 - Check that the native pointer has not been detached from the java object when dispatched to different thread fennec crash fix, beta55+
Attachment #8879272 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 16•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/e8e60131e908
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
•