Bug 135348

Summary: [GTK] WebkitWebProcess crashing navigating away from ogg video element
Product: WebKit Reporter: Matt Watson <watson>
Component: WebKitGTKAssignee: Víctor M. Jáquez L. <vjaquez>
Status: RESOLVED FIXED    
Severity: Normal CC: abucur, allan.jensen, bfulgham, cgarcia, clopez, commit-queue, eric.carlson, mcatanzaro, mihnea, oliver, pnormand, svillar, vjaquez, zan
Priority: P2 Keywords: Gtk
Version: 525.x (Safari 3.2)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
backtrace of WebkitWebProcess segfault
none
tarball of gjs test case
none
Patch
none
Patch
none
Patch
none
Patch none

Description Matt Watson 2014-07-28 13:02:23 PDT
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.
Comment 1 Matt Watson 2014-07-28 13:04:02 PDT
Created attachment 235610 [details]
tarball of gjs test case
Comment 2 Víctor M. Jáquez L. 2014-07-29 01:34:59 PDT
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.
Comment 3 Matt Watson 2014-07-29 11:10:18 PDT
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
Comment 4 Víctor M. Jáquez L. 2014-08-07 08:35:13 PDT
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.
Comment 5 Víctor M. Jáquez L. 2014-08-07 08:39:12 PDT
The above is with a debug build in x86-32bits
Comment 6 Víctor M. Jáquez L. 2014-08-07 09:43:19 PDT
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.
Comment 7 Víctor M. Jáquez L. 2014-08-08 08:06:45 PDT
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)
Comment 8 Víctor M. Jáquez L. 2014-08-11 06:57:42 PDT
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.
Comment 9 Philippe Normand 2014-08-11 07:12:14 PDT
Adding Alan and Eric in CC, if this bug is related with the fix of bug 109350
Comment 10 Víctor M. Jáquez L. 2014-08-11 07:15:20 PDT
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.
Comment 11 Víctor M. Jáquez L. 2014-08-15 06:47:59 PDT
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.
Comment 12 Víctor M. Jáquez L. 2014-08-19 04:53:19 PDT
Created attachment 236809 [details]
Patch
Comment 13 Philippe Normand 2014-08-19 07:10:08 PDT
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
Comment 14 Víctor M. Jáquez L. 2014-08-19 07:41:49 PDT
Created attachment 236812 [details]
Patch
Comment 15 Carlos Alberto Lopez Perez 2014-08-19 07:50:18 PDT
(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 :)
Comment 16 Víctor M. Jáquez L. 2014-08-21 01:53:22 PDT
(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.
Comment 17 Víctor M. Jáquez L. 2014-08-21 02:24:41 PDT
Created attachment 236918 [details]
Patch
Comment 18 Philippe Normand 2014-08-21 02:27:58 PDT
Comment on attachment 236918 [details]
Patch

Thanks!
Comment 19 WebKit Commit Bot 2014-08-21 02:40:51 PDT
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
Comment 20 Philippe Normand 2014-08-21 02:44:56 PDT
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!)
Comment 21 Víctor M. Jáquez L. 2014-08-21 03:06:57 PDT
Created attachment 236921 [details]
Patch
Comment 22 Víctor M. Jáquez L. 2014-08-21 03:08:13 PDT
(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.
Comment 23 WebKit Commit Bot 2014-08-21 03:51:16 PDT
Comment on attachment 236921 [details]
Patch

Clearing flags on attachment: 236921

Committed r172828: <http://trac.webkit.org/changeset/172828>
Comment 24 WebKit Commit Bot 2014-08-21 03:51:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 25 Víctor M. Jáquez L. 2014-08-21 04:08:46 PDT
*** Bug 131223 has been marked as a duplicate of this bug. ***