REGRESSION(r221514): [GTK] UI process crash in WebKit::WaylandCompositor::Surface::flushPendingFrameCallbacks

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

We have 569 reports of this crash in Fedora:

Thread 1 (Thread 0x7f05c353aac0 (LWP 8748)):
#0  0x00007f05bdce0430 in WebKit::WaylandCompositor::Surface::flushPendingFrameCallbacks() (this=this@entry=0x7f05ab0f0d10) at /usr/src/debug/webkitgtk4-2.18.5-1.fc27.x86_64/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:258
        resource = <optimized out>
        __for_range = <synthetic pointer>: {<WTF::VectorBuffer<wl_resource*, 0>> = {<WTF::VectorBufferBase<wl_resource*>> = {m_buffer = 0x616d612d73656761, m_capacity = <optimized out>, m_size = <optimized out>}, <No data fields>}, <No data fields>}
        __for_begin = 0x616d612d73656761
        list = {<WTF::VectorBuffer<wl_resource*, 0>> = {<WTF::VectorBufferBase<wl_resource*>> = {m_buffer = 0x616d612d73656761, m_capacity = <optimized out>, m_size = <optimized out>}, <No data fields>}, <No data fields>}
#1  0x00007f05bdce0496 in WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*) (this=0x7f05ab0f0d10, webPage=0x0) at /usr/src/debug/webkitgtk4-2.18.5-1.fc27.x86_64/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:181
#2  0x00007f05bdcd8663 in WebKit::AcceleratedBackingStoreWayland::~AcceleratedBackingStoreWayland() (this=0x7f054839c030, __in_chrg=<optimized out>) at /usr/src/debug/webkitgtk4-2.18.5-1.fc27.x86_64/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:61
#3  0x00007f05bdcd8689 in WebKit::AcceleratedBackingStoreWayland::~AcceleratedBackingStoreWayland() (this=0x7f054839c030, __in_chrg=<optimized out>) at /usr/src/debug/webkitgtk4-2.18.5-1.fc27.x86_64/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:62
#4  0x00007f05bdcc2f3a in std::default_delete<WebKit::AcceleratedBackingStore>::operator()(WebKit::AcceleratedBackingStore*) const (this=<optimized out>, __ptr=<optimized out>) at /usr/include/c++/7/bits/unique_ptr.h:78
        __ptr = <optimized out>
        webView = 0x55e3c6bcbdd0 [EphyWebView]
#5  0x00007f05bdcc2f3a in std::unique_ptr<WebKit::AcceleratedBackingStore, std::default_delete<WebKit::AcceleratedBackingStore> >::reset(WebKit::AcceleratedBackingStore*) (__p=<optimized out>, this=<optimized out>) at /usr/include/c++/7/bits/unique_ptr.h:376
        webView = 0x55e3c6bcbdd0 [EphyWebView]
#6  0x00007f05bdcc2f3a in std::unique_ptr<WebKit::AcceleratedBackingStore, std::default_delete<WebKit::AcceleratedBackingStore> >::operator=(decltype(nullptr)) (this=<optimized out>) at /usr/include/c++/7/bits/unique_ptr.h:312
        webView = 0x55e3c6bcbdd0 [EphyWebView]
#7  0x00007f05bdcc2f3a in webkitWebViewBaseDispose(GObject*) (gobject=0x55e3c6bcbdd0 [EphyWebView]) at /usr/src/debug/webkitgtk4-2.18.5-1.fc27.x86_64/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:508
        webView = 0x55e3c6bcbdd0 [EphyWebView]
#8  0x00007f05c2c56e5c in g_object_run_dispose (object=0x55e3c6bcbdd0 [EphyWebView]) at gobject.c:1100
        __func__ = "g_object_run_dispose"
#9  0x00007f05c1eee690 in gtk_overlay_forall (overlay=<optimized out>, include_internals=<optimized out>, callback=0x7f05c2003750 <gtk_widget_destroy>, callback_data=0x0) at gtkoverlay.c:625
        priv = 0x55e3c72845c0
        child = <optimized out>
        children = <optimized out>
        main_widget = <optimized out>
#10 0x00007f05c1deee0e in gtk_container_destroy (widget=0x55e3c72846f0 [GtkOverlay]) at gtkcontainer.c:1700
        container = 0x55e3c72846f0 [GtkOverlay]
        priv = 0x55e3c72845e0

[Michael Catanzaro]

I guess the WaylandCompositor::Surface is already destroyed before the call to flushPendingFrameCallbacks? It's not clear.

The use of auto list = WTFMove(m_*CallbackList) throughout this file is confusing: I guess that swaps the member variable vector with the default-initialized empty vector, so it's probably OK, but maybe too clever.

[Carlos Garcia Campos]

Created attachment 334595 [details]
Patch

[Michael Catanzaro]

Comment on attachment 334595 [details]
Patch

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

Thanks

> Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:354
>                      auto* surface = static_cast<WaylandCompositor::Surface*>(wl_resource_get_user_data(resource));
> +                    WaylandCompositor::singleton().willDestroySurface(surface);
>                      delete surface;

Aha, so this is what I missed when staring at this bug the other day.

> Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:571
> +            break;

You could equivalently use return instead.

[Carlos Garcia Campos]

Committed r229126: <https://trac.webkit.org/changeset/229126>

[Xabier Rodríguez Calvar]

Michael, after giving my first son's blood I managed to get a backtrace with asan for the crash I am having lately that could be causing https://gitlab.gnome.org/GNOME/gtk/issues/1232 and it looks like it is this one? 

Carlos, do you know when this is going to be released in GNOME JHBuild conf?

[Michael Catanzaro]

(In reply to Xabier Rodríguez Calvar from comment #5)
> Michael, after giving my first son's blood I managed to get a backtrace with
> asan for the crash

Poor Simon :(

> I am having lately that could be causing
> https://gitlab.gnome.org/GNOME/gtk/issues/1232 and it looks like it is this
> one? 
> 
> Carlos, do you know when this is going to be released in GNOME JHBuild conf?

This was committed way back in March. Exactly what version of WebKitGTK+ are you using? Could you please post the trace from asan?

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #6)
> This was committed way back in March.

And it was backported for 2.19.92.