GNOME Bugzilla – Bug 745163
Pending frame callbacks aren't destroyed with its surface
Last modified: 2015-03-05 16:35:06 UTC
In the unlikely event that a surface is destroyed right after having done wl_surface_frame()/wl_surface_commit(), mutter may have already pushed the callback to the compositor in order to be called, resulting in callbacks running for the just destroyed surface. I'm attaching a patch that ensures pending frame callbacks are removed at the time of destroying a surface.
Created attachment 297890 [details] [review] wayland: Destroy pending frame callbacks when destroying a surface MetaWaylandFrameCallback has been added a surface field, which is then checked when destroying the surfaces. This prevents unintended callbacks to run after a surface has been destroyed.
FWIW, bug https://bugzilla.gnome.org/show_bug.cgi?id=743427 lead to the discovery of this
Review of attachment 297890 [details] [review]: ::: src/wayland/meta-wayland-surface.c @@ +697,3 @@ + meta_wayland_compositor_destroy_frame_callbacks (compositor, surface); + Hmm. I think we might have a race condition if we remove them here. It would occur when: client: wl_surface.frame client: xdg_surface.destroy server: repaint - won't draw buffer from wl_surface as xdg_surface.destroy unmanaged the window client: wl_surface.destroy In this situation we'd still respond to the frame callback even though the surface was never drawn.
(In reply to Jonas Ådahl from comment #3) > Review of attachment 297890 [details] [review] [review]: > > ::: src/wayland/meta-wayland-surface.c > @@ +697,3 @@ > > + meta_wayland_compositor_destroy_frame_callbacks (compositor, surface); > + > > Hmm. I think we might have a race condition if we remove them here. It would > occur when: > > client: wl_surface.frame > client: xdg_surface.destroy > server: repaint - won't draw buffer from wl_surface as xdg_surface.destroy > unmanaged the window > client: wl_surface.destroy > > In this situation we'd still respond to the frame callback even though the > surface was never drawn. Right... I guess the proper fix is ensuring this is called from the destructor of the roles implying a surface makes it onscreen. This call on wl_surface_destructor should stay I guess, both as a safety net, and for surfaces that don't need a role to be presented onscreen (eg. the pointer cursor). I'm updating the patch.
Created attachment 298448 [details] [review] wayland: Destroy pending frame callbacks when destroying a surface MetaWaylandFrameCallback has been added a surface field, which is then checked when destroying the surfaces. This prevents unintended callbacks to run after a surface has been destroyed.
Review of attachment 298448 [details] [review]: Looks good to me.
Attachment 298448 [details] pushed as d3988c0 - wayland: Destroy pending frame callbacks when destroying a surface