Community
Participate
Working Groups
Steps to reproduce: Call a context menu in a view/editor more times The memory leak happens when calling a context menu in the Package Explorer view, the Java editor, the Outline view ... Many contribution items and elements are leaked. The issue happens for the following two reasons: - the PopupMenuExtender class seems to have to call the unlink method when the context menu is hidden. - the MenuManagerRendere class contains the sharedElementToRecord hash map that uses MMenuElement as a key. The classes that implement the MMenuElement interface don't override hashCode (and equals), but the ContributionRecord.mergeIntoModel method clones those elements that are contributed using the 'popup:org.eclipse.ui.popup.any' uri. That's why the sharedElementToRecord hash map contains leaked elements. I have solved the issue so that the PopupMenuExtender remembers such entries and clears them when the menu is hidden. Attached is a patch.
Created attachment 225753 [details] A patch
The mergeIntoModel discard the clone if it can find an already exiting model element: shared.setVisibleWhen(merge(copy.getVisibleWhen(), shared.getVisibleWhen())); copy = shared; The existing model element should be used then, right? PW
I suppose it should work that way. But it seems there are elements that don't have equals/hashCode (or it isn't based on the elementId property). Because of that the line: MMenu shared = findExistingMenu(copy.getElementId()) can't always find the existing menu and those elements aren't removed from the sharedElementToRecord hash map.
Can we not switch getList(item) to addRecord/removeRecord? Then when the last ContributionRecord is removed from association from a specific item, we can do a sharedElementToRecord.remove(item) and the keys won't be leaked any more. PW
Switching to an add/remove for the sharedElementToRecord does retain some elements, but at a stable number (opening/closing the same context menu over and over doesn't cause the number of keys to continue to grow). Was this map the main concern of leaked objects? Or were there others? PW
Suggested fix with add/remove https://git.eclipse.org/r/#/c/18692/ PW
(In reply to Paul Webster from comment #6) > Suggested fix with add/remove https://git.eclipse.org/r/#/c/18692/ > > PW Can we first test drive this on 'master'?
(In reply to Dani Megert from comment #7) > > Can we first test drive this on 'master'? Yes, I agree. PW
Released a fix in master to testdrive the fix for a while: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9c42f70f507a5906e6a9faf634b1951fee9f7509 PW
(In reply to Paul Webster from comment #6) > Suggested fix with add/remove https://git.eclipse.org/r/#/c/18692/ Paul, could you please review this? PW
Fix looks good. Released on R43_maintenance as: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_3_maintenance&id=8c46fc911dbe339c175723fe1fe2fd5c9ba9f95e
Verified in 4.3.2.M20140129-0800
