Skip to content

Re-work weston_surface destruction

This was prompted by some issues observed when destroying/closing windows.

V6 changes:

  • introduce a weston_curtain_entry structure to be used in retrieving a weston_curtain when tearing down the compositor
  • various comments addressed

V5 changes:

  • removed the shell destroy flag, with this remove we'd another way to properly clean-up views from workspace layer (active current clients).
  • minor changes and re-order of bits of code, but fairly intact

V4 changes:

  • decided to remove libweston-desktop changes because that makes weston_desktop_surface_unlink_view() inert fand I also found out issues with kiosk-shell when attempting to find a view to de-activate from the inactive layers. As it causes issues, and it is bit controversial I've decided to take it out.
  • added a patch to defer desktop_surface to NULL in desktop_shell_destroy_surface(). Fixes an issue with attempting to destroy grabbed windows.

Third attempt (v3), and currently latest, on fixing this matter has the following changes since the v2:

  • creates an additional view to handle fade-out closing animations - the view created but also ownership is handled straight in the fade_out callback, or directly in shell_destroy()
  • re-works desktop_shell_destroy_surface() to explicitly handle the compositor tear down
  • introduces a small libweston helper to differentiate between cases where libweston-desktop should handle the weston_view destruction
  • lastly, adds some potential workarounds for libweston-desktop to avoid fight with it when destroying the view
  • while at it, this fixes #583 (closed), #584 (closed), #586 (closed)

The second attempt which does a few things in order to clarify some aspects on who/how surfaces/views should be destroyed:

  • renames weston_surface_destroy() to weston_surface_unref()
  • introduces weston_surface_ref() and makes it work similarly to how drm_fb is being used
  • avoids calling weston_view_destroy() w/ some known, documented exceptions
  • avoid fighting with libweston-desktop over the weston_view

The following is keep for history, but no longer applies:

This fixes an issue where we were artificially increasing the ref-count for weston_surface when the surface was mapped and decreased it when the surface was being destroyed. All happening only if the window animation for closing it was set to fade out.

Now as it happens, wl_compositor_interface already does that when the surface is being destroyed by registering a destroy_surface that does an explicit call to weston_surface_destroy(). This had the side effect of calling twice weston_surface_destroy() leading to a user-after free for that particular view only when the compositor was suspended and if the closing animation was set to fade out (which is by default).

Signed-off-by: Marius Vlad marius.vlad@collabora.com

Edited by Marius Vlad

Merge request reports