860446 - WebGL is broken on B2G after layers refactor

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Sotaro Ikeda [:sotaro]]

After gfx layer refactoring, CubeVid app start makes system reboot on gonk.
I confirmed on unagi by using m-i source code.

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 531.553]
0x41d99e80 in ~ImageLayerComposite (this=0x49504400, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ImageLayerComposite.cpp:36
36        MOZ_ASSERT(mDestroyed);
(gdb) bt
#0  0x41d99e80 in ~ImageLayerComposite (this=0x49504400, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ImageLayerComposite.cpp:36
#1  0x41d99f1a in ~ImageLayerComposite (this=0x49504400, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ImageLayerComposite.cpp:39
#2  0x40e96dea in mozilla::layers::Layer::Release (this=0x49504490) at ../../dist/include/Layers.h:588
#3  0x41d98cec in mozilla::layers::ContainerLayerComposite::RemoveChild (this=0x47eae000, aChild=0x49504490)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:246
#4  0x41d98938 in ~ContainerLayerComposite (this=0x47eae000, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:176
#5  0x41d989be in ~ContainerLayerComposite (this=0x47eae000, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:178
#6  0x40e96dea in mozilla::layers::Layer::Release (this=0x47eae090) at ../../dist/include/Layers.h:588
#7  0x41d98cec in mozilla::layers::ContainerLayerComposite::RemoveChild (this=0x493ccc00, aChild=0x47eae090)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:246
#8  0x41d98938 in ~ContainerLayerComposite (this=0x493ccc00, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:176
#9  0x41d989be in ~ContainerLayerComposite (this=0x493ccc00, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:178
#10 0x40e96dea in mozilla::layers::Layer::Release (this=0x493ccc90) at ../../dist/include/Layers.h:588
#11 0x41d6727a in ~nsRefPtr (this=0x47ebd0e0, __in_chrg=<value optimized out>) at ../../dist/include/nsAutoPtr.h:880
#12 0x41dc9df8 in ~ShadowLayersParent (this=0x47ebd0b0, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/ipc/ShadowLayersParent.cpp:150
#13 0x41dc9e56 in ~ShadowLayersParent (this=0x47ebd0b0, __in_chrg=<value optimized out>)
    at /hack/mozilla-graphics/gfx/layers/ipc/ShadowLayersParent.cpp:150
#14 0x41db5b60 in mozilla::layers::CrossProcessCompositorParent::DeallocPLayers (this=0x487bb0e0, aLayers=0x47ebd0b0)
    at /hack/mozilla-graphics/gfx/layers/ipc/CompositorParent.cpp:1430
#15 0x41990f5e in mozilla::layers::PCompositorParent::DeallocSubtree (this=0x487bb0e0)
    at /hack/b2g/B2G/objdir-gecko/ipc/ipdl/PCompositorParent.cpp:831
#16 0x41991162 in mozilla::layers::PCompositorParent::OnChannelError (this=0x487bb0e0)
    at /hack/b2g/B2G/objdir-gecko/ipc/ipdl/PCompositorParent.cpp:664
#17 0x41926074 in mozilla::ipc::AsyncChannel::NotifyMaybeChannelError (this=0x487bb0e8) at /hack/mozilla-graphics/ipc/glue/AsyncChannel.cpp:570
#18 0x4192734c in mozilla::ipc::AsyncChannel::OnNotifyMaybeChannelError (this=0x487bb0e8) at /hack/mozilla-graphics/ipc/glue/AsyncChannel.cpp:535
#19 0x418fb538 in DispatchToMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)()> (this=<value optimized out>)
    at /hack/mozilla-graphics/ipc/chromium/src/base/tuple.h:383
#20 RunnableMethod<mozilla::dom::ContentParent, void (mozilla::dom::ContentParent::*)(), Tuple0>::Run (this=<value optimized out>)
    at /hack/mozilla-graphics/ipc/chromium/src/base/task.h:307
#21 0x41d0a99e in MessageLoop::RunTask (this=0x47affdd0, task=0x4931b380) at /hack/mozilla-graphics/ipc/chromium/src/base/message_loop.cc:334
#22 0x41d0b1c8 in MessageLoop::DeferOrRunPendingTask (this=0x76, pending_task=<value optimized out>)
    at /hack/mozilla-graphics/ipc/chromium/src/base/message_loop.cc:342
#23 0x41d0bf1e in MessageLoop::DoWork (this=0x47affdd0) at /hack/mozilla-graphics/ipc/chromium/src/base/message_loop.cc:442
#24 0x41d0c29a in base::MessagePumpDefault::Run (this=0x459daf40, delegate=0x47affdd0)
    at /hack/mozilla-graphics/ipc/chromium/src/base/message_pump_default.cc:23
#25 0x41d0af52 in MessageLoop::RunInternal (this=0x47affdd0) at /hack/mozilla-graphics/ipc/chromium/src/base/message_loop.cc:216
#26 0x41d0afb2 in MessageLoop::RunHandler (this=0x47affdd0) at /hack/mozilla-graphics/ipc/chromium/src/base/message_loop.cc:209
#27 MessageLoop::Run (this=0x47affdd0) at /hack/mozilla-graphics/ipc/chromium/src/base/message_loop.cc:183
#28 0x41d14ee4 in base::Thread::ThreadMain (this=0x46a5f1c0) at /hack/mozilla-graphics/ipc/chromium/src/base/thread.cc:156
#29 0x41d22616 in ThreadFunc (closure=0x76) at /hack/mozilla-graphics/ipc/chromium/src/base/platform_thread_posix.cc:39
#30 0x40016e18 in __thread_entry (func=0x41d2260d <ThreadFunc>, arg=0x46a5f1c0, tls=<value optimized out>) at bionic/libc/bionic/pthread.c:217
#31 0x4001696c in pthread_create (thread_out=<value optimized out>, attr=0xbef49178, start_routine=0x41d2260d <ThreadFunc>, arg=0x46a5f1c0)
    at bionic/libc/bionic/pthread.c:357

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Benoit Jacob [:bjacob] from comment #1)
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 531.553]
> 0x41d99e80 in ~ImageLayerComposite (this=0x49504400, __in_chrg=<value

This is probably because of a crash in the content process.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Now testing the Gallery app, I first get a crash in the main process, with this stack:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 105.237]
0x80497dc4 in ?? ()
(gdb) bt
#0  0x80497dc4 in ?? ()
#1  0x41e24798 in mozilla::layers::SurfaceStreamHostOGL::Lock (this=0x45102240) at /hack/mozilla-graphics/gfx/layers/opengl/TextureHostOGL.cpp:336
#2  0x41e1fe18 in mozilla::layers::ImageHostSingle::Composite (this=0x45101400, aEffectChain=..., aOpacity=1, aTransform=..., aOffset=..., 
    aFilter=@0x477ff1b4, aClipRect=..., aVisibleRegion=0x0, aLayerProperties=0x0) at /hack/mozilla-graphics/gfx/layers/composite/ImageHost.cpp:61
#3  0x41de9c22 in mozilla::layers::CanvasLayerComposite::RenderLayer (this=0x4511f800, aOffset=..., aClipRect=...)
    at /hack/mozilla-graphics/gfx/layers/composite/CanvasLayerComposite.cpp:98
#4  0x41deb7f0 in mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite> (aContainer=0x46b19400, aOffset=..., 
    aManager=0x48d85c60, aClipRect=...) at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:122
#5  0x41deafce in mozilla::layers::ContainerLayerComposite::RenderLayer (this=0x46b19400, aOffset=..., aClipRect=...)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:319
#6  0x41deb7f0 in mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite> (aContainer=0x47fb5c00, aOffset=..., 
    aManager=0x48d85c60, aClipRect=...) at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:122
#7  0x41deb30e in mozilla::layers::RefLayerComposite::RenderLayer (this=0x47fb5c00, aOffset=..., aClipRect=...)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:363
#8  0x41deb7f0 in mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite> (aContainer=0x451d9c00, aOffset=..., 
    aManager=0x48d85c60, aClipRect=...) at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:122
#9  0x41deafce in mozilla::layers::ContainerLayerComposite::RenderLayer (this=0x451d9c00, aOffset=..., aClipRect=...)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:319
#10 0x41deb7f0 in mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite> (aContainer=0x45674000, aOffset=..., 
    aManager=0x48d85c60, aClipRect=...) at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:122
#11 0x41deafce in mozilla::layers::ContainerLayerComposite::RenderLayer (this=0x45674000, aOffset=..., aClipRect=...)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:319
#12 0x41deb7f0 in mozilla::layers::ContainerRender<mozilla::layers::ContainerLayerComposite> (aContainer=0x45673400, aOffset=..., 
    aManager=0x48d85c60, aClipRect=...) at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:122
#13 0x41deafce in mozilla::layers::ContainerLayerComposite::RenderLayer (this=0x45673400, aOffset=..., aClipRect=...)
    at /hack/mozilla-graphics/gfx/layers/composite/ContainerLayerComposite.cpp:319

At frame 1 we have

(gdb) frame 1
#1  0x41e24798 in mozilla::layers::SurfaceStreamHostOGL::Lock (this=0x45102240) at /hack/mozilla-graphics/gfx/layers/opengl/TextureHostOGL.cpp:336
336       sharedSurf = surfStream->SwapConsumer();

Here surfStream is a non-null SurfaceStream* pointer, but SurfaceStream constructor has never been called. Looking.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

This surfStream is obtained by this code from the TSurfaceStreamDescriptor:

(gdb) p/x streamDesc
$11 = (const mozilla::layers::SurfaceStreamDescriptor &) @0x451024c0: {handle_ = 0x45670680, yflip_ = 0x0}
(gdb) p surfStream
$12 = (class mozilla::gfx::SurfaceStream *) 0x45670680
(gdb) p/x *(uint32_t*)(mBuffer->mValue.VSurfaceStreamDescriptor)
$13 = 0x45670680

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

(Thanks for Bas) So this pointer is a valid SurfaceStream* pointer in the _content_ process. We seem to be getting raw pointers from across processes here, which would explain the crash. Question: how did this work in old-layers?

Comment 7

[Nicolas Silva [:nical]]

the stack trace in comment 1 is the same as the one in bug 860463

Comment 8

[Bas Schouten (:bas.schouten)]

Attachment: Properly readback from WebGL with B2G

This fixes the problem in 2 steps:

1. On B2G we need to readback rather than try to use Stream buffers, as we've already concluded they don't work cross process.
2. We need to make things understand the resulting texture host is valid. This is currently based on if it has a texture, which is only true if the first lock on the host side has been called. This is never true because it isn't valid. We break this cycle by basing our IsValid decision off whether we have a GraphicBuffer.

It should be noted this leaves one final thing to look into, but it's not a severe problem. In theory a composition scheduled in the past could be compositing a newer WebGL frame, since we're reading back into the same Gralloc buffer for the new frame. This situation shouldn't be very bad though, so we should be able to easily deal with it in a follow-up bug with a lower priority. I'm not even sure if this was properly dealt with on m-c.

Comment 9

[Andreas Gal :gal]

Hey Bas, why do stream buffers not work cross process?

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 736157 [details] [diff] [review]
Properly readback from WebGL with B2G

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

Thanks for the heroic debugging here.

R=me with just a request for an explanatory comment:

::: gfx/layers/basic/BasicCanvasLayer.h
@@ +147,5 @@
>    }
>  
>    CompositableType GetCompositableClientType()
>    {
> +    if (mGLContext && XRE_GetProcessType() == GeckoProcessType_Default) {

Please add a detailed comment here. Will this have to be changed whenever SurfaceStream's are updated to be able to work on B2G? If yes, that's worth mentioning here.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 736157 [details] [diff] [review]
Properly readback from WebGL with B2G

(In reply to Bas Schouten (:bas.schouten) from comment #8)
> 1. On B2G we need to readback rather than try to use Stream buffers, as
> we've already concluded they don't work cross process.

Not sure where we concluded that; they work fine cross-process, we just don't have the full GraphicBuffer code implemented to enable it.  We're currently doing readback, and it's what we need to continue to do.

This looks fine, we're going to look at fixing everything next week; please land as soon as you can!

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Andreas Gal :gal from comment #9)
> Hey Bas, why do stream buffers not work cross process?

Because they currently assume that they can pass raw SurfaceStream* pointers across processes as said in comment 6, i.e. they are not yet doing IPC. IIUC, Jeff Gilbert intends to implement that in the near future.

Comment 13

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #11)
> Comment on attachment 736157 [details] [diff] [review]
> Properly readback from WebGL with B2G
> 
> (In reply to Bas Schouten (:bas.schouten) from comment #8)
> > 1. On B2G we need to readback rather than try to use Stream buffers, as
> > we've already concluded they don't work cross process.
> 
> Not sure where we concluded that; they work fine cross-process,

To be clear: they work fine cross-process doing read-back of frames in the old-layers code, and that read-back path hadn't been implemented yet in the new-layers code, which is what Bas' patch is fixing here. In the new-layers code, SurfaceStreamHostOGL was wrongly assuming that it could pass raw SurfaceStream* pointers from content to compositor.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e82da4f52af8

I forgot to change the author to Bas.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e82da4f52af8