GNOME Bugzilla – Bug 612318
Add generic way to catch memory leak
Last modified: 2014-10-24 20:17:19 UTC
Created attachment 155667 [details] [review] Add shell_global_debug_exit Primarily useful for debugging. If Shell was run with -v option, It will show amount of leak JS objects. (It will show Gtype for leaked object With small gjs modification)
Created attachment 155668 [details] [review] Fix g_file_enumerator_next_files_finish annotation (Easily catched with previous patch.)
Created attachment 155669 [details] [review] Strange memory leak. Strange memory leak. ClutterText & StThemeNode was leak.
Review of attachment 155669 [details] [review]: ::: js/ui/workspace.js @@ +564,3 @@ + return; + if (this.title.get_stage()) The function is only called when this.title is parented, so you are effectively disabling the function here: if (true) return; As a result, the positions of both captions and close buttons are completely messed up.
Created attachment 156372 [details] [review] unref theme_node in st_widget_dispose
Created attachment 156373 [details] [review] Fix JSObject leak in workspace.js
Review of attachment 156372 [details] [review]: Good catch!
Created attachment 156401 [details] [review] Add shell_global_mem_report Primarily useful for debugging. If Shell was run with -v option, It will show amount of leak JS objects. If mozilla-js was compiled with --enable-debug, It Dump the object graph of heap-allocated things.
Attachment 155668 [details] pushed as d6223d6 - Fix g_file_enumerator_next_files_finish annotation
Review of attachment 156373 [details] [review]: ::: js/ui/workspace.js @@ +204,3 @@ this._zoomLightbox.destroy(); if (this._zoomLightbox) + this._draggable.disconnectAll(); Maybe DND.makeDraggable() should actor.connect('destroy') and call disconnectAll() and unset this.actor on itself? @@ +1374,1 @@ This looks obviously right. @@ +1480,3 @@ + Main.overview.disconnect(this._overviewHiddenId); + if (this._overviewHiddenId) This is correct too, but to make me happy, please set this._overviewHiddenId = 0; @@ +1496,3 @@ for (let w = 1; w < this._windows.length; w++) + this._windows[i].disconnectAll(); + for (let i = 0; i < this._windows.length; i++) I think we should move the disconnectAll() into WindowClone:destroy @@ +1497,3 @@ + this._windows = []; + this._windows[i].disconnectAll(); + for (let i = 0; i < this._windows.length; i++) Looks right.
Review of attachment 156401 [details] [review]: See also: https://bugzilla.gnome.org/show_bug.cgi?id=613048
Created attachment 156405 [details] [review] Fix JSObject leak in workspace.js
Review of attachment 156405 [details] [review]: This looks good.
Created attachment 156510 [details] [review] [WindowClone] Fix dnd regression dnd in SingleView stop work after 'Fix JSObject leak in workspace.js' commit
Review of attachment 156510 [details] [review]: What bug is this patch fixing? I see that we're trying to ensure we emit drag-end, but what issue is that fixing?
Review of attachment 156401 [details] [review]: Are you calling this from looking glass or ./src/gnome-shell --eval? I don't see a patch to add a "-v" option. Unfortunately we don't have a very good way to hook in stuff after the mainloop exits since that comes from mutter. ::: src/shell-global.c @@ +166,3 @@ +{ +shell_global_mem_report (ShellGlobal *global) +void What defines this? Are you expected to run "make -DDEBUG?
Comment on attachment 156401 [details] [review] Add shell_global_mem_report I don't think we want this in its current form. Marking rejected to get off of review list.
Comment on attachment 156510 [details] [review] [WindowClone] Fix dnd regression If this is still necessary, please file a separate bug.
There is a way to see quickly the memory leak : click many times on an element of the top bar (like calendar or the username menu) and see the memory consumption of the gnome-shell process increases. It's working for the activites menu too.
I see three patches that were committed as a part of this bug, and the report hasn't been touched for a long time. If there is anything else to do, can someone please state what it is?
In my opinion, we can consider now that it's a duplicate of that bugreport: https://bugzilla.gnome.org/show_bug.cgi?id=685513
(In reply to comment #20) > I see three patches that were committed as a part of this bug, and the report > hasn't been touched for a long time. If there is anything else to do, can > someone please state what it is? ==> No reply; closing as INCOMPLETE.