With WebKitGTK 2.28 running under Epiphany, the latter crashes with this backtrace. Moved from https://gitlab.gnome.org/GNOME/epiphany/issues/1132 . Thread 1 (Thread 0x7f205dcdb280 (LWP 22706)): #0 WebKit::WebPageProxy::viewWidget() (this=<optimized out>) at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/API/gtk/PageClientImpl.h:59 #1 0x00007f206b7f70fc in WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*) (this=this@entry=0x7f204eeeb480, webPage=webPage@entry=0x0) at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:180 #2 0x00007f206b7f75b2 in WebKit::WaylandCompositor::Surface::~Surface() (this=0x7f204eeeb480, __in_chrg=<optimized out>) at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:158 pendingList = <optimized out> list = <optimized out> #3 0x00007f206b7f7706 in WebKit::<lambda(wl_client*, wl_resource*, uint32_t)>::<lambda(wl_resource*)>::operator() (__closure=0x0, resource=<optimized out>) at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:372 surface = 0x7f204eeeb480 #4 WebKit::<lambda(wl_client*, wl_resource*, uint32_t)>::<lambda(wl_resource*)>::_FUN(wl_resource *) () at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:373 #5 0x00007f2061bfd1df in destroy_resource (element=0x568ae40, data=<optimized out>, flags=0) at ../src/wayland-server.c:724 resource = 0x568ae40 #6 0x00007f2061c01631 in for_each_helper (func=func@entry=0x7f2061bfd180 <destroy_resource>, data=data@entry=0x7ffdc5d131bc, entries=<optimized out>, entries=<optimized out>) at ../src/wayland-util.c:372 start = <optimized out> end = 0x2475ef0 p = 0x2475ec8 ret = WL_ITERATOR_CONTINUE #7 0x00007f2061c01b10 in wl_map_for_each (map=map@entry=0x56c1030, func=func@entry=0x7f2061bfd180 <destroy_resource>, data=data@entry=0x7ffdc5d131bc) at ../src/wayland-util.c:385 ret = <optimized out> #8 0x00007f2061bfd35e in wl_client_destroy (client=client@entry=0x56c1000) at ../src/wayland-server.c:883 serial = 0 #9 0x00007f2061bfd413 in destroy_client_with_error (reason=<optimized out>, client=<optimized out>) at ../src/wayland-server.c:319 client = 0x56c1000 connection = <optimized out> resource = <optimized out> object = <optimized out> closure = <optimized out> message = <optimized out> p = {1857958179, 32544} resource_flags = <optimized out> opcode = <optimized out> size = <optimized out> since = <optimized out> len = <optimized out> #10 wl_client_connection_data (fd=<optimized out>, mask=<optimized out>, data=0x56c1000) at ../src/wayland-server.c:342 client = 0x56c1000 connection = <optimized out> resource = <optimized out> object = <optimized out> closure = <optimized out> message = <optimized out> p = {1857958179, 32544} resource_flags = <optimized out> opcode = <optimized out> size = <optimized out> since = <optimized out> len = <optimized out> #11 0x00007f2061bff0a2 in wl_event_loop_dispatch (loop=0x22e7610, timeout=timeout@entry=0) at ../src/event-loop.c:1027 ep = {{events = 25, data = {ptr = 0x3b3fae0, fd = 62126816, u32 = 62126816, u64 = 62126816}}, {events = 0, data = {ptr = 0x7ffdc5d131f0, fd = -976145936, u32 = 3318821360, u64 = 140727922274800}}, {events = 80, data = {ptr = 0x0, fd = 0, u32 = 0, u64 = 0}}, {events = 0, data = {ptr = 0x22b7e30, fd = 36404784, u32 = 36404784, u64 = 36404784}}, {events = 3318821688, data = {ptr = 0xc5d1333c00007ffd, fd = 32765, u32 = 32765, u64 = 14254230628395417597}}, {events = 32765, data = {ptr = 0x7ffdc5d13340, fd = -976145600, u32 = 3318821696, u64 = 140727922275136}}, {events = 4294967295, data = {ptr = 0xc5d1333000000000, fd = 0, u32 = 0, u64 = 14254230576855777280}}, {events = 32765, data = {ptr = 0x22b7fc0, fd = 36405184, u32 = 36405184, u64 = 36405184}}, {events = 36405192, data = {ptr = 0x22b7fc000000000, fd = 0, u32 = 0, u64 = 156359074684862464}}, {events = 11, data = {ptr = 0x7ffdc5d13360, fd = -976145568, u32 = 3318821728, u64 = 140727922275168}}, {events = 1601089651, data = {ptr = 0x7f20, fd = 32544, u32 = 32544, u64 = 32544}}, {events = 13893632, data = {ptr = 0x7f205f6f94d8 <_dbus_platform_rmutex_unlock+24>, fd = 1601148120, u32 = 1601148120, u64 = 139777016829144}}, {events = 3318821728, data = {ptr = 0xc5d1338800007ffd, fd = 32765, u32 = 32765, u64 = 14254230954812932093}}, {events = 32765, data = {ptr = 0x7ffdc5d13360, fd = -976145568, u32 = 3318821728, u64 = 140727922275168}}, {events = 1601106952, data = {ptr = 0xc5d1336000007f20, fd = 32544, u32 = 32544, u64 = 14254230783014240032}}, {events = 32765, data = {ptr = 0x7ffdc5d13388, fd = -976145528, u32 = 3318821768, u64 = 140727922275208}}, {events = 3318821792, data = {ptr = 0x5f6fcd5f00007ffd, fd = 32765, u32 = 32765, u64 = 6876940963923656701}}, {events = 32544, data = {ptr = 0x300000002, fd = 2, u32 = 2, u64 = 12884901890}}, {events = 36406400, data = {ptr = 0xc5d133a000000000, fd = 0, u32 = 0, u64 = 14254231057892114432}}, {events = 32765, data = {ptr = 0x7f205f6cb94e <dbus_connection_unref+157>, fd = 1600960846, u32 = 1600960846, u64 = 139777016641870}}, {events = 36405104, data = {ptr = 0x22b848000000000, fd = 0, u32 = 0, u64 = 156364297365094400}}, {events = 0, data = {ptr = 0xc5d133bc, fd = -976145476, u32 = 3318821820, u64 = 3318821820}}, {events = 0, data = {ptr = 0xc5d133e000000003, fd = 3, u32 = 3, u64 = 14254231332770021379}}, {events = 32765, data = {ptr = 0x7f205f6cd409 <_dbus_connection_update_dispatch_status_and_unlock+257>, fd = 1600967689, u32 = 1600967689, u64 = 139777016648713}}, {events = 3318821856, data = {ptr = 0x224402000000000, fd = 0, u32 = 0, u64 = 154318793420570624}}, {events = 0, data = {ptr = 0x22ba040, fd = 36413504, u32 = 36413504, u64 = 36413504}}, {events = 1863211885, data = {ptr = 0x7f20, fd = 32544, u32 = 32544, u64 = 32544}}, {events = 0, data = {ptr = 0x1022b8480, fd = 36406400, u32 = 36406400, u64 = 4331373696}}, {events = 3318821920, data = {ptr = 0x6ebd507c00007ffd, fd = 32765, u32 = 32765, u64 = 7979622608276586493}}, {events = 32544, data = {ptr = 0x7ffdc5d13410, fd = -976145392, u32 = 3318821904, u64 = 140727922275344}}, {events = 8, data = {ptr = 0x220974000000000, fd = 0, u32 = 0, u64 = 153288688464297984}}, {events = 0, data = {ptr = 0x7ffdc5d13428, fd = -976145368, u32 = 3318821928, u64 = 140727922275368}}} source = <optimized out> i = <optimized out> count = <optimized out> has_timers = <optimized out> #12 0x00007f206b7f69c3 in WebKit::<lambda(GSource*, GSourceFunc, gpointer)>::operator() (__closure=0x0, callback=<optimized out>, userData=<optimized out>, source=0x2630980) at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:474 wlLoopSource = 0x2630980 events = <optimized out> #13 WebKit::<lambda(GSource*, GSourceFunc, gpointer)>::_FUN(GSource *, GSourceFunc, gpointer) () at /src/gnome/webkitgtk-2.28.0/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp:476 #14 0x00007f206f0c8bad in g_main_dispatch (context=0x2244020) at ../glib/gmain.c:3179 dispatch = 0x7f206b7f6980 <WebKit::<lambda(GSource*, GSourceFunc, gpointer)>::_FUN(GSource *, GSourceFunc, gpointer)> prev_source = 0x0 was_in_call = 0 user_data = 0x0 callback = 0x0 cb_funcs = 0x0 cb_data = <optimized out> need_destroy = <optimized out> source = 0x2630980 current = 0x2244110 i = 2 __FUNCTION__ = "g_main_dispatch" #15 g_main_context_dispatch (context=context@entry=0x2244020) at ../glib/gmain.c:3844 #16 0x00007f206f0c8e00 in g_main_context_iterate (context=context@entry=0x2244020, block=block@entry=0, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:3917 max_priority = 0 timeout = 0 some_ready = 1 nfds = <optimized out> allocated_nfds = <optimized out> fds = 0x2352db0 #17 0x00007f206f0c8e8f in g_main_context_iteration (context=context@entry=0x2244020, may_block=may_block@entry=0) at ../glib/gmain.c:3978 retval = <optimized out> #18 0x00007f206f2cbdca in g_application_run (application=0x223a720 [EphyShell], argc=<optimized out>, argv=<optimized out>) at ../gio/gapplication.c:2583 arguments = 0x238a810 status = 0 context = 0x2244020 acquired_context = 1 __FUNCTION__ = "g_application_run" #19 0x0000000000404b0e in main (argc=1, argv=0x7ffdc5d13768) at ../src/ephy-main.c:427 option_context = 0x2220dc0 option_group = 0x2220e40 error = 0x0 user_time = 0 arbitrary_url = 0 ctx = 0x2355220 mode = EPHY_EMBED_SHELL_MODE_INCOGNITO status = 0 flags = (EPHY_FILE_HELPERS_PRIVATE_PROFILE | EPHY_FILE_HELPERS_ENSURE_EXISTS | EPHY_FILE_HELPERS_STEAL_DATA) desktop_info = 0x0
I suspect that WaylandCompositor is incompatible with PSON, but I'm having trouble reproducing the crash. If anyone has a reproducer, that would be lovely.
I can provide whatever information you want, but I do not know by myself what information to provide. Epiphany continues crashing occasionally after a while. I have do add, that I have only Waylang/Weston without Gnome Shell, without XWayland (the latter also stopped magically working nearly at the same time when I upgraded to WebKitGTK 2.28). And I ofter use Ctrl+Alt+F2 to exit the GUI and then come back. In the past these forth and back switches lead to Epiphany crashing, but then this was fixed. Could WebKit be more likely to crash, if it is moved in the background (leave GUI completely, change tab, or change application) during the webpage rendering?
(In reply to Дилян Палаузов from comment #2) > I can provide whatever information you want, but I do not know by myself > what information to provide. > > Epiphany continues crashing occasionally after a while. > > I have do add, that I have only Waylang/Weston without Gnome Shell, without > XWayland (the latter also stopped magically working nearly at the same time > when I upgraded to WebKitGTK 2.28). And I ofter use Ctrl+Alt+F2 to exit the > GUI and then come back. In the past these forth and back switches lead to > Epiphany crashing, but then this was fixed. Could WebKit be more likely to > crash, if it is moved in the background (leave GUI completely, change tab, > or change application) during the webpage rendering? I regularly (+10 times a day) get the crash when I close a tab. Unfortunately, I can’t find a way to reproduce the bug.
OK, so with some help from Jan-Michael, I've managed to reproduce. The error occurs when closing a browser tab that was using AC mode and has had at least one process swap. It only happens when closing a single tab, not when closing the entire browser. To reproduce: * Build with -DUSE_WPE_RENDERER=OFF (workaround for bug #209118) * Open two browser tabs in Epiphany: one www.duckduckgo.com and one www.example.com. * Load https://webkit.org/blog-files/3d-transforms/poster-circle.html in the www.duckduckgo.com tab * Close the poster circle tab * UI process crashes The example.com tab is just here to ensure we have two tabs and can therefore close the other tab without closing the entire browser. It seems to require that the process has swapped from one using AC mode to another using AC mode. The reproducer still works if you swap duckduckgo.com with the poster circle page, but using example.com to replace either one of them does not work. It actually only requires one website using AC mode. You can also reproduce by transitioning the tab from poster circle -> example.com -> back to poster circle -> close the poster circle tab.
Debugging: $ jhbuild run epiphany _WebKitWebViewBasePrivate::_WebKitWebViewBasePrivate(): creating this=0x173d160 WebKit::AcceleratedBackingStoreWayland::AcceleratedBackingStoreWayland(WebKit::WebPageProxy&): creating this=0x7fb8725eeb10 void WebKit::WaylandCompositor::registerWebPage(WebKit::WebPageProxy&): webPage=0x7fb8172e6000 void WebKit::WaylandCompositor::bindWebPage(WebKit::WebPageProxy&): webPage=0x7fb8172e6000 <<< example.com tab, unrelated _WebKitWebViewBasePrivate::_WebKitWebViewBasePrivate(): creating this=0x121a8a0 WebKit::AcceleratedBackingStoreWayland::AcceleratedBackingStoreWayland(WebKit::WebPageProxy&): creating this=0x7fb8725f3390 void WebKit::WaylandCompositor::registerWebPage(WebKit::WebPageProxy&): webPage=0x7fb8172e2a00 void WebKit::WaylandCompositor::bindWebPage(WebKit::WebPageProxy&): webPage=0x7fb8172e2a00 >>> WebKit::WaylandCompositor::Surface::Surface(): Creating this=0x7fb872590a80 void WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*): this=0x7fb872590a80 webPage=0x7fb8172e6000 m_webPage=(nil) WebKit::WaylandCompositor::Surface::Surface(): Creating this=0x7fb87258fd20 void WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*): this=0x7fb87258fd20 webPage=0x7fb8172e6000 m_webPage=(nil) So far so good. Now: void WebKit::WaylandCompositor::unbindWebPage(WebKit::WebPageProxy&): webPage=0x7fb8172e6000 void WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*): this=0x7fb87258fd20 webPage=(nil) m_webPage=0x7fb8172e6000 virtual We unbound only one of the two Surfaces. Surface 0x7fb87258fd20 (the surface for the second page, poster circle) is OK but Surface 0x7fb872590a80 (the surface for DuckDuckGo) got missed. WebKit::AcceleratedBackingStoreWayland::~AcceleratedBackingStoreWayland(): destroying this=0x7fb8725eeb10 void WebKit::WaylandCompositor::unregisterWebPage(WebKit::WebPageProxy&): webPage=0x7fb8172e6000 void WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*): this=0x7fb87258fd20 webPage=(nil) m_webPage=(nil) Here, we again unregistered only one of the two Surfaces. The DuckDuckGo surface is left with a dangling pointer to m_webPage that is no longer valid. _WebKitWebViewBasePrivate::~_WebKitWebViewBasePrivate(): destroying this=0x173d160 WebKit::WaylandCompositor::Surface::~Surface(): Destroying this=0x7fb87258fd20 void WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*): this=0x7fb87258fd20 webPage=(nil) m_webPage=(nil) WebKit::WaylandCompositor::Surface::~Surface(): Destroying this=0x7fb872590a80 void WebKit::WaylandCompositor::Surface::setWebPage(WebKit::WebPageProxy*): this=0x7fb872590a80 webPage=(nil) m_webPage=0x7fb8172e6000 Segmentation fault (core dumped) Now the Surfaces get deleted by the lambda passed to wl_resource_set_implementation() (inside: static const struct wl_compositor_interface compositorInterface). m_webPage is dangling, so Surface::setWebPages tries to use it, and we die. So problem is that both WaylandCompositor::unbindWebPage and WaylandCompositor::unregisterWebPage failed to call surface->setWebPage(nullptr) for the second Surface. They both should have done that. Will continue to debug.
(In reply to Michael Catanzaro from comment #5) > So problem is that both WaylandCompositor::unbindWebPage and > WaylandCompositor::unregisterWebPage failed to call > surface->setWebPage(nullptr) for the second Surface. They both should have > done that. Will continue to debug. Oh shit, it's very simple. We are not using a loop there. We just take the first matching Surface from m_pageMap, assuming that Surface objects never share the same WebPageProxy. Since PSON, that is no longer correct.
Created attachment 395898 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Well this turned out to be more complicated than I expected. I uploaded a WIP patch as proof of concept. It fixes the crash, but introduces a new regression: exit from AC mode no longer works, the non-accelerated page does not get rendered. To reproduce, using just one tab: load gnome.org (AC mode) and then example.com (exits AC mode). I've tried pretty hard to ensure that no accelerated surface is rendering at the time (at least, that we enter the eglDestroyImage/glDeleteTextures codepath inside WaylandCompositor::Surface::setWebPage) and I haven't made progress on this for a little while now, so I'm going to stop and just upload what I have and maybe come back to this tomorrow. The patch is peppered with debug statements to try to figure out what's going wrong with exiting AC mode.
Comment on attachment 395898 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395898&action=review > Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:284 > +// FIXME: this check isn't working > + if (m_acceleratedCompositingModeEnabled) > + WaylandCompositor::singleton().bindWebPage(m_webPage); BTW I should have rephrased this confusing debug comment before uploading. The check itself is working properly. It's just not accomplishing its goal, which is to ensure that exiting AC mode works. (Without this check, bindWebPage would display the accelerated surface, so it needs to be here.)
OK, sorry, I've failed to debug what's going wrong with the rendering, and I'm out of time for further debugging. My fallback plan is to disable PSON when building without WPE_RENDERER enabled, bug #210287. That should be an OK workaround for now.
Created attachment 396075 [details] Patch
Comment on attachment 396075 [details] Patch Smart. And much nicer than my patch. But problem is that the Surface previously bound to the WebPageProxy is still alive until the web process is closed, but now the Surface is no longer tracked anywhere by the WaylandCompositor class. Right? Unbinding it here removes it from m_pageMap in favor of this new Surface. Now what happens when we process swap back to the original Surface (e.g. using back/forward list)? Won't the call to bindWebPage fail? It has no way to find the correct Surface anymore. So I think your change is good, but not sufficient to fully fix WaylandCompositor. Do you agree? I think a separate list of all Surfaces is needed. And identifying the proper Surface from that list in bindWebPage requires the surface to know the WebCore::PageIdentifier that it was originally associated with. Yes?
(In reply to Michael Catanzaro from comment #13) > Comment on attachment 396075 [details] > Patch > > Smart. And much nicer than my patch. > > But problem is that the Surface previously bound to the WebPageProxy is > still alive until the web process is closed, This is not true. The Surface object will be deleted when wayland compositor destroys the wayland surface resource. wl_resource_set_implementation(surfaceResource, &surfaceInterface, new WaylandCompositor::Surface(), [](struct wl_resource* resource) { auto* surface = static_cast<WaylandCompositor::Surface*>(wl_resource_get_user_data(resource)); delete surface; }); > but now the Surface is no > longer tracked anywhere by the WaylandCompositor class. Right? Not true either. The Surface is not tracked by the page map, that only tracks the relationship between page and surface, but not its life time, what we store in the map is indeed a weak pointer to the Surface object. The Surface lifetime is tracked by the Wayland compositor, when the wayland surface is destroyed, the Surface object is deleted. > Unbinding it > here removes it from m_pageMap in favor of this new Surface. This is right. > Now what > happens when we process swap back to the original Surface (e.g. using > back/forward list)? The web process creates a new wayland surface (which destroys the previous one, and then the Surface object too) and bindSurfaceToWebPage message is sent for the new surface. > Won't the call to bindWebPage fail? No, it's called from bindSurfaceToWebPage. > It has no way to > find the correct Surface anymore. The right surface has been added to the map in bindSurfaceToWebPage. > So I think your change is good, but not > sufficient to fully fix WaylandCompositor. Do you agree? No. > I think a separate list of all Surfaces is needed. And identifying the > proper Surface from that list in bindWebPage requires the surface to know > the WebCore::PageIdentifier that it was originally associated with. Yes? No.
> Not true either. The Surface is not tracked by the page map, that only > tracks the relationship between page and surface, but not its life time, > what we store in the map is indeed a weak pointer to the Surface object. The > Surface lifetime is tracked by the Wayland compositor, when the wayland > surface is destroyed, the Surface object is deleted. I know, because I made it use the WeakPtr. :) > > Now what > > happens when we process swap back to the original Surface (e.g. using > > back/forward list)? > > The web process creates a new wayland surface (which destroys the previous > one, and then the Surface object too) and bindSurfaceToWebPage message is > sent for the new surface. Excellent. I just assumed it was not doing this. If I had added more prints, I would have seen that what I was trying to do wasn't needed at all.
No more crashes for me with the patch, thanks!
Committed r259942: <https://trac.webkit.org/changeset/259942>