[Wayland-bugs] [Bug 761312] memory leak

[gtk+ (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=761312

--- Comment #23 from Jonas Ådahl <jadahl at gmail.com> ---
(In reply to Jonas Ådahl from comment #22)
> Review of attachment 320235 [details] [review]:
> 
> ::: gdk/wayland/gdkwindow-wayland.c
> @@ +404,3 @@
> +      cairo_surface_destroy (impl->commited_surface);
> +      impl->commited_surface = NULL;
> +    }
> 
> Shouldn't we do this on begin_paint instead? We might receive a .release
> after .frame but before we enter the paint loop.
> 
> @@ +483,3 @@
> +      g_warning ("Received buffer release notification for untracked
> buffer");
> +      return;
> +    }
> 
> Is this true branch block really an error? If we
> 
> 1. commit a buffer A
> 2. receive .frame
> 3. commit a buffer B
> 4. receive .frame
> 5. receive A.release
> 
> At 5. impl->commited will be NULL, and it wouldn't be an error.
> 
> The same would happen if we create and read back on .begin_paint.
> 
> 1. draw frame on A
> 2. commit a buffer A
> 3. draw frame on B
> 4. commit a buffer B
> 5. receive A.release
> 
> At 5. impl->commited would also be NULL, and it wouldn't be an error.
> 

Actually, it wouldn't since committed would only be NULL during paint, and we
wouldn't handle events during painting. We still need to handle the window tear
down situation though, i.e. a .release after the window was destroyed.

> I think what we need to do is something like:
> 
>   cairo_surface_t *cairo_surface = wl_buffer_get_user_data (wl_buffer);
>   
>   /* Destroy any old released cairo surface we no longer will re-use. */
>   if (impl->commited_surface != cairo_surface)
>     {
>       cairo_surface_destroy (cairo_surface);
>       return;
>     }
> 
>   if (impl->staging_surface == NULL)
>     impl->staging_surface = cairo_surface;
>   else
>     cairo_surface_destroy (cairo_surface);
> 
>   impl->commited_surface = NULL;
> 

And I just realized you replaced the buffer listener, so this won't work
either. So I suppose what needs to be done is to add the GdkWindow the cairo
surface is attached to (assuming we don't attach the same cairo surface to
multiple GdkWindow's), and set the wl_buffer user data back to the associated
cairo_surface_t. That seems to me the most reasonable way to handle the above
described situation.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-bugs/attachments/20160202/2502825e/attachment.html>