Attempting to use JNI in child process [@ mozilla::AndroidBridge::EnsureJNIThread]

RESOLVED FIXED

Status

()

Core
Widget: Android
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jdm, Unassigned)

Tracking

({crash, topcrash})

Trunk
x86
Mac OS X
crash, topcrash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
This bug was filed from the Socorro interface and is 
report bp-dab1a704-3c13-4109-8c6c-349c12111106 .
============================================================= 

Here's some craziness: we're using the JNI to retrieve driver information for GfxInfo, but that's called from the content process.

0 	libxul.so 	mozilla::AndroidBridge::EnsureJNIThread 	widget/src/android/AndroidBridge.cpp:239
1 	libxul.so 	mozilla::AndroidBridge::AutoLocalJNIFrame::AutoLocalJNIFrame 	widget/src/android/AndroidBridge.h:105
2 	libxul.so 	mozilla::AndroidBridge::GetStaticStringField 	widget/src/android/AndroidBridge.cpp:856
3 	libxul.so 	mozilla::widget::GfxInfo::GetAdapterVendorID 	widget/src/android/GfxInfo.cpp:192
4 	libxul.so 	mozilla::widget::GfxInfoBase::GetFeatureStatusImpl 	widget/src/xpwidgets/GfxInfoBase.cpp:597
5 	libxul.so 	mozilla::widget::GfxInfo::GetFeatureStatusImpl 	widget/src/android/GfxInfo.cpp:321
6 	libxul.so 	mozilla::widget::GfxInfoBase::GetFeatureStatus 	widget/src/xpwidgets/GfxInfoBase.cpp:574
7 	libxul.so 	mozilla::WebGLContext::SetDimensions 	content/canvas/src/WebGLContext.cpp:616
8 	libxul.so 	nsHTMLCanvasElement::UpdateContext 	content/html/content/src/nsHTMLCanvasElement.cpp:622
9 	libxul.so 	nsHTMLCanvasElement::GetContext 	content/html/content/src/nsHTMLCanvasElement.cpp:540
10 	libxul.so 	nsIDOMHTMLCanvasElement_GetContext 	obj-firefox/js/xpconnect/src/dom_quickstubs.cpp:18716
11 	libxul.so 	js::InvokeKernel 	js/src/jscntxtinlines.h:297
12 	libxul.so 	js::Interpret 	js/src/jsinterp.cpp:3948
13 	libxul.so 	js::RunScript 	js/src/jsinterp.cpp:584
14 	libxul.so 	js::Execute 	js/src/jsinterp.cpp:783
15 	libxul.so 	JS_EvaluateUCScriptForPrincipalsVersion 	js/src/jsapi.cpp:5038
16 	libxul.so 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1490
17 	libxul.so 	nsScriptLoader::EvaluateScript 	content/base/src/nsScriptLoader.cpp:905
18 	libxul.so 	nsScriptLoader::ProcessRequest 	content/base/src/nsScriptLoader.cpp:799
19 	libxul.so 	nsScriptLoader::ProcessPendingRequests 	content/base/src/nsScriptLoader.cpp:948
20 	libxul.so 	nsScriptLoader::OnStreamComplete 	content/base/src/nsScriptLoader.cpp:1182
21 	libxul.so 	nsStreamLoader::OnStopRequest 	netwerk/base/src/nsStreamLoader.cpp:125
22 	libxul.so 	nsHTTPCompressConv::OnStopRequest 	netwerk/streamconv/converters/nsHTTPCompressConv.cpp:127
23 	libxul.so 	mozilla::net::HttpChannelChild::OnStopRequest 	netwerk/protocol/http/HttpChannelChild.cpp:484
(Reporter)

Updated

6 years ago
Component: General → Widget: Android
Keywords: topcrash
Product: Fennec → Core
QA Contact: general → android
(Reporter)

Comment 1

6 years ago
This is killing Fennec nightly on every use of a canvas element. We should either back out the Android changes or properly serialize the relevant information in the parent and send it on process creation.
Blocks: 668004
(Reporter)

Comment 2

6 years ago
And when I say canvas, I really mean webgl canvas. Still, this is a topcrash.
Created attachment 572313 [details] [diff] [review]
avoid calling GfxInfoBase::GetFeatureStatusImpl for now on Android

this should remove this crash, according to the call stack.

So... if we can't use this interface to query hardware information in the content process on Android, what do you think we should do to get that infomation? Query it from the chrome process and pipe it to the content process?
Attachment #572313 - Flags: review?(josh)
(Reporter)

Updated

6 years ago
Attachment #572313 - Flags: review?(josh) → review+
(Reporter)

Comment 4

6 years ago
I think the best plan would be to query and send the information in the chrome process inside ContentParent::ContentParent, and keep it all under an #ifdef ANDROID.
http://hg.mozilla.org/mozilla-central/rev/161c6106d787

Reopen if crashes persist.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [mobile-crash]
Duplicate of this bug: 700322
I'm unable to repro this when I backout the patch. jdm: what hardware/setup are you using, and is there any other info you can give me to repro it? I'd also appreciate a more in-depth description as to how you think this should be dealt with properly.
(Reporter)

Comment 8

6 years ago
I merely saw this topcrash on crash-stats, I didn't try reproducing it. I'm not certain it's worth solving properly, given that desktop e10s is on hold and mobile e10s is going away imminently on nightlies.
This is a problem that still has to be dealt with. It was caused by some additions I made to support blocklisting Android devices from graphics features (OpenGL Layers, WebGL, etc). Benoit wrote up a temporary fix for it, but I need to reactivate this code properly. Any info you can provide would be appreciated.
(Reporter)

Comment 10

6 years ago
It looks like any content process which triggers a SetDimensions call on a webgl canvas should cause the problem. I outlined a plan to solve this properly in comment 4 - we send other bits of information to child processes already, so this would just be querying amd sending one more thing, then storing that data somewhere so that we don't need to hit the JNI for the feature status later.
Ok, I think I completely missed the e10s being phased out part. It sounds like based on that, there's no reason not to do these queries on a single process once this is done, i.e. the way we were already doing them minus the mistakes in handling SDK versions detailed in bug 700931.
(Reporter)

Comment 12

6 years ago
Actually, it might remain necessary to solve this properly for content processes if B2G is planning to be using them, which I believe they are.
(Reporter)

Comment 13

6 years ago
mwu tells me that Gonk doesn't care about this, so we can probably leave it be.
Ok, thanks for checking into that.
We should definitely fix this the "right way", if it's not incredibly difficult to solve generally.  Rumors of the death of multi-process Gecko are greatly exaggerated, even if particular products (desktop Firefox) aren't planning on using it in the near future.
Ok, I will start looking into solving it correctly then. If anyone has any repro steps or additional information on it, please provide it as right now I can't repro it.
Blocks: 706702
Occurs in XUL; reopening based on comment 5.

Using the Nightly XUL 2012-01-24, created a new tab and tried to go to the address while another tab was already trying to load www.cnn.com 

bp-611b0a7e-96fd-4dcd-8575-5ff982120125

Thunderbolt, 2.3
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 18

6 years ago
Comment 17 is a different crash, tracked in bug 720400.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.