After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 789984 - Plug some leaks
Plug some leaks
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2017-11-06 19:50 UTC by Carlos Garnacho
Modified: 2017-11-07 10:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
compositor: Keep reference to the screen on the MetaBackground (1022 bytes, patch)
2017-11-06 19:52 UTC, Carlos Garnacho
committed Details | Review
clutter: Plug evdev ClutterDeviceManager leaks (1.71 KB, patch)
2017-11-06 19:52 UTC, Carlos Garnacho
committed Details | Review
wayland: Plug leak (1.07 KB, patch)
2017-11-06 19:52 UTC, Carlos Garnacho
committed Details | Review
core: Plug leaks (1.28 KB, patch)
2017-11-06 19:52 UTC, Carlos Garnacho
committed Details | Review
backends: Plug leaks (2.24 KB, patch)
2017-11-06 19:52 UTC, Carlos Garnacho
committed Details | Review

Description Carlos Garnacho 2017-11-06 19:50:27 UTC
Been running gnome-shell on valgrind. No big fish, but I've found some leaks worth fixing (the one on page flip is small but potentially *very* frequent). Attaching some patches.
Comment 1 Carlos Garnacho 2017-11-06 19:52:04 UTC
Created attachment 363077 [details] [review]
compositor: Keep reference to the screen on the MetaBackground

This is not a leak per se, but it seems too easy to make valgrind
SIGSEGV due to MetaBackground disconnecting signals from an already
destroyed MetaScreen when trying to SIGTERM gnome-shell. Keeping a
reference fixes this.
Comment 2 Carlos Garnacho 2017-11-06 19:52:10 UTC
Created attachment 363078 [details] [review]
clutter: Plug evdev ClutterDeviceManager leaks

The unused ID GList element is leaked, and so is the node path.
Comment 3 Carlos Garnacho 2017-11-06 19:52:16 UTC
Created attachment 363079 [details] [review]
wayland: Plug leak

The remote DBus error is leaked.
Comment 4 Carlos Garnacho 2017-11-06 19:52:21 UTC
Created attachment 363080 [details] [review]
core: Plug leaks

The MetaWorkspaceLogicalMonitorData structs are leaked, and so is the
stamps hashtable in MetaDisplay.
Comment 5 Carlos Garnacho 2017-11-06 19:52:27 UTC
Created attachment 363081 [details] [review]
backends: Plug leaks

The DRM properties container must be destroyed with
drmModeFreeObjectProperties, and the connectors must be freed on every
caller. Also make it sure that gbm_device structs are destroyed with the
MetaRendererNativeGpuData that owns them.
Comment 6 Jonas Ådahl 2017-11-07 08:37:53 UTC
Review of attachment 363081 [details] [review]:

lgtm. does any of these needed for gnome-3-26 too?
Comment 7 Jonas Ådahl 2017-11-07 08:38:25 UTC
Review of attachment 363080 [details] [review]:

lgtm.
Comment 8 Jonas Ådahl 2017-11-07 08:39:28 UTC
Review of attachment 363079 [details] [review]:

lgtm.

::: src/wayland/meta-wayland.c
@@ +278,3 @@
   if (error)
     {
+      gchar *remote_error = g_dbus_error_get_remote_error (error);

nit: s/gchar/char/ and newline after declaration
Comment 9 Jonas Ådahl 2017-11-07 08:39:59 UTC
Review of attachment 363078 [details] [review]:

lgtm
Comment 10 Jonas Ådahl 2017-11-07 08:40:27 UTC
Review of attachment 363077 [details] [review]:

lgtm
Comment 11 Carlos Garnacho 2017-11-07 10:12:11 UTC
(In reply to Jonas Ådahl from comment #6)
> Review of attachment 363081 [details] [review] [review]:
> 
> lgtm. does any of these needed for gnome-3-26 too?

After some inspection the first chunk does. The others don't which I guess is good, but breaks my working theory wrt gnome-shell memory growth. Oh well...

(In reply to Jonas Ådahl from comment #8)
> Review of attachment 363079 [details] [review] [review]:
> nit: s/gchar/char/ and newline after declaration

Did that :).
Comment 12 Carlos Garnacho 2017-11-07 10:13:24 UTC
Pushed to master and gnome-3-26

Attachment 363077 [details] pushed as c2fad2d - compositor: Keep reference to the screen on the MetaBackground
Attachment 363078 [details] pushed as c86c5d6 - clutter: Plug evdev ClutterDeviceManager leaks
Attachment 363079 [details] pushed as 1809850 - wayland: Plug leak
Attachment 363080 [details] pushed as 0a36a45 - core: Plug leaks
Attachment 363081 [details] pushed as eb23664 - backends: Plug leaks