GNOME Bugzilla – Bug 782109
wayland: memory leak when exporting handle
Last modified: 2017-05-09 15:23:37 UTC
In https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c#n3962 the impl->exported.user_data is never freed, even though impl->exported even has a destroy_func member.
not the most obvious use, but here it is: g_clear_pointer (&impl->exported.user_data, impl->exported.destroy_func);
When running the org.gnome.PortalTest app in flatpak with -d --command=sh and thenin valgrind, I get ==5== 41 bytes in 1 blocks are definitely lost in loss record 7,880 of 14,775 ==5== at 0x4C28E0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==5== by 0x7050937: vasprintf (vasprintf.c:76) ==5== by 0x6A281CD: g_vasprintf (gprintf.c:316) ==5== by 0x6A0376F: g_strdup_vprintf (gstrfuncs.c:514) ==5== by 0x6A0380B: g_strdup_printf (gstrfuncs.c:540) ==5== by 0x40CE94: ??? (in /app/bin/portal-test) ==5== by 0x57C09E4: xdg_exported_handle (gdkwindow-wayland.c:4025) ==5== by 0xC1D4D03: ffi_call_unix64 (in /usr/lib/libffi.so.6.0.4) ==5== by 0xC1D47F9: ffi_call (in /usr/lib/libffi.so.6.0.4) ==5== by 0xA0D4AB9: wl_closure_invoke (connection.c:935) ==5== by 0xA0D1A2F: dispatch_event.isra.4 (wayland-client.c:1310) ==5== by 0xA0D2B6B: dispatch_queue (wayland-client.c:1456) ==5== by 0xA0D2B6B: wl_display_dispatch_queue_pending (wayland-client.c:1698) ==5== by 0x57BA4C3: _gdk_wayland_display_queue_events (gdkeventsource.c:198) ==5== by 0x5763198: gdk_display_get_event (gdkdisplay.c:438) ==5== by 0x57BA1B1: gdk_event_source_dispatch (gdkeventsource.c:117) ==5== by 0x69E4536: g_main_dispatch (gmain.c:3203) ==5== by 0x69E4536: g_main_context_dispatch (gmain.c:3856) ==5== by 0x69E4767: g_main_context_iterate.isra.29 (gmain.c:3929) ==5== by 0x69E480B: g_main_context_iteration (gmain.c:3990) ==5== by 0x644CDEC: g_application_run (gapplication.c:2381) ==5== by 0x40ABA1: ??? (in /app/bin/portal-test) ==5== by 0x700468F: (below main) (libc-start.c:289) when clicking the linkbutton. I also see a critical but I'm not sure if that's related.
Created attachment 351347 [details] [review] GdkWaylandWindow: Clear export user data when used It was only cleared when unexported, but we could just as well clear it when its used too.
Created attachment 351348 [details] [review] GdkWaylandWindow: Unexport when finalizing The application might not have exported, and it'll be too late for it at this point anyway, so lets be helpful and not leak it.
Review of attachment 351347 [details] [review]: sure, ok
Review of attachment 351348 [details] [review]: sure
Attachment 351347 [details] pushed as 6d77498 - GdkWaylandWindow: Clear export user data when used Attachment 351348 [details] pushed as 251e216 - GdkWaylandWindow: Unexport when finalizing
Had to revert the second patch, since it doesn't build...
(In reply to Matthias Clasen from comment #8) > Had to revert the second patch, since it doesn't build... Oops. The build fixes had "leaked" into the bug 782325 patches. I'll update both.
Created attachment 351443 [details] [review] GtkWindow: Don't double free export user data The user data passed when exporting a Wayland window was supposed to be freed using the destroy_func, as is commonly done. This was previously broken, as the user data was just NULL:ed when exported, and only actually destroyed when unexporting before having exported. While e016d9a5dba6f6f99aee94d0b72c00bee299b96a fixed this, it introduced a regression, as GtkWindow was nice enough to free the memory anyway after having received the exported handle, causing it now to double free.
Comment on attachment 351443 [details] [review] GtkWindow: Don't double free export user data Attachment 351443 [details] pushed as d8bb385 - GtkWindow: Don't double free export user data