Bug 116654 – [BlackBerry] DRT - crashed on GraphicsContext3D::makeContextCurrent

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Xiaobo Wang]

crashed test:
fast/canvas/webgl/webgl-exceptions.html

bt:
#0  WebCore::GraphicsContext3D::makeContextCurrent (this=0xae59820)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/GraphicsContext3DBlackBerry.cpp:336
#1  0x79d0581a in ~WebGLLayerWebKitThread (this=0xa4b0200, 
    __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/WebGLLayerWebKitThread.cpp:37
#2  WebCore::WebGLLayerWebKitThread::~WebGLLayerWebKitThread (
    this=<optimized out>, __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/WebGLLayerWebKitThread.cpp:39
#3  0x79d0263e in deref (this=0xa4b03b4)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/RefCounted.h:210
#4  deref (this=0xa4b03b4)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:439
#5  derefIfNotNull<WebCore::LayerWebKitThread> (ptr=0xa4b0200)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/PassRefPtr.h:53
#6  ~RefPtr (this=<synthetic pointer>, __in_chrg=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/RefPtr.h:56
#7  WebCore::LayerWebKitThread::removeAll (this=0xacdb510, vector=...)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:448
#8  0x79d03912 in removeAll (vector=..., this=0xacdb510)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:441
#9  WebCore::LayerWebKitThread::~LayerWebKitThread (this=0xacdb510, 
    __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:85
#10 0x79d03c74 in WebCore::LayerWebKitThread::~LayerWebKitThread (
    this=0xacdb510, __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:87
#11 0x79cf1a2c in deref (this=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/RefCounted.h:210
#12 deref (this=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/GraphicsLayerBlackBerry.cpp:106
#13 derefIfNotNull<WebCore::LayerWebKitThread> (ptr=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/PassRefPtr.h:53
#14 ~RefPtr (this=0xacdb8c8, __in_chrg=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/RefPtr.h:56
#15 WebCore::GraphicsLayerBlackBerry::~GraphicsLayerBlackBerry (
    this=0xacdb730, __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/GraphicsLayerBlackBerry.cpp:110
#16 0x79cf1a54 in WebCore::GraphicsLayerBlackBerry::~GraphicsLayerBlackBerry (
    this=0xacdb730, __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/graphics/blackberry/GraphicsLayerBlackBerry.cpp:110
#17 0x7985703a in deleteOwnedPtr<WebCore::GraphicsLayer> (ptr=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/OwnPtrCommon.h:63
#18 clear (this=0xa511a20)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/OwnPtr.h:119
#19 operator= (this=0xa511a20)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/OwnPtr.h:81
#20 WebCore::RenderLayerBacking::destroyGraphicsLayers (this=0xa511a10)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:334
#21 0x7985880c in WebCore::RenderLayerBacking::~RenderLayerBacking (
    this=0xa511a10, __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:162
#22 0x798588d0 in WebCore::RenderLayerBacking::~RenderLayerBacking (
    this=0xa511a10, __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerBacking.cpp:163
#23 0x7984b48c in deleteOwnedPtr<WebCore::RenderLayerBacking> (
    ptr=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/OwnPtrCommon.h:63
#24 clear (this=<optimized out>)
    at /home/yanbin/workspace/playbook/webkit/Source/WTF/wtf/OwnPtr.h:119
#25 clearBacking (this=0xac5d7b0, layerBeingDestroyed=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:5509
#26 WebCore::RenderLayer::~RenderLayer (this=0xac5d7b0, 
    __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:268
#27 0x7984b5f4 in WebCore::RenderLayer::~RenderLayer (this=0xac5d7b0, 
    __in_chrg=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:275
#28 0x7984579c in WebCore::RenderLayer::destroy (this=0xac5d7b0, 
    renderArena=0xa77ebc8)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayer.cpp:1720
#29 0x79863b68 in WebCore::RenderLayerModelObject::destroyLayer (
    this=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderLayerModelObject.cpp:58
#30 0x7987eafa in WebCore::RenderObject::willBeDestroyed (this=0xac5d750)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderObject.cpp:2454
#31 0x7987cc0c in WebCore::RenderObject::destroy (this=0xac5d750)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderObject.cpp:2575
#32 0x7987cb78 in WebCore::RenderObject::destroyAndCleanupAnonymousWrappers (
    this=0xac5d750)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/rendering/RenderObject.cpp:2553
#33 0x794e7f32 in WebCore::Node::detach (this=0xa0bf160)
    at /home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/Node.cpp:1114
#34 0x794de684 in WebCore::Element::detach (this=0xa0bf160)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/Element.cpp:1310
#35 0x794b36d2 in detachChildren (this=0xa9be9e0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/ContainerNode.h:219
#36 WebCore::ContainerNode::detach (this=0xa9be9e0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/ContainerNode.cpp:832
#37 0x794de684 in WebCore::Element::detach (this=0xa9be9e0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/Element.cpp:1310
#38 0x794b36d2 in detachChildren (this=0xa9becb0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/ContainerNode.h:219
#39 WebCore::ContainerNode::detach (this=0xa9becb0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/ContainerNode.cpp:832
#40 0x794de684 in WebCore::Element::detach (this=0xa9becb0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/Element.cpp:1310
#41 0x794b36d2 in detachChildren (this=0xa61f298)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/ContainerNode.h:219
#42 WebCore::ContainerNode::detach (this=0xa61f298)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/ContainerNode.cpp:832
#43 0x794ca012 in WebCore::Document::detach (this=0xa61f298)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/Document.cpp:2124
#44 0x794bb8f4 in WebCore::Document::prepareForDestruction (this=0xa61f298)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/dom/Document.cpp:2153
#45 0x79702a9e in WebCore::Frame::setView (this=0x80d58f8, view=...)
    at /home/yanbin/workspace/playbook/webkit/Source/WebCore/page/Frame.cpp:269
#46 0x79704704 in WebCore::Frame::createView (this=0x80d58f8, 
    viewportSize=..., backgroundColor=..., transparent=<optimized out>, 
    fixedReportedSize=..., fixedLayoutSize=..., fixedVisibleContentRect=..., 
    useFixedLayout=true, horizontalScrollbarMode=WebCore::ScrollbarAlwaysOff, 
    horizontalLock=true, verticalScrollbarMode=WebCore::ScrollbarAlwaysOff, 
    verticalLock=true)
    at /home/yanbin/workspace/playbook/webkit/Source/WebCore/page/Frame.cpp:796
#47 0x793bdc34 in
WebCore::FrameLoaderClientBlackBerry::transitionToCommittedForNewPage
(this=0x80a0630)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:452
#48 0x7968fabc in WebCore::FrameLoader::transitionToCommitted (
    this=0x80d5938, cachedPage=...)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/FrameLoader.cpp:1912
#49 0x796913a2 in WebCore::FrameLoader::commitProvisionalLoad (this=0x80d5938)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/FrameLoader.cpp:1754
#50 0x79681c2e in commitIfReady (this=0xa7fc158)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/DocumentLoader.cpp:290
#51 commitIfReady (this=0xa7fc158)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/DocumentLoader.cpp:377
#52 WebCore::DocumentLoader::commitLoad (this=0xa7fc158, 
    data=0xa18fc58 "<!DOCTYPE html>\n<html>\n<head>\n<script
src=\"../../../fast/js/resources/js-test-pre.js\"></script>\n<script
src=\"resources/webgl-test.js\"></script>\n</head>\n<body>\n<script>\nvar gl =
create3DContext();\nshou"..., length=864)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/DocumentLoader.cpp:384
#53 0x796c829a in WebCore::CachedRawResource::data (this=0xac56698, data=..., 
    allDataReceived=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/cache/CachedRawResource.cpp:70
#54 0x796b6c14 in WebCore::SubresourceLoader::sendDataToResource (
    this=<optimized out>, data=0x2a22d000 <Address 0x2a22d000 out of bounds>, 
    length=864)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/SubresourceLoader.cpp:267
#55 0x796b6d9a in WebCore::SubresourceLoader::didReceiveDataOrBuffer (
    this=0xabd8cb0, data=<optimized out>, length=<optimized out>, 
    prpBuffer=..., encodedDataLength=864, 
    dataPayloadType=WebCore::DataPayloadBytes)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/SubresourceLoader.cpp:241
#56 0x796b6e7c in didReceiveDataOrBuffer (
    dataPayloadType=WebCore::DataPayloadBytes, encodedDataLength=864, 
    prpBuffer=..., length=864, 
    data=0x2a22d000 <Address 0x2a22d000 out of bounds>, this=0xabd8cb0)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/SubresourceLoader.cpp:228
#57 WebCore::SubresourceLoader::didReceiveData (this=0xabd8cb0, 
    data=0x2a22d000 <Address 0x2a22d000 out of bounds>, length=864, 
    encodedDataLength=864, dataPayloadType=WebCore::DataPayloadBytes)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/SubresourceLoader.cpp:218
#58 0x796b0244 in WebCore::ResourceLoader::didReceiveData (this=0xabd8cb0, 
    data=0x2a22d000 <Address 0x2a22d000 out of bounds>, length=864, 
    encodedDataLength=864)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/loader/ResourceLoader.cpp:507
#59 0x7a1070ac in WebCore::NetworkJob::handleNotifyDataReceived (
    this=0xafe0df0, buf=0x2a22d000 <Address 0x2a22d000 out of bounds>, 
    len=<optimized out>)
    at
/home/yanbin/workspace/playbook/webkit/Source/WebCore/platform/network/blackberry/NetworkJob.cpp:518

[Xiaobo Wang]

The instance of GraphicsContext3D has already been destroyed but WebGLLayerWebKitThread is not aware of that. In WebGLLayerWebKitThread m_webGLContext is a raw pointer, it will be destroyed in WebGLRenderingContext::destroyGraphicsContext3D before destructing WebGLLayerWebKitThread. Fix by making it ref counted.

[Xiaobo Wang]

Created attachment 202648 [details]
patch

[Arvid Nilsson]

Hang on Xiabo, it looks like you're setting up a circular reference here between the layer and the context...

[Arvid Nilsson]

Comment on attachment 202648 [details]
patch

Sorry for the misleading internal review - you should make the WebGLLayerWebKitThread::m_webGLContext a weak pointer rather than a ref pointer... Or fix it some other way...

[Arvid Nilsson]

(In reply to comment #4)
> (From update of attachment 202648 [details])
> Sorry for the misleading internal review - you should make the WebGLLayerWebKitThread::m_webGLContext a weak pointer rather than a ref pointer... Or fix it some other way...

In 

GraphicsContext3D::~GraphicsContext3D()
{
    if (m_texture) {
        makeContextCurrent();
        ::glDeleteTextures(1, &m_texture);
        if (m_attrs.stencil || m_attrs.depth)
            ::glDeleteRenderbuffers(1, &m_depthStencilBuffer);
        ::glDeleteFramebuffers(1, &m_fbo);
    }

    m_compositingLayer = 0; // Must release compositing layer before destroying the context.
    BlackBerry::Platform::Graphics::destroyWebGLContext(m_context);
}

You can call some method on the m_compositingLayer, like m_compositingLayer->contextDestroyed(), before actually destroying the WebGL context. Something like this:

GraphicsContext3D::~GraphicsContext3D()
{
    ...
    m_compositingLayer->webGLContextDestroyed();
    BlackBerry::Platform::Graphics::destroyWebGLContext(m_context);
}

WebGLLayerWebKitThread::webGLContextDestroyed()
{
    if (m_webGLContext && m_webGLContext->makeContextCurrent())
        deleteFrontBuffer();
}

[Arvid Nilsson]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 202648 [details] [details])
> > Sorry for the misleading internal review - you should make the WebGLLayerWebKitThread::m_webGLContext a weak pointer rather than a ref pointer... Or fix it some other way...
> 
...
 
> WebGLLayerWebKitThread::webGLContextDestroyed()
> {
>     if (m_webGLContext && m_webGLContext->makeContextCurrent())
>         deleteFrontBuffer();
> }

Actually, there's already a method that does exactly this, deleteTextures(). It should be like this:

WebGLLayerWebKitThread::webGLContextDestroyed()
{
    deleteTextures();
    m_webGLContext = 0;
}

You can also replace the call in the destructor to call deleteTextures():

 WebGLLayerWebKitThread::~WebGLLayerWebKitThread()
 {
-    if (m_webGLContext && m_webGLContext->makeContextCurrent())
-        deleteFrontBuffer();
+    deleteTextures();
 }

[Carlos Garcia Campos]

Comment on attachment 202648 [details]
patch

I'm not sure adding a circular dependency is the right fix here. The problem is that when the context is destroyed, someone else has a reference of the compositing layer, so it's not deleted when the context is destroyed. A possible solution would be to set the context to NULL in GraphicsContext3DBlackBerry destructor so that the layer doesn't keep a pointer to the deleted context.

[Xiaobo Wang]

Oops, good catch Arvid! I'll update the patch according to your suggestion.

[Xiaobo Wang]

(In reply to comment #7)
> (From update of attachment 202648 [details])
> I'm not sure adding a circular dependency is the right fix here. The problem is that when the context is destroyed, someone else has a reference of the compositing layer, so it's not deleted when the context is destroyed. A possible solution would be to set the context to NULL in GraphicsContext3DBlackBerry destructor so that the layer doesn't keep a pointer to the deleted context.

Yes, this is consistent with the comments from Arvid.

[Xiaobo Wang]

Created attachment 202665 [details]
patch - revised by Arvid

[Xiaobo Wang]

Created attachment 202667 [details]
patch - revised by Arvid

[WebKit Commit Bot]

Comment on attachment 202667 [details]
patch - revised by Arvid

Clearing flags on attachment: 202667

Committed r150635: <http://trac.webkit.org/changeset/150635>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.