Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MenuManager.setRemoveAllWhenShown() may encourage resource leaks #172

Open
stippi opened this issue Jun 24, 2022 · 0 comments
Open

MenuManager.setRemoveAllWhenShown() may encourage resource leaks #172

stippi opened this issue Jun 24, 2022 · 0 comments

Comments

@stippi
Copy link

stippi commented Jun 24, 2022

The following pattern is often seen in code using MenuManager:

menuManager.setRemoveAllWhenShown(true);
menuManager.addMenuListener(new IMenuListener() {
	@Override
	public void menuAboutToShow(IMenuManager manager) {
		// add stuff to the manager dynamically depending on some state
	}
});

The ContributionItems added in the IMenuListener are removed before the next invocation of menuAboutToShow(), but their dispose() method is not also called. This assumes that Contribution items are created only once, and the instances are re-used between calls to menuAboutToShow(). This was not clear to me at all. I have tons of code which creates MenuManagers on the fly, or directly calls menu.add(actionInstance), which creates an anonymous ActionContributionItem that noone keeps a reference to when it is removed via setRemoveAllWhenShown(true).

If I am not mistaken MenuManager.dispose() and other implementations of IContributionItem.dispose() are written in such a way that the item remains useable. Resources are simply recreated when needed. Either the dispose() hook should be called when removing all items, or at least the documentation should make it very clear that this is what needs to be done by the user of the API.

mickaelistria pushed a commit to mickaelistria/eclipse.platform.ui that referenced this issue Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant