Bug 186206

Summary: REGRESSION(r230950): [GTK] WebKit::CoordinatedBackingStoreTile::setBackBuffer(): WebKitWebProcess killed by SIGSEGV (ASSERTION FAILED: it != m_tiles.end())
Product: WebKit Reporter: Ryan Farmer <rfarmer84>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboya, agomez, bugs-noreply, cadubentzen, calvaris, Hironori.Fujii, magomez, mcatanzaro, pnormand, zan
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=1585064
Attachments:
Description Flags
Google News crash backtrace.
none
Backtrace of similar crash on Chase Bank website.
none
Youtube crash backtrace
none
Patch to revert bug 184143 change none

Description Ryan Farmer 2018-06-01 13:18:42 PDT
Created attachment 341780 [details]
Google News crash backtrace.

I've gotten this crash with WebkitGTK 2.20.2 on Fedora 28 while using Epiphany 3.28.2.1. 

First it happened when loading news.google.com in a new tab and then it happened again while visiting Chase Bank's website. 

Fedora's ABRT made a bug report on Red Hat's Bugzilla, but I'm copying it over here as well. 

The link to the Fedora bug report is: https://bugzilla.redhat.com/show_bug.cgi?id=1585064

I've copied the backtrace files out of that bug report and will attach them here. There's additional information on this issue as collected by ABRT over there.

----
Comment 1 Ryan Farmer 2018-06-01 13:19:15 PDT
Created attachment 341781 [details]
Backtrace of similar crash on Chase Bank website.
Comment 2 Ryan Farmer 2018-06-01 13:23:02 PDT
Truncated backtrace:
Thread no. 1 (10 frames)
 #0 WebKit::CoordinatedBackingStoreTile::setBackBuffer at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedBackingStore.cpp:58
 #1 WebKit::CoordinatedBackingStore::updateTile at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedBackingStore.cpp:84
 #2 WebKit::CoordinatedGraphicsScene::updateTilesIfNeeded at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/x86_64-redhat-linux-gnu/DerivedSources/ForwardingHeaders/wtf/ThreadSafeRefCounted.h:42
 #3 WebKit::CoordinatedGraphicsScene::setLayerState at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:292
 #4 WebKit::CoordinatedGraphicsScene::commitSceneState at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:556
 #5 WebKit::CoordinatedGraphicsScene::applyStateChanges at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:91
 #6 WebKit::ThreadedCompositor::renderLayerTree at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:259
 #7 WTF::RunLoop::TimerBase::<lambda(gpointer)>::operator() at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WTF/wtf/glib/RunLoopGLib.cpp:170
 #8 WTF::RunLoop::TimerBase::<lambda(gpointer)>::_FUN(gpointer) at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WTF/wtf/glib/RunLoopGLib.cpp:176
 #13 WTF::RunLoop::run at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WTF/wtf/glib/RunLoopGLib.cpp:96
Comment 3 Carlos Bentzen 2018-06-01 16:22:53 PDT
Tried it a few times and could not reproduce it here on Epiphany Technology Preview 3.29.2-11-g22c0f554e, WebKitGTK+ 2.21.2.

Not sure if it's been solved in this mean time. 

FWIW, I tested it on Arch.

Can you check if you reproduce it with Epiphany Technology Preview from Flatpak?

https://git.gnome.org/browse/gnome-apps-nightly/plain/epiphany.flatpakref
Comment 4 Michael Catanzaro 2018-06-01 18:14:02 PDT
There's currently no working debuginfo in the flatpak, so no way to know if it's the same crash. And even if there was, getting a backtrace out of a flatpak is far from straightforward. 2.20.2 is the latest stable release, so knowing that's affected should suffice.

Crashes like these are often not reproducible, but random occurrences, so code analysis will be required to figure it out. My first impressions:

#0  0x00007fa8cabca5a9 in WebKit::CoordinatedBackingStoreTile::setBackBuffer (this=0x8, tileRect=..., sourceRect=..., buffer=..., offset=...)
    at /usr/src/debug/webkit2gtk3-2.20.2-1.fc28.x86_64/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedBackingStore.cpp:58
No locals.

The this=0x8 is suspicious. Not very likely that valid memory is allocated at address 8. It's probably being used after it was already destroyed. But after skimming CoordinatedBackingStore.cpp, I'm not sure how this could happen.
Comment 5 Carlos Bentzen 2018-06-01 18:26:12 PDT
(In reply to Michael Catanzaro from comment #4)
> There's currently no working debuginfo in the flatpak, so no way to know if
> it's the same crash. And even if there was, getting a backtrace out of a
> flatpak is far from straightforward. 2.20.2 is the latest stable release, so
> knowing that's affected should suffice.

Thanks for pointing this out! I forgot to mention that. Just getting a crash would be an indication, although not conclusive as we can't get the backtrace.
Comment 6 Ryan Farmer 2018-06-01 22:23:35 PDT
It does seem to be a freak occurrence.  

I doubt that the site loaded has much to do with being able to provoke this crash, otherwise I could just sit there and keep hammering on the refresh button until it happens.

The Flatpak isn't super convenient to run in a number of ways. For starters, it doesn't support the system media codecs, and for another, it replaces the icons for the regular version, at least on Fedora. Although, I'm thinking about reporting the fact that it doesn't co-exist with the regular version so well as an issue in Epiphany. I tried running the flatpak to see if I could make a password manager crash happen with it the other day and although the crash did happen, the backtrace was totally useless. Like, to the point where it may not even be apparent that any crash that does happen is this crash. :(
Comment 7 Ryan Farmer 2018-06-03 15:17:11 PDT
(In reply to Carlos Eduardo Ramalho from comment #5)
> (In reply to Michael Catanzaro from comment #4)
> > There's currently no working debuginfo in the flatpak, so no way to know if
> > it's the same crash. And even if there was, getting a backtrace out of a
> > flatpak is far from straightforward. 2.20.2 is the latest stable release, so
> > knowing that's affected should suffice.
> 
> Thanks for pointing this out! I forgot to mention that. Just getting a crash
> would be an indication, although not conclusive as we can't get the
> backtrace.

I can confirm that this is still a problem. I pulled and built a copy of Epiphany and WebkitGTK from master today and I managed to get it to crash on Youtube.com.

I'll attach the backtrace it generated.
Comment 8 Ryan Farmer 2018-06-03 15:18:48 PDT
Created attachment 341873 [details]
Youtube crash backtrace
Comment 9 Michael Catanzaro 2018-06-12 08:48:03 PDT
*** Bug 186563 has been marked as a duplicate of this bug. ***
Comment 10 Michael Catanzaro 2018-06-18 11:31:51 PDT
I've added an expectation for layout test media/video-seek-after-end.html, which sometimes hits this crash.
Comment 11 Michael Catanzaro 2018-06-21 08:11:56 PDT
(In reply to Michael Catanzaro from comment #10)
> I've added an expectation for layout test media/video-seek-after-end.html,
> which sometimes hits this crash.

Also, Alicia added one for media/video-duration-known-after-eos.html. These expectations need to be removed when fixing this bug.
Comment 12 Michael Catanzaro 2018-06-21 08:12:03 PDT
*** Bug 185910 has been marked as a duplicate of this bug. ***
Comment 13 Michael Catanzaro 2018-06-21 12:32:35 PDT
Philippe and Fujii both hit this today in debug builds. In debug builds, instead of crashing in CoordinatedBackingStoreTile::setBackBuffer, it asserts one frame higher, in CoordinatedBackingStoreTile::updateTile:

ASSERTION FAILED: it != m_tiles.end()
Comment 14 Michael Catanzaro 2018-06-21 12:33:43 PDT
(In reply to Michael Catanzaro from comment #13)
> Philippe and Fujii both hit this today in debug builds. In debug builds,
> instead of crashing in CoordinatedBackingStoreTile::setBackBuffer, it
> asserts one frame higher, in CoordinatedBackingStoreTile::updateTile:
> 
> ASSERTION FAILED: it != m_tiles.end()

BTW, it was Alicia who connected the dots here for me... I didn't recognize the assertion. Thanks!
Comment 15 Michael Catanzaro 2018-06-21 12:57:25 PDT
Forgot to add, to reproduce, run media/event-attributes.html (bug #116977).
Comment 16 Fujii Hironori 2018-06-21 21:24:01 PDT
Created attachment 343306 [details]
Patch to revert bug 184143 change

This crash can be solved by reverting Bug 184143 change.
Comment 17 Fujii Hironori 2018-06-22 03:37:22 PDT
CoordinatedGraphicsLayer::shouldHaveBackingStore() and
CoordinatedGraphicsScene's layerShouldHaveBackingStore() must
always match.

But, mismatch happens because an opacity animation is stopped in
CoordinatedGraphicsScene side. Then,
CoordinatedGraphicsScene::prepareContentBackingStore calls
CoordinatedGraphicsScene::removeBackingStoreIfNeeded to remove
all CoordinatedBackingStoreTile because
layerShouldHaveBackingStore() returns false.

Then, CoordinatedBackingStoreTile can't be found in CoordinatedBackingStore.
Comment 18 Michael Catanzaro 2018-06-22 07:46:55 PDT
Awesome, thank you for debugging it, Fujii.
Comment 19 Michael Catanzaro 2018-06-22 08:43:49 PDT
Committed r233080: <https://trac.webkit.org/changeset/233080>