[PATCH] xwm: Fix memory leak

[Scott Moreau]

Hi Pekka,

On Tue, Mar 20, 2018 at 2:21 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Mon, 19 Mar 2018 18:06:03 -0600
> Scott Moreau <oreaus at gmail.com> wrote:
>
> > Fix memory leak introduced by 6b58ea8c. weston_wm_handle_icon() was
> > calling xcb_get_property_reply() without freeing the reply.
> > ---
> >  xwayland/window-manager.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> > index c307e19..24e7213 100644
> > --- a/xwayland/window-manager.c
> > +++ b/xwayland/window-manager.c
> > @@ -1387,6 +1387,8 @@ weston_wm_handle_icon(struct weston_wm *wm, struct
> weston_wm_window *window)
> >                                                   CAIRO_FORMAT_ARGB32,
> >                                                   width, height, width *
> 4);
> >
> > +     free(reply);
> > +
> >       /* Bail out in case anything wrong happened during surface
> creation. */
> >       if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
> >               cairo_surface_destroy(new_surface);
>
> Hi,
>
> this is strictly an improvement, so I pushed it:
>    dc402462..d2cb711d  master -> master
>

Thanks for picking it up, I thought this was a simple one at first too.


>
> It does still miss the early error returns the function, though, so
> it's only a partial solution.
>
> A few seconds later, I thought of a problem with this patch, so I had
> to push a revert immediately (no force-pushing allowed):
>    d2cb711d..9fe5d5fa  master -> master
>
> I explained the issue in the revert, and it boils down to
> cairo_image_surface_create_for_data() not making a copy of the memory
> it is passed in, instead it will just keep using the pointer passed in,
> which means we cannot free the data until the cairo surface is
> destroyed. So while there is a leak to fix, the fix needs to be more
> involved.
>

Yes it does need a more involved fix. After hooking up a destroy listener
for the cairo surface, I noticed that window->icon_surface is never being
destroyed either, and there isn't a straight forward fix especially since
window->icon_surface being set to NULL in weston_wm_window_create_frame().

Also as a side note, I notice some strange effect sometimes where the icon
will appear where the mouse cursor is for a split second when opening a
window with an icon, and quickly do fade out animation on it.

Thanks,
Scott


>
> This happens when I try to rush things. Sorry.
>
>
> Thanks,
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180320/a307aedf/attachment.html>