Bug 36615 - memory leaks in UVMM JS code
memory leaks in UVMM JS code
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - Virtual machines (UVMM)
UCS 4.0
Other Linux
: P5 normal (vote)
: UCS 4.0-1-errata
Assigned To: Johannes Keiser
Alexander Kläser
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-11-12 18:10 CET by Florian Best
Modified: 2015-04-16 10:49 CEST (History)
4 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2014-11-12 18:10:41 CET
After a short amount of ca. 30 seconds using UVMM and closing it afterwards there are about 1600 widgets which aren't cleaned up. (caused mainly by the combination with the auto grid updater). In the dijit registry I saw repeatedly the following 3 widgets: Text, ToolTip and ProgressBar.
Comment 1 Florian Best univentionstaff 2014-11-12 18:15:57 CET
I hope i got all in svn r55723. If not we have to also apply this hunk:

diff --git a/ucs-4.0-0/virtualization/univention-virtual-machine-manager-daemon/umc/js/uvmm.js b/ucs-4.0-0/virtualization/univention-virtual-machine-manager-daemon/umc/js/uvmm.js
index 3e87636..b2955a2 100644
--- a/ucs-4.0-0/virtualization/univention-virtual-machine-manager-daemon/umc/js/uvmm.js
+++ b/ucs-4.0-0/virtualization/univention-virtual-machine-manager-daemon/umc/js/uvmm.js

@@ -1539,6 +1542,7 @@ define([
 
                                // destroy the tooltip when the widget is destroyed
                                tooltip.connect( widget, 'destroy', 'destroy' );
+                               widget.own(tooltip);
                        }
 
                        return widget;
Comment 2 Florian Best univentionstaff 2014-11-13 11:51:03 CET
The changes does not break anything but they aren't enough. The widgets should be destroyed directly when they aren't anymore referenced in the DOM node. Currently they are destroyed when the module gets closed.
Comment 3 Alexander Kläser univentionstaff 2014-11-13 12:00:11 CET
The problem persists, as a reference to the widget (albeit already deleted from the DOM) still exists in dijit/registry, because the widget's destroy() method has never been called. A possible way to handle this could be to store the id strings from widgets that are being created in the columns' formatter methods. After a refresh of the grid, theses widget ids could be checked manually whether their corresponding DOM nodes do still exist. If not destroy() could be called (or they could be removed from dijit/registry in another way).
Comment 4 Alexander Kläser univentionstaff 2015-04-09 16:42:28 CEST
Referring to r59673, I would suggest to remove widgets that are already destroyed from the internal widget list, i.e., in _cleanupWidgets and array.filter after the more complex if statement:

if (iwidget._destroyed) {
    return false;
}
Comment 5 Johannes Keiser univentionstaff 2015-04-09 19:11:42 CEST
Fixed: 

Frontend:
r59673
univention-management-console-frontend (4.1.106-51):
* Bug #36615: fixed memory leak in UVMM UMC module.

 + implemented suggestion from alex:
(In reply to Alexander Kläser from comment #4)
> Referring to r59673, I would suggest to remove widgets that are already
> destroyed from the internal widget list, i.e., in _cleanupWidgets and
> array.filter after the more complex if statement:
> 
> if (iwidget._destroyed) {
>     return false;
> }

r59677
univention-management-console-frontend (4.1.106-52):
* Bug #36615: widgets in grids that do not have a corresponding DOM node
  anymore are now getting destroyed.
---

UVMM:
r59678
univention-virtual-machine-manager-daemon (4.0.24-8):
* Bug #36615: fixed a memory leak in the UVMM UMC module.
---

YAML: r59680
Comment 6 Alexander Kläser univentionstaff 2015-04-15 15:21:00 CEST
The changes look good, I adjusted this.own(...) to this._grid.own(...) in the formatter functions to make sure that widgets are owned and cleaned up by the grid in the postrender() method.

I also adjusted the YAML file entries to be a bit more verbose

r59832 | Bug #36615: adjusted YAML files
r59831 | Bug #36615: make sure to clean up widgets created in formatter functions