Opened 14 years ago
Closed 9 years ago
#4641 closed defect (wontfix)
Menu: leak on destroy
Reported by: | guest | Owned by: | |
---|---|---|---|
Priority: | high | Milestone: | future |
Component: | Dijit | Version: | 0.9 |
Keywords: | IE, Menu, Destroy | Cc: | |
Blocked By: | Blocking: |
Description (last modified by )
Easy repeat:
for (var i = 0; i < 10000000; i++) { var x = new dijit.Menu(); x.destroy(); };
Tracked from a larger leak in menu usage; but I figure if the root menu leaks it will cause the others.
Uncertain if it's leaking in firefox, doesn't grow memory at a perceivable rate.
Leak from menus is massive if destroyRecursive isn't used when there are actual children; that may be another subject; but I mention it here only if relevant to tracking down the root cause.
I'm marking this a critical. Unless I'm doing something wrong, this is bad bad bad.
Attachments (1)
Change History (18)
comment:1 follow-up: 2 Changed 14 years ago by
Description: | modified (diff) |
---|---|
Milestone: | → 1.0 |
severity: | normal → critical |
comment:2 follow-up: 3 Changed 14 years ago by
Looking at code:
destroyDescendants in _Widget.js must be recursive. I tried a recursive version with getChildren() instead of getDescendants(), but dojo.query("> [widgetId]") (getChildren) returns the same result of dojo.query("[widgetId]") (getDescendants)
Regards,
Nicola
comment:3 Changed 14 years ago by
Update: dijit.byId("submenu2").domNode isn't child of dijit.byId("submenu1").domNode[[BR]]
Nicola
comment:4 Changed 14 years ago by
Ah right, all the menus are attached (directly) to document.body, to make positioning easier and also so the menus don't get hidden if they are inside a div with overflow: auto or overflow: hidden. But that appendChild() happens in startup() so it doesn't apply to the original test case, since the original testcase isn't even calling startup()
comment:5 Changed 14 years ago by
Matt here - I submitted the original 4641 here, and worked with SFoster in identifying the (most likely distinct) pileup of MenuItems? at the root of the DOM.
IMHO both are serious problems; but very most likely distinct issues. The only common theme is that they decimate the viability of DropDownButtons? used dynamically; but that does not necessarily mean they have the same cause.
The latter - the pileup of menu items - has nothing to do with IE. A (more) core contributor should take this bug and either kill it, or put up the "menuitems piling up in DOM" as a distinct critical bug.
comment:6 Changed 14 years ago by
Component: | General → Dijit |
---|---|
Owner: | anonymous deleted |
I can take a look although I probably won't have time to address it for 1.0. If someone else wants to address it for 1.0 that would be good.
comment:7 Changed 14 years ago by
Owner: | set to Sam Foster |
---|---|
Status: | new → assigned |
comment:8 Changed 14 years ago by
Status: | assigned → new |
---|
Cant promise I can fix this, but I'll own it for now. This really needs to be 2 bugs.. but I'll look into it before splitting them out - maybe fixing one issue wil fix the other.
comment:9 Changed 14 years ago by
Summary: | Chronic leak in dijit.Menu(destroy won't destroy) in IE → Chronic leak in dijit.Menu(destroy won't destroy) |
---|
comment:10 Changed 13 years ago by
Description: | modified (diff) |
---|
Second issue filed as #4902. Leaving this bug open for original issue on IE of memory leak from
for (var i = 0; i < 10000000; i = i + 1) { var x = new dijit.Menu(); x.destroy(); };
comment:11 Changed 13 years ago by
Seems like there are a number of issues here. One big one is a cleanup problem for the onkeypress event; I filed bug #4908 (with patch) for that.
comment:12 Changed 13 years ago by
Milestone: | 1.0 → 1.1 |
---|
comment:13 Changed 13 years ago by
Probably will be fixed once #5382 is fixed, but I'll leave it open to test.
comment:15 Changed 13 years ago by
Description: | modified (diff) |
---|---|
Milestone: | 1.2 → future |
Owner: | changed from Sam Foster to bill |
comment:16 Changed 13 years ago by
Owner: | bill deleted |
---|
comment:17 Changed 9 years ago by
Description: | modified (diff) |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
Summary: | Chronic leak in dijit.Menu(destroy won't destroy) → Menu: leak on destroy |
Closing this, it's unlikely we'll workaround this IE bug given the age of this ticket and the fact that IE6 and IE7 are fast becoming extinct. I don't see any issue [anymore] on firefox, and although IE8 has a memory increase too, it's less than on IE6 and IE7.
sIeve isn't showing any memory leaks so I'm not sure what could be done anyway.
See also #5382.
I can confirm this in Firefox. In both the 0.9.0 release and current trunk, if you go to: http://archive.dojotoolkit.org/nightly/checkout/dijit/tests/test_Menu.html that has one top-level menu on the page, with a bunch of submenus. The toplevel one is submenu1. So, dijit.byId("submenu1").destroyRecursive() should cleanly unhook and remove everything, but doesnt - elements are left in the dom, and event handlers are left hooked up - right click now yields an error "Object cannot be created in this context"
dijit.byId("submenu1") is now gone, but dijit.byId("submenu2") and its friends are still there - but orphaned.
No clue as to the cause right now. But before you destroy anything, if you try dijit.byId("submenu1").getDescendants() that collection seems correct.