<div dir="ltr">Hi Pekka,<br><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 20, 2018 at 2:21 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-4575604582633521893m_7635767603271625729m_-6876530226067825794gmail-">On Mon, 19 Mar 2018 18:06:03 -0600<br>
Scott Moreau <<a href="mailto:oreaus@gmail.com" target="_blank">oreaus@gmail.com</a>> wrote:<br>
<br>
> Fix memory leak introduced by 6b58ea8c. weston_wm_handle_icon() was<br>
> calling xcb_get_property_reply() without freeing the reply.<br>
> ---<br>
>  xwayland/window-manager.c | 2 ++<br>
>  1 file changed, 2 insertions(+)<br>
><br>
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c<br>
> index c307e19..24e7213 100644<br>
> --- a/xwayland/window-manager.c<br>
> +++ b/xwayland/window-manager.c<br>
> @@ -1387,6 +1387,8 @@ weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)<br>
>                                                   CAIRO_FORMAT_ARGB32,<br>
>                                                   width, height, width * 4);<br>
><br>
> +     free(reply);<br>
> +<br>
>       /* Bail out in case anything wrong happened during surface creation. */<br>
>       if (cairo_surface_status(new_surf<wbr>ace) != CAIRO_STATUS_SUCCESS) {<br>
>               cairo_surface_destroy(new_sur<wbr>face);<br>
<br>
</span>Hi,<br>
<br>
this is strictly an improvement, so I pushed it:<br>
   dc402462..d2cb711d  master -> master<br></blockquote><div><br></div><div>Thanks for picking it up, I thought this was a simple one at first too.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
It does still miss the early error returns the function, though, so<br>
it's only a partial solution.<br>
<br>
A few seconds later, I thought of a problem with this patch, so I had<br>
to push a revert immediately (no force-pushing allowed):<br>
   d2cb711d..9fe5d5fa  master -> master<br>
<br>
I explained the issue in the revert, and it boils down to<br>
cairo_image_surface_create_for<wbr>_data() not making a copy of the memory<br>
it is passed in, instead it will just keep using the pointer passed in,<br>
which means we cannot free the data until the cairo surface is<br>
destroyed. So while there is a leak to fix, the fix needs to be more<br>
involved.<br></blockquote><div><br></div><div>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(<wbr>).<br><br>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.<br><br></div><div>Thanks,<br></div><div>Scott<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This happens when I try to rush things. Sorry.<br>
<br>
<br>
Thanks,<br>
pq<br>
</blockquote></div><br></div></div></div>