On 2018-03-23 02:41 PM, Scott Moreau wrote:
> A memory leak introduced by 6b58ea8c led to me finding a bigger leak,
> which is xwm was calling frame_create() without calling frame_destroy().
> This meant that the associated icon_surface was not being destroyed,
> leaving the destroy handler for it broken. Here we fix this by calling
> frame_destroy() when the window is destroyed and free the reply in
> the icon_surface destroy handler.
Reviewed-by: Derek Foreman <der...@osg.samsung.com>
Though, I guess this should probably be split into two, in case the icon
stuff needs to be pulled before the RC.
I can do that when I land it though.
Will wait on this a little longer to see if anyone else wants to review.
Looks trivially correct to me, but xwm has tricked me before.
Thanks,
Derek
> ---
>
> Changed in v2:
>
> - Setup destroy handler to free reply when surface is destroyed
> - Call frame_destroy() for window->frame
>
> Changed in v3:
>
> - Fix whitespace
> - Drop unnecessary cast in handle_icon_surface_destroy()
>
> xwayland/window-manager.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..77df8cf 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -1353,6 +1353,12 @@ weston_wm_window_schedule_repaint(struct
> weston_wm_window *window)
> }
>
> static void
> +handle_icon_surface_destroy(void *data)
> +{
> + free(data);
> +}
> +
> +static void
> weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window)
> {
> xcb_get_property_reply_t *reply;
> @@ -1371,16 +1377,20 @@ weston_wm_handle_icon(struct weston_wm *wm, struct
> weston_wm_window *window)
> length = xcb_get_property_value_length(reply);
>
> /* This is in 32-bit words, not in bytes. */
> - if (length < 2)
> + if (length < 2) {
> + free(reply);
> return;
> + }
>
> data = xcb_get_property_value(reply);
> width = *data++;
> height = *data++;
>
> /* Some checks against malformed input. */
> - if (width == 0 || height == 0 || length < 2 + width * height)
> + if (width == 0 || height == 0 || length < 2 + width * height) {
> + free(reply);
> return;
> + }
>
> new_surface =
> cairo_image_surface_create_for_data((unsigned char *)data,
> @@ -1390,9 +1400,13 @@ weston_wm_handle_icon(struct weston_wm *wm, struct
> weston_wm_window *window)
> /* Bail out in case anything wrong happened during surface creation. */
> if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
> cairo_surface_destroy(new_surface);
> + free(reply);
> return;
> }
>
> + cairo_surface_set_user_data(new_surface, NULL, reply,
> + &handle_icon_surface_destroy);
> +
> if (window->frame)
> frame_set_icon(window->frame, new_surface);
> else /* We don’t have a frame yet */
> @@ -1502,6 +1516,9 @@ weston_wm_window_destroy(struct weston_wm_window
> *window)
> window->frame_id = XCB_WINDOW_NONE;
> }
>
> + if (window->frame)
> + frame_destroy(window->frame);
> +
> if (window->surface_id)
> wl_list_remove(&window->link);
>
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
