Possible resource leak in Weston

Thu Apr 23 09:43:18 UTC 2020

On Wed, 22 Apr 2020 16:45:42 +0200
Guillermo Rodriguez <guillerodriguez.dev at gmail.com> wrote:

> Hi all,
> 
> I am investigating something that looks like a resource leak in
> Weston. I first saw the problem in an application involving Gstreamer,
> which would run out of fds after a number of iterations (~1000).
> However I have also been able to reproduce it without using Gstreamer.
> 
> This is the scenario:
> 
> I have a wayland application that uses Gstreamer to display video. The
> application creates the display connection (wl_display_connect) and a
> top-level surface; these last for the lifetime of the application.
> Every time the application needs to show a video fragment, it builds a
> Gstreamer pipeline, and passes the display and surface handles to
> Gstreamer's waylandsink.
> 
> This ends up calling the gst_wl_display_new_existing function [1],
> which registers a listener for the wayland registry and binds to some
> interfaces [2]. Binding to the wl_shell interface results in a call to
> weston_desktop_wl_shell_bind -> weston_desktop_client_create where
> some resources are created, including, among others, a ping timer.
> 
> When Gstreamer is done showing the video, it cleans up and releases
> all resources. As part of this process it calls wl_shell_destroy [3],
> however this does NOT result in a call to
> weston_desktop_client_destroy, and so the associated resources are not
> released. In fact, the resources are only released when the
> application disconnects (wl_display_disconnect) and exits. At this
> point, all "pending" calls to weston_desktop_client_destroy are made.
> 
> The original symptoms (application running out of fds) are only
> visible with wayland < 1.18.0. This is because up to 1.18.0, one
> timerfd was being used for each ping timer. This was changed in this
> commit [4]. However even if the symptoms are less visible, the issue
> still exists in Weston.
> 
> Does this make sense?

Hi,

yes, it does make sense, I think.

> In case I am not overlooking something and this is indeed a Weston
> issue, any hints on how to fix it?

It is mostly a Weston issue, but there are a couple Wayland protocol
issues as well:

- wl_registry does not have a destroy request defined, hence the
  compositor cannot know when a client destroys a wl_registry. The
  compositor side of a wl_registry object can only be reaped when the
  client disconnects. Fixing this is cumbersome.

- wl_shell interface does not have a destroy request either. However,
  adding one would be much easier that with wl_registry. OTOH, wl_shell
  is deprecated, so even if we add one, I doubt the clients still using
  it would get fixed.

The above are fundamental but relatively minor leaks during a client's
lifetime in the compositor. Solving them should not be necessary for
solving your immediate problem, but they would be needed for a complete
solution rather than just pushing the failure point even further out.

There is also a peculiarity with the wl_shell_surface interface: it
does not have a destroy request defined, but the protocol object is
documented to be destroyed when the wl_surface is destroyed. You might
want to check libweston-desktop actually implements this, and that the
client also does destroy the wl_surface.

In the early days of Wayland, we missed to add destroy requests to many
interfaces which we assumed would only be instantiated once in a
client's lifetime. That's why the oldest levels of the protocol
interfaces are inconsistent wrt. freeing.

The rest is issues with libweston-desktop or weston in general. I'm not
too familiar with libweston-desktop, but I have some ideas.

To me it looks like libweston-desktop hangs too much data (including
the ping timer) on the client. E.g. the ping timer is not needed if
there is nothing to ping, so the timer could perhaps be torn down
earlier and created on demand. Note however, that while wl_shell pings
on the wl_shell_surface, xdg-shell pings on the xdg_wm_base which is
not a per-surface object. Fortunately xdg_wm_base does have a destroy
request, so hooking up to that is simple.

I assume that 'struct weston_desktop_client' is really supposed to be
per-client, and not e.g. per wl_shell object. All wl_shell objects are
totally equivalent, so ensuring this would be nice. I'm not sure if all
xdg_wm_base objects are intended to be totally equivalent as well,
because it contains the ping/pong messages, and I don't how it is
supposed to work if a client has multiple xdg_wm_base objects, e.g. can
they be used interchangeably to respond to pings.

Thanks,
pq

> 
> Thank you,
> 
> Guillermo Rodriguez
> 
>  [1]: https://github.com/GStreamer/gst-plugins-bad/blob/master/ext/wayland/wldisplay.c#L295
>  [2]: https://github.com/GStreamer/gst-plugins-bad/blob/master/ext/wayland/wldisplay.c#L204
>  [3]: https://github.com/GStreamer/gst-plugins-bad/blob/master/ext/wayland/wldisplay.c#L91
>  [4]: https://github.com/wayland-project/wayland/commit/60a8d29ad8526c18cc670612e2c94f443873011a
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20200423/51dd5a55/attachment-0001.sig>