Last Comment Bug 745243 - Startup crash in AndroidGLController::ProvideEGLSurface during LayerManagerOGL::CreateContext
: Startup crash in AndroidGLController::ProvideEGLSurface during LayerManagerOG...
Status: RESOLVED FIXED
[native-crash][gfx]
: crash, topcrash
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: -- critical (vote)
: Firefox 15
Assigned To: Ali Juma [:ajuma]
:
Mentors:
Depends on: 741222 741315
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-13 10:26 PDT by Scoobidiver (away)
Modified: 2012-06-08 09:36 PDT (History)
7 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
+


Attachments
Wait for surfaceChanged before calling GLController.provideEGLSurface. (3.39 KB, patch)
2012-05-18 07:59 PDT, Ali Juma [:ajuma]
bugmail.mozilla: review+
blassey.bugs: approval‑mozilla‑aurora+
Details | Diff | Review

Description Scoobidiver (away) 2012-04-13 10:26:47 PDT
This bug tracks residual crashes after the fix of bug 741166, 3 crashes so far.

There are two kinds of stack, the one in bug 741166 on startup and the following one:
Frame 	Module 	Signature 	Source
0 	libdvm.so 	libdvm.so@0x431ae 	
1 	libxul.so 	AndroidGLController::ProvideEGLSurface 	jni.h:706
2 	libxul.so 	mozilla::AndroidBridge::ProvideEGLSurface 	widget/android/AndroidBridge.cpp:1135
3 	libxul.so 	mozilla::gl::GLContextEGL::RenewSurface 	gfx/gl/GLContextProviderEGL.cpp:1522
4 	libxul.so 	mozilla::layers::CompositorParent::ResumeComposition 	gfx/layers/ipc/CompositorParent.cpp:146
5 	libxul.so 	RunnableMethod<mozilla::layers::CompositorParent, void , Tuple0>::Run 	ipc/chromium/src/base/tuple.h:383
6 	libxul.so 	MessageLoop::RunTask 	ipc/chromium/src/base/message_loop.cc:318
7 	libxul.so 	MessageLoop::DeferOrRunPendingTask 	ipc/chromium/src/base/message_loop.cc:326
8 	libxul.so 	MessageLoop::DoWork 	ipc/chromium/src/base/message_loop.cc:426
9 	libxul.so 	base::MessagePumpDefault::Run 	ipc/chromium/src/base/message_pump_default.cc:23
10 	libxul.so 	MessageLoop::RunInternal 	ipc/chromium/src/base/message_loop.cc:208
11 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:201
12 	libxul.so 	base::Thread::ThreadMain 	ipc/chromium/src/base/thread.cc:156
13 	libxul.so 	ThreadFunc 	ipc/chromium/src/base/platform_thread_posix.cc:26
14 	libc.so 	libc.so@0x11e32 	
15 	libc.so 	libc.so@0x11a06 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=AndroidGLController%3A%3AProvideEGLSurface
Comment 1 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-04-16 11:14:43 PDT
top crash in 14a1 in 3 day period. (4/16/2012)
Comment 2 Scoobidiver (away) 2012-04-17 00:01:39 PDT
There are 3 crashes with the AndroidGLController::ProvideEGLSurface signature and 5 crashes with the JNI_GetCreatedJavaVMs signature.
Comment 3 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-04-18 11:59:00 PDT
received this crash on the nexus s when going to dartlang.org and then typing something in the editable content using 4/18/2012
Comment 4 Erin Lancaster [:elan] 2012-04-26 20:01:58 PDT
Just got this crash on an HTC phone; signing into Google Reader [http://www.google.com/reader/i] navigate to BBC article and play back flash content. Not able to repro but will keep trying.
Comment 5 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-05-10 10:06:39 PDT
Possible regression or similar from bug 741166?  Removing topcrash as it's no longer in the topcrash listings.
Comment 6 Scoobidiver (away) 2012-05-11 22:50:41 PDT
(In reply to Naoki Hirata :nhirata from comment #5)
> Removing topcrash as it's no longer in the topcrash listings.
I have to disagree. With combined signatures, there are 60 crashes in 14.0a2 over the last week making it #13 top crasher in 14.0a2.
Comment 7 Scoobidiver (away) 2012-05-15 04:54:16 PDT
In Aurora, with combined signatures, it's #4 top crasher over the last day, #10 over the last 3 days, #16 over the last week.
Comment 8 Ali Juma [:ajuma] 2012-05-18 07:59:51 PDT
Created attachment 625106 [details] [diff] [review]
Wait for surfaceChanged before calling GLController.provideEGLSurface.

There are really two groups of crashes here: those occurring at startup (during LayerManagerOGL::CreateContext) and those occurring later (during CompositorParent::ResumeComposition).

It's conceivable that the startup crashes are caused by us calling GLController.provideEGLSurface before receiving surfaceChanged. Right now, we only wait for surfaceCreated, and hence the surface might still be zero-sized at this point. This patch makes us wait for surfaceChanged instead.
Comment 9 Ali Juma [:ajuma] 2012-05-18 13:04:25 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/47c8f2d06763

The bug should be kept open when this merges to m-c, since it likely will not fix the non-startup crashes.
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-05-18 18:42:29 PDT
https://hg.mozilla.org/mozilla-central/rev/47c8f2d06763
Comment 11 Ali Juma [:ajuma] 2012-05-22 06:59:10 PDT
Comment on attachment 625106 [details] [diff] [review]
Wait for surfaceChanged before calling GLController.provideEGLSurface.

[Approval Request Comment]
User impact if declined: Possible startup crashes.
Testing completed (on m-c, etc.): On m-c since May 18.
Risk to taking this patch (and alternatives if risky): Low risk.
String or UUID changes made by this patch: None.
Comment 12 Ali Juma [:ajuma] 2012-05-22 07:00:15 PDT
(In reply to Ali Juma [:ajuma] from comment #11)
> Comment on attachment 625106 [details] [diff] [review]
> Risk to taking this patch (and alternatives if risky): Low risk.

Forgot to add that this code is mobile-only.
Comment 14 Ali Juma [:ajuma] 2012-05-28 07:07:57 PDT
(In reply to Ali Juma [:ajuma] from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/47c8f2d06763
> 
> The bug should be kept open when this merges to m-c, since it likely will
> not fix the non-startup crashes.

Indeed, looking at crash reports on 14.0b3, the startup crashes (during LayerManagerOGL::CreateContext) are virtually all gone (there is only one such crash so far), but the non-startup crashes remain.
Comment 15 Joe Drew (not getting mail) 2012-05-28 11:43:18 PDT
Ali, for easier tracking, can you open a new bug for the second half of this bug, and mark this one as fixed?
Comment 16 Ali Juma [:ajuma] 2012-05-28 12:04:51 PDT
(In reply to Joe Drew (:JOEDREW!) from comment #15)
> Ali, for easier tracking, can you open a new bug for the second half of this
> bug, and mark this one as fixed?

Filed bug 759162 for the remaining crashes.
Comment 17 Kevin Brosnan [:kbrosnan] 2012-05-30 19:30:42 PDT
This cut the crash rate on Beta in half for this signature. With there being a follow up bug for the other crashers I'm willing to call this verified.
Comment 19 Ali Juma [:ajuma] 2012-06-08 06:48:46 PDT
Re-nomming since the crash volume is low.

There was only a single crash with this signature during LayerManagerOGL::CreateContext in 14.0b5. The fact that we already have 3 crashes in 14.0b6 is a bit concerning (it's too early to say whether this is a real increase or just noise), but the volume is still low.
Comment 21 Ali Juma [:ajuma] 2012-06-08 07:22:27 PDT
(In reply to Scoobidiver from comment #20)
> (In reply to Ali Juma [:ajuma] from comment #19)
> > There was only a single crash with this signature during
> > LayerManagerOGL::CreateContext in 14.0b5.
> There are more than that (without counting dalvik-LinearAlloc (deleted)
> signatures).

Ah, indeed, I missed those. Nevertheless, the crash volume still seems too low for this to be a release blocker.
Comment 22 Joe Drew (not getting mail) 2012-06-08 08:37:37 PDT
We should probably open a new bug if startup crashes remain, since bug 759162 tracks the other crashes. I'm going to leave this bug closed for now; makes tracking easier.

Note You need to log in before you can comment on or make changes to this bug.