Bug 135348 – [GTK] WebkitWebProcess crashing navigating away from ogg video element

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Matt Watson]

Created attachment 235609 [details]
backtrace of WebkitWebProcess segfault

Getting a segfault from within the WebkitWebProcess on fedora 20 using webkit gtk.

To reproduce
 - load a page with a ogg video in a html5 video element
 - load another page
 - WebkitWebProcess will crash

Attaching the complete backtrace of the WebkitWebProcess (backtrace.txt).

Also wrote a minimal test case using gjs and webkitgtk3, which I'll attach here. To run extract and cd webkit_crash, then run gjs browser.js, this will load the video page, click the link to load a second page and crash webkit.

Alternately in epiphany
 - open http://www.w3schools.com/html/tryit.asp?filename=tryhtml_video_html5_4
 - open http://google.com
 - "Oops something has gone wrong"

Filed this over at gnome and was directed over here, https://bugzilla.gnome.org/show_bug.cgi?id=733707 Didn't get a repro on a 64 bit machine, so some possibility this is a problem only on 32 bit.

[Matt Watson]

Created attachment 235610 [details]
tarball of gjs test case

[Víctor M. Jáquez L.]

What WebKitGTK+ are you using? 2.2.x or 2.4.x?

My guts say that this is also related with this bug https://bugs.webkit.org/show_bug.cgi?id=128818

I'll try to make a build disabling the fastmalloc and see how it goes.

[Matt Watson]

I'm using 2.2.7

$ yum list installed | grep webkit
libwebkit2gtk.i686             2.2.7-3.fc20         @updates                    
webkitgtk3.i686                2.2.7-3.fc20         @updates                    
webkitgtk3-debuginfo.i686      2.2.7-3.fc20         @updates-debuginfo          
webkitgtk3-devel.i686          2.2.7-3.fc20         @updates

[Víctor M. Jáquez L.]

I can reproduce it in WebKitGTK+-2.4.4, but the test case is wrong: simply modify the a.html and change the link to b.html instead of a.html.

An the resulting backtrace is not related with fastMalloc at all, but with the TextureMapper

Program received signal SIGSEGV, Segmentation fault.
0xf37da44c in WebCore::TextureMapperLayer::paintSelf (this=0x9d51e00, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:157
157         m_contentsLayer->paintToTextureMapper(options.textureMapper, m_state.contentsRect, transform, options.opacity);
(gdb) bt
#0  0xf37da44c in WebCore::TextureMapperLayer::paintSelf (this=0x9d51e00, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:157
#1  0xf37da5da in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x9d51e00, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:176
#2  0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x9d51e00, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#3  0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x9d51e00, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#4  0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x9dbf118, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#5  0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x9dbf118, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#6  0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x9dbf118, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#7  0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x987b3f8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#8  0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x987b3f8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#9  0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x987b3f8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#10 0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x991bcc8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#11 0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x991bcc8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#12 0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x991bcc8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#13 0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x991b858, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#14 0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x991b858, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#15 0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x991b858, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#16 0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x987a480, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#17 0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x987a480, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#18 0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x987a480, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#19 0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x973a768, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#20 0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x973a768, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#21 0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x973a768, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#22 0xf37da74c in WebCore::TextureMapperLayer::paintSelfAndChildren (this=0x98a6ab8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:191
#23 0xf37daa3a in WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica (this=0x98a6ab8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:231
#24 0xf37dbf6a in WebCore::TextureMapperLayer::paintRecursive (this=0x98a6ab8, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:455
#25 0xf37d9d31 in WebCore::TextureMapperLayer::paint (this=0x98a6ab8) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:92
#26 0xf2bb2439 in WebKit::LayerTreeHostGtk::compositeLayersToContext (this=0x9672148, purpose=WebKit::LayerTreeHostGtk::NotForResize)
    at ../../../opt/webkit/WebKit/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:341
#27 0xf2bb254d in WebKit::LayerTreeHostGtk::flushAndRenderLayers (this=0x9672148) at ../../../opt/webkit/WebKit/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:366
#28 0xf2bb20cc in WebKit::LayerTreeHostGtk::layerFlushTimerFired (this=0x9672148) at ../../../opt/webkit/WebKit/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:301
#29 0xf2bb2061 in WebKit::LayerTreeHostGtk::layerFlushTimerFiredCallback (layerTreeHost=0x9672148) at ../../../opt/webkit/WebKit/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:292
#30 0xee8eed11 in ?? () from /lib/i386-linux-gnu/libglib-2.0.so.0
#31 0xee8ee1d7 in g_main_context_dispatch () from /lib/i386-linux-gnu/libglib-2.0.so.0
#32 0xee8ee598 in ?? () from /lib/i386-linux-gnu/libglib-2.0.so.0
#33 0xee8ee89b in g_main_loop_run () from /lib/i386-linux-gnu/libglib-2.0.so.0
#34 0xf064d369 in WTF::RunLoop::run () at ../../../opt/webkit/WebKit/Source/WTF/wtf/gtk/RunLoopGtk.cpp:59
#35 0xf2a9e5e7 in WebKit::WebProcessMainGtk (argc=2, argv=0xffb19894) at ../../../opt/webkit/WebKit/Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:75
#36 0x08048668 in main (argc=2, argv=0xffb19894) at ../../../opt/webkit/WebKit/Source/WebKit2/gtk/MainGtk.cpp:31

It looks like if the m_contentLayer got invalid at some point.

[Víctor M. Jáquez L.]

The above is with a debug build in x86-32bits

[Víctor M. Jáquez L.]

I've cooked a web page here:

http://people.igalia.com/vjaquez/wk/ogg

In a debug built for x86 32bits, the WebProcess crashes when clicking the link, independently of the state of the video.

[Víctor M. Jáquez L.]

more debugging info:

Program received signal SIGSEGV, Segmentation fault.
0xf6268293 in WebCore::TextureMapperLayer::paintSelf (this=this@entry=0xf0594000, options=...) at ../../../opt/webkit/WebKit/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:157
157         m_contentsLayer->paintToTextureMapper(options.textureMapper, m_state.contentsRect, transform, options.opacity);
(gdb) p m_contentsLayer
$1 = (WebCore::TextureMapperPlatformLayer *) 0xf0518184
(gdb) p *m_contentsLayer
$2 = {_vptr.TextureMapperPlatformLayer = 0xbadbeefb, m_client = 0x493b0b1}


 0xbadbeefb is set by tcmalloc when unrefing:

#define POISON_DEALLOCATION_EXPLICIT(allocation, allocationSize, startPoison, endPoison) do { \
    ASSERT((allocationSize) >= 2 * sizeof(uint32_t)); \
    reinterpret_cast_ptr<uint32_t*>(allocation)[0] = 0xbadbeef9; \
    reinterpret_cast_ptr<uint32_t*>(allocation)[1] = 0xbadbeefb; \
    if ((allocationSize) < 4 * sizeof(uint32_t)) \
        break; \
    reinterpret_cast_ptr<uint32_t*>(allocation)[2] = (startPoison) ^ PTR_TO_UINT32(allocation); \
    reinterpret_cast_ptr<uint32_t*>(allocation)[END_POISON_INDEX(allocationSize)] = (endPoison) ^ PTR_TO_UINT32(allocation); \
} while (false)

[Víctor M. Jáquez L.]

After tracing a bit, I think I've found the cause of the crash: r153937 from bug 109350

If that patch is reverted, the crash is gone since the player is removed by the GC.

[Philippe Normand]

Adding Alan and Eric in CC, if this bug is related with the fix of bug 109350

[Víctor M. Jáquez L.]

I see two options:

1) we revert r153937 and let the GC do its job
2) notify to the render tree that media player element is gone.

[Víctor M. Jáquez L.]

I've dug a bit more and found that this crash only happens when the page cache is enabled.

As a workaround we could run this 

Programs/MiniBrowser --enable-page-cache=false http://people.igalia.com/vjaquez/wk/ogg/

And it won't crash.

[Víctor M. Jáquez L.]

Created attachment 236809 [details]
Patch

[Philippe Normand]

Comment on attachment 236809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236809&action=review

> Source/WebKit2/WebProcess/soup/WebProcessSoup.cpp:114
> +    WebCore::pageCache()->setShouldClearBackingStores(true);

Why is this change needed? The changelog doesn't explain it

[Víctor M. Jáquez L.]

Created attachment 236812 [details]
Patch

[Carlos Alberto Lopez Perez]

(In reply to comment #14)
> Created an attachment (id=236812) [details]
> Patch

I have just tested this patch on 64-bit release GTK (trunk as of now: r172751) and it fixes completely the crash on the test media/restore-from-page-cache.html. Good work :)

[Víctor M. Jáquez L.]

(In reply to comment #13)
> (From update of attachment 236809 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236809&action=review
> 
> > Source/WebKit2/WebProcess/soup/WebProcessSoup.cpp:114
> > +    WebCore::pageCache()->setShouldClearBackingStores(true);
> 
> Why is this change needed? The changelog doesn't explain it

This is the core of the problem:

When a page is cached, by default doesn't recreate the backing store (an optimization added in r89316).

Not all the ports uses that optimization. For example IOS port  doesn't use it (r161185).

In the case of the GTK port, the MediaPlayerPrivateGStreamer, not only processes video buffers, also display them, because it is a TextureMapperPlatformLayer too.

So far, so good.

Nevertheless, in r153937, when a page is cached, the player is destroyed. But our player has a backing store and the render tree doesn't know that the player has gone. Hence, when the page is redraw, the TextureMapper tree visits the video element, which doesn't exist anymore, a segmentation fault occurs.

So, as our media player renders, and as we cannot trust that the player exists when a page is painted, we cannot rely in the  r89316 optimization.

Disabling it fixes the problem.

[Víctor M. Jáquez L.]

Created attachment 236918 [details]
Patch

[Philippe Normand]

Comment on attachment 236918 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 236918 [details]
Patch

Rejecting attachment 236918 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 236918, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebKit2/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/5586411457609728

[Philippe Normand]

Comment on attachment 236918 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236918&action=review

> Source/WebKit2/ChangeLog:5
> +

Missing "Reviewed by NOBODY (OOPS!)

[Víctor M. Jáquez L.]

Created attachment 236921 [details]
Patch

[Víctor M. Jáquez L.]

(In reply to comment #20)
> (From update of attachment 236918 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236918&action=review
> 
> > Source/WebKit2/ChangeLog:5
> > +
> 
> Missing "Reviewed by NOBODY (OOPS!)

Ooops... fixed. Thanks for your patience.

[WebKit Commit Bot]

Comment on attachment 236921 [details]
Patch

Clearing flags on attachment: 236921

Committed r172828: <http://trac.webkit.org/changeset/172828>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Víctor M. Jáquez L.]

*** Bug 131223 has been marked as a duplicate of this bug. ***