On 2018-03-20 10:26 AM, 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.
Ran valgrind memcheck to try to confirm this resolved a leak, found a lot more leaks. ugh. :) > --- > xwayland/window-manager.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c > index c307e19..9e61a67 100644 > --- a/xwayland/window-manager.c > +++ b/xwayland/window-manager.c > @@ -1353,6 +1353,14 @@ weston_wm_window_schedule_repaint(struct > weston_wm_window *window) > } > > static void > +handle_icon_surface_destroy(void *data) > +{ > + xcb_get_property_reply_t *reply = (xcb_get_property_reply_t *)data; > + > + free(reply); free takes a void * anyway, so just free(data); should be fine. > +} > + > +static void > weston_wm_handle_icon(struct weston_wm *wm, struct weston_wm_window *window) > { > xcb_get_property_reply_t *reply; > @@ -1371,16 +1379,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 +1402,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, (wrong tab size again) Those nits resolved and this is Reviewed-by: Derek Foreman <der...@osg.samsung.com> Thanks, Derek > + > &handle_icon_surface_destroy); > + > if (window->frame) > frame_set_icon(window->frame, new_surface); > else /* We don’t have a frame yet */ > @@ -1502,6 +1518,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