[Wayland-bugs] [Bug 761312] memory leak

[gtk+ (GNOME Bugzilla)]

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

Jonas Ådahl <jadahl at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #320310|none                        |reviewed
             status|                            |

--- Comment #45 from Jonas Ådahl <jadahl at gmail.com> ---
Review of attachment 320310:

Think I've mostly managed to wrap my head around the flow now. I see a couple
of issues, but for the most part it looks good. I do however think that the two
patches before this would be better squashed into this one, since the
intermediate state isn't really an improvement (looks like we'll just always
draw on the live buffer, but maybe I'm wrong).

::: gdk/wayland/gdkwindow-wayland.c
@@ +627,3 @@
 }

+static const cairo_user_data_key_t gdk_wayland_cairo_key;

Please call this something other than the gdk_wayland_cairo_key in
gdkdisplay-wayland.c.

@@ +668,3 @@
+   * the old committed buffer again.
+   */
+  impl->staging_cairo_surface = g_steal_pointer
(&impl->committed_cairo_surface);

If we get a wl_buffer.release after a ..update_size() we'd set the staging
cairo surface to the committed surface which has the wrong size.

Simply unsetting impl->committed_cairo_surface on update_size() should fix that
I suppose, but you'd also have to unset the user_data so it can destroy
gracefully even on GdkWindow tear down.

Doing that I think could just drop impl->backfill_cairo_surface and just always
use impl->committed_surface (and always "detach" it i.e. unset user_data) when
we've read back its content. Not sure that's better than using a separate
reference ala fillback_cairo_surface though.

@@ +1961,3 @@

+  g_clear_pointer (&impl->staging_cairo_surface, cairo_surface_destroy);
+  g_clear_pointer (&impl->backfill_cairo_surface, cairo_surface_destroy);

Must we not unset the cairo surface's user data, and handle a post-GdkWindow
destruction wl_buffer.release event? Now, if the GdkWindow is destroyed before
the wl_buffer.release of the committed buffer it'll fetch a pointer pointing to
freed memory.

-- 
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/20160203/a4b83f10/attachment-0001.html>