Bug 210636 – [GTK] Crash in cairo_surface_mark_dirty_rectangle() in accelerated compositing mode under X11

• Component: WebKitGTK
• Status: RESOLVED FIXED

Ref:
https://gitlab.gnome.org/GNOME/epiphany/-/issues/1153
https://gitlab.freedesktop.org/cairo/cairo/-/issues/400

Using epiphany-3.36.1, webkitgtk-2.28.0, cairo-1.16.0, gtk+-3.24.18 compiled both 32-bit and 64-bit.

When clicking on a youtube video, epiphany aborts with:

epiphany: cairo-surface.c:1733: cairo_surface_mark_dirty_rectangle: Assertion `! _cairo_surface_has_snapshots (surface)' failed.

The suggested fix is:

I would expect that this problem goes away when adding cairo_surface_flush(m_surface.get()); at line 203 in https://github.com/WebKit/webkit/blob/ed000b08b8305908a4802b1889578e68f4c18e9b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.cpp#L194-L196

gdb trace below:

Thread 1 (LWP 11461):
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7e44d7a in __GI_abort () at abort.c:79
#2  0x00007ffff7e3ccb8 in __assert_fail_base (fmt=0x7ffff7f84280 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x7ffff687f4d8 "! _cairo_surface_has_snapshots (surface)", file=file@entry=0x7ffff687f328 "cairo-surface.c", line=line@entry=1733, function=function@entry=0x7ffff687f920 <__PRETTY_FUNCTION__.13728> "cairo_surface_mark_dirty_rectangle") at assert.c:92
#3  0x00007ffff7e3cd36 in __GI___assert_fail (assertion=0x7ffff687f4d8 "! _cairo_surface_has_snapshots (surface)", file=0x7ffff687f328 "cairo-surface.c", line=1733, function=0x7ffff687f920 <__PRETTY_FUNCTION__.13728> "cairo_surface_mark_dirty_rectangle") at assert.c:101
#4  0x00007ffff67e4f09 in INT_cairo_surface_mark_dirty_rectangle (surface=0xc71800, x=0, y=0, width=1024, height=768) at cairo-surface.c:1733
#5  0x00007ffff68149f7 in _cairo_xlib_xcb_surface_mark_dirty (abstract_surface=0xcdb320, x=0, y=0, width=1024, height=768) at cairo-xlib-xcb-surface.c:265
#6  0x00007ffff67e500b in INT_cairo_surface_mark_dirty_rectangle (surface=0xcdb320, x=0, y=0, width=1024, height=768) at cairo-surface.c:1756
#7  0x00007ffff67e4e50 in INT_cairo_surface_mark_dirty (surface=0xcdb320) at cairo-surface.c:1687
#8  0x00007fffee807a7c in WebKit::AcceleratedBackingStoreX11::paint (this=0x7fffddffab80, cr=0xb9b6d0, clipRect=...) at ../Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.cpp:196
#9  0x00007fffee6cccc9 in webkitWebViewBaseDraw (widget=0xb46d00, cr=0xb9b6d0) at ../Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:614
#10 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0xb46d00, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#11 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0xaadf90, child=0xb46d00, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#12 0x00007ffff75f7707 in gtk_container_draw (widget=0xaadf90, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3673
#13 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0xaadf90, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#14 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0xb69240, child=0xaadf90, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#15 0x00007ffff776fafb in gtk_paned_render (gadget=0xb678e0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768, data=0x0) at ../gtk/gtkpaned.c:1818
#16 0x00007ffff75ff184 in gtk_css_custom_gadget_draw (gadget=0xb678e0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768) at ../gtk/gtkcsscustomgadget.c:159
#17 0x00007ffff7605268 in gtk_css_gadget_draw (gadget=0xb678e0, cr=0xb9b6d0) at ../gtk/gtkcssgadget.c:885
#18 0x00007ffff776f926 in gtk_paned_draw (widget=0xb69240, cr=0xb9b6d0) at ../gtk/gtkpaned.c:1782
#19 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0xb69240, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#20 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0x652360, child=0xb69240, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#21 0x00007ffff75f7707 in gtk_container_draw (widget=0x652360, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3673
#22 0x00007ffff758836d in gtk_box_draw_contents (gadget=0xb51df0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768, unused=0x0) at ../gtk/gtkbox.c:453
#23 0x00007ffff75ff184 in gtk_css_custom_gadget_draw (gadget=0xb51df0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768) at ../gtk/gtkcsscustomgadget.c:159
#24 0x00007ffff7605268 in gtk_css_gadget_draw (gadget=0xb51df0, cr=0xb9b6d0) at ../gtk/gtkcssgadget.c:885
#25 0x00007ffff75883b7 in gtk_box_draw (widget=0x652360, cr=0xb9b6d0) at ../gtk/gtkbox.c:462
#26 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0x652360, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#27 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0x9522f0, child=0x652360, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#28 0x00007ffff7758db7 in gtk_notebook_draw_stack (gadget=0x784960, cr=0xb9b6d0, x=0, y=0, width=1024, height=768, unused=0x0) at ../gtk/gtknotebook.c:2544
#29 0x00007ffff75ff184 in gtk_css_custom_gadget_draw (gadget=0x784960, cr=0xb9b6d0, x=0, y=0, width=1024, height=768) at ../gtk/gtkcsscustomgadget.c:159
#30 0x00007ffff7605268 in gtk_css_gadget_draw (gadget=0x784960, cr=0xb9b6d0) at ../gtk/gtkcssgadget.c:885
#31 0x00007ffff758ed97 in gtk_box_gadget_draw (gadget=0x8a04c0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768) at ../gtk/gtkboxgadget.c:512
#32 0x00007ffff7605268 in gtk_css_gadget_draw (gadget=0x8a04c0, cr=0xb9b6d0) at ../gtk/gtkcssgadget.c:885
#33 0x00007ffff7758e33 in gtk_notebook_draw (widget=0x9522f0, cr=0xb9b6d0) at ../gtk/gtknotebook.c:2559
#34 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0x9522f0, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#35 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0xae6560, child=0x9522f0, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#36 0x00007ffff75f7707 in gtk_container_draw (widget=0xae6560, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3673
#37 0x00007ffff758836d in gtk_box_draw_contents (gadget=0xaf88e0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768, unused=0x0) at ../gtk/gtkbox.c:453
#38 0x00007ffff75ff184 in gtk_css_custom_gadget_draw (gadget=0xaf88e0, cr=0xb9b6d0, x=0, y=0, width=1024, height=768) at ../gtk/gtkcsscustomgadget.c:159
#39 0x00007ffff7605268 in gtk_css_gadget_draw (gadget=0xaf88e0, cr=0xb9b6d0) at ../gtk/gtkcssgadget.c:885
#40 0x00007ffff75883b7 in gtk_box_draw (widget=0xae6560, cr=0xb9b6d0) at ../gtk/gtkbox.c:462
#41 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0xae6560, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#42 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0x93a390, child=0xae6560, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#43 0x00007ffff780dc25 in gtk_stack_render (gadget=0x966100, cr=0xb9b6d0, x=0, y=0, width=1024, height=768, data=0x0) at ../gtk/gtkstack.c:2207
#44 0x00007ffff75ff184 in gtk_css_custom_gadget_draw (gadget=0x966100, cr=0xb9b6d0, x=0, y=0, width=1024, height=768) at ../gtk/gtkcsscustomgadget.c:159
#45 0x00007ffff7605268 in gtk_css_gadget_draw (gadget=0x966100, cr=0xb9b6d0) at ../gtk/gtkcssgadget.c:885
#46 0x00007ffff780d8ec in gtk_stack_draw (widget=0x93a390, cr=0xb9b6d0) at ../gtk/gtkstack.c:2119
#47 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0x93a390, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#48 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0x950130, child=0x93a390, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#49 0x00007ffff75f7707 in gtk_container_draw (widget=0x950130, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3673
#50 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0x950130, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#51 0x00007ffff75f7ce0 in gtk_container_propagate_draw (container=0x9383a0, child=0x950130, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3853
#52 0x00007ffff75f7707 in gtk_container_draw (widget=0x9383a0, cr=0xb9b6d0) at ../gtk/gtkcontainer.c:3673
#53 0x00007ffff791ed0e in gtk_window_draw (widget=0x9383a0, cr=0xb9b6d0) at ../gtk/gtkwindow.c:10473
#54 0x00007ffff78f2bd3 in gtk_widget_draw_internal (widget=0x9383a0, cr=0xb9b6d0, clip_to_size=1) at ../gtk/gtkwidget.c:7080
#55 0x00007ffff7907e66 in gtk_widget_render (widget=0x9383a0, window=0xb81380, region=0xc3c670) at ../gtk/gtkwidget.c:17606
#56 0x00007ffff77209aa in gtk_main_do_event (event=0x7fffffffe0d0) at ../gtk/gtkmain.c:1840
#57 0x00007ffff696ea16 in _gdk_event_emit (event=0x7fffffffe0d0) at ../gdk/gdkevents.c:73
#58 0x00007ffff69866f6 in _gdk_window_process_updates_recurse_helper (window=0xb81380, expose_region=0xa28a80) at ../gdk/gdkwindow.c:3874
#59 0x00007ffff6986917 in _gdk_window_process_updates_recurse (window=0xb81380, expose_region=0xa28a80) at ../gdk/gdkwindow.c:3931
#60 0x00007ffff69940ae in gdk_window_impl_process_updates_recurse (window=0xb81380, region=0xa28a80) at ../gdk/gdkwindowimpl.c:333
#61 0x00007ffff6986bac in gdk_window_process_updates_internal (window=0xb81380) at ../gdk/gdkwindow.c:4020
#62 0x00007ffff698706d in gdk_window_process_updates_with_mode (window=0xb81380, recurse_mode=2) at ../gdk/gdkwindow.c:4215
#63 0x00007ffff6992c6a in gdk_window_paint_on_clock (clock=0x6792f0, data=0xb81380) at ../gdk/gdkwindow.c:11721
#64 0x00007ffff72369d2 in g_cclosure_marshal_VOID__VOIDv (closure=0xc37a70, return_value=0x0, instance=0x6792f0, args=0x7fffffffe658, marshal_data=0x0, n_params=0, param_types=0x0) at ../gobject/gmarshal.c:165
#65 0x00007ffff7233818 in _g_closure_invoke_va (closure=0xc37a70, return_value=0x0, instance=0x6792f0, args=0x7fffffffe658, n_params=0, param_types=0x0) at ../gobject/gclosure.c:873
#66 0x00007ffff724f568 in g_signal_emit_valist (instance=0x6792f0, signal_id=32, detail=0, var_args=0x7fffffffe658) at ../gobject/gsignal.c:3407
#67 0x00007ffff72507b2 in g_signal_emit (instance=0x6792f0, signal_id=32, detail=0) at ../gobject/gsignal.c:3554
#68 0x00007ffff6979d73 in _gdk_frame_clock_emit_paint (frame_clock=0x6792f0) at ../gdk/gdkframeclock.c:643
#69 0x00007ffff697a8d6 in gdk_frame_clock_paint_idle (data=0x6792f0) at ../gdk/gdkframeclockidle.c:450
#70 0x00007ffff695fed3 in gdk_threads_dispatch (data=0x839a40) at ../gdk/gdk.c:777
#71 0x00007ffff713862e in g_timeout_dispatch (source=0xcda4f0, callback=0x7ffff695fe8e <gdk_threads_dispatch>, user_data=0x839a40) at ../glib/gmain.c:4800
#72 0x00007ffff71366e6 in g_main_dispatch (context=0x66abf0) at ../glib/gmain.c:3309
#73 0x00007ffff7137543 in g_main_context_dispatch (context=0x66abf0) at ../glib/gmain.c:3974
#74 0x00007ffff7137727 in g_main_context_iterate (context=0x66abf0, block=1, dispatch=1, self=0x6a3290) at ../glib/gmain.c:4047
#75 0x00007ffff71377eb in g_main_context_iteration (context=0x66abf0, may_block=1) at ../glib/gmain.c:4108
#76 0x00007ffff738e79b in g_application_run (application=0x6e81a0, argc=1, argv=0x7fffffffeb38) at ../gio/gapplication.c:2559
#77 0x0000000000403bc5 in ?? ()
#78 0x00007ffff7e30622 in __libc_start_main (main=0x40350c, argc=1, argv=0x7fffffffeb38, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffeb28) at ../csu/libc-start.c:308
#79 0x0000000000403c5a in ?? ()

Created attachment 396763 [details]
cairo surface patch

I recompiled webkitgtk with the attached patch and it seems to have fixed things.

[Michael Catanzaro]

From Uli in https://gitlab.freedesktop.org/cairo/cairo/-/issues/400:

"""
That code is clearly wrong. Well, at least from the comment, I guess that there is no call to cairo_surface_flush() anywhere. The docs for cairo_surface_mark_dirty() state that one must call cairo_surface_flush() first.

I guess the easiest fix would be to add a call to cairo_surface_flush(m_surface.get()); at the end of that function.
"""

[Michael Catanzaro]

Created attachment 396768 [details]
Patch

[Adrian Perez]

Comment on attachment 396768 [details]
Patch

I wonder if this might unbreak some of the layout tests which
have been crashing 🤾‍♂️️

[Carlos Garcia Campos]

Comment on attachment 396768 [details]
Patch

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

> Source/WebKit/ChangeLog:1
> [phone]  Michael Catanzaro  <[e-mail]>

You should credit John or even Uli, I think.

> Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:373
> +    cairo_surface_flush(m_surface.get());
> +

This is not needed in Wayland. In this case we are writing the pixel data directly, so there aren't snapshots, and the surface is never modified after the paint, like in X11.

[Michael Catanzaro]

The documentation of cairo_surface_mark_dirty() says: "Tells cairo that drawing has been done to surface using means other than cairo, and that cairo should reread any cached areas. Note that you must call cairo_surface_flush() before doing such drawing."

(In reply to Carlos Garcia Campos from comment #5)
> This is not needed in Wayland. In this case we are writing the pixel data
> directly, so there aren't snapshots, and the surface is never modified after
> the paint, like in X11.

Are we "drawing to the surface using means other than cairo"?

If so: it needs both mark_dirty() and flush().

If not: it should not use mark_dirty() or flush(). Right?

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #6)
> The documentation of cairo_surface_mark_dirty() says: "Tells cairo that
> drawing has been done to surface using means other than cairo, and that
> cairo should reread any cached areas. Note that you must call
> cairo_surface_flush() before doing such drawing."

Right.

> (In reply to Carlos Garcia Campos from comment #5)
> > This is not needed in Wayland. In this case we are writing the pixel data
> > directly, so there aren't snapshots, and the surface is never modified after
> > the paint, like in X11.
> 
> Are we "drawing to the surface using means other than cairo"?
> 
> If so: it needs both mark_dirty() and flush().
> 
> If not: it should not use mark_dirty() or flush(). Right?

I'll try to explain how this works in both X11 and Wayland.

 - X11: We create an xlib surface for a pixmap that is rendered by the web process using a redirected xcomposite window. Using xdamage we detect updates in the pixmap and use the surface as a source to draw in the view cairo context. We use mark dirty as the comment says, because between the xdamage notification and the draw signal the web process might have modified the pixmap. When cairo is configured to use xcb instead of xlib, it might use an image surface attached to the xlib one as snapshot. In that case flush() is needed to detach that snapshot after we have drawn the surface in the context.

 - Wayland: We only create a cairo surface when gdk_cairo_draw_from_gl() can't be used. In that case we create an image surface, read all the pixels using glReadPixels and write them directly in the image surface data. We need to use mark_dirty here because we have modified the pixels data, and cairo doesn't know it. And then we use the surface as a source to draw in the view cairo context. After that the surface is not used at all, it's reused only as an optimization to avoid allocating a new one on every draw, but we are always writing all the pixels data and there's no other operation in the surface. The comment before mark_dirty was copied and it's indeed wrong, but we don't need to flush for sure.

I hope it's clear now.

[Michael Catanzaro]

OK... maybe makes more sense for you to prepare the patch, since you understand? :)

[Carlos Garcia Campos]

Created attachment 397340 [details]
Patch

[EWS]

Committed r260570: <https://trac.webkit.org/changeset/260570>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397340 [details].