Possible resource leak in Weston

Thu Apr 23 14:14:43 UTC 2020

Hi Pekka,

El jue., 23 abr. 2020 a las 11:43, Pekka Paalanen
(<ppaalanen at gmail.com>) escribió:
>
> 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.

The lack of a destroy request for wl_shell seems to be the root of the
problem in my case.
Each time my application needs to show a video fragment it builds a
Gstreamer pipeline, Gstreamer's waylandsink binds to the wl_shell
client, and this results in the creation of a new ping timer. Once the
video stops the timer resources are not released. After a number of
iterations of this process, the application cannot show video anymore.

You mention that "solving this should not be necessary for solving my
immediate problem", but I am not sure why you say that. If the
wl_shell objects (and associated resources) are being leaked, how do I
work around this issue?

>
> 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.

Looks like the struct weston_desktop_client is per shell object. If I
add a registry listener twice in a row, e.g.:

===
wl_registry_add_listener (WlDisplay.registry, &registry_listener, NULL);
for (i = 0; i < 2; i++) {
    if (_wl_display_roundtrip () < 0) {
        ...handle error
    }
}

wl_registry_add_listener (WlDisplay.registry, &registry_listener, NULL);
for (i = 0; i < 2; i++) {
    if (_wl_display_roundtrip () < 0) {
        ...handle error
    }
}
===

then weston_desktop_client_create is called twice....

Thanks,

Guillermo

> 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
>