MenuManagers and You

If you’re using Eclipse RCP (or even standalone SWT with JFace) you are most likely using MenuManagers in various places, for popup menus, toolbar menus, etc. You might be happily using them without much of a thought or care of where they end up after they have been shown or how their internal workings actually “work”. Danger lies down that happy path.

You probably never paid much attention to the disposal of widgets except that “master widget” you call dispose() on (usually a shell).. and just because of this, it’s time to dig into the “why”, “what” and “no kidding” of MenuManagers as there are undoubtedly a few of the “no kidding” unless you are already fully familiar with them.

If you look at the MenuManager API, you’ll see an often overlooked dispose() method. MenuManagers need disposal?! Yep, they sure do. Here’s a seemingly harmless sample snippet. We create a Shell, a Toolbar, on which we create a MenuManager, and then we dispose the toolbar. You’d think that the MenuManager would get disposed also wouldn’t you? It doesn’t. Where is it? In memory.

import org.eclipse.jface.action.Action;
import org.eclipse.jface.action.MenuManager;
import org.eclipse.jface.action.ToolBarManager;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.ToolBar;

public class MenuManagerTest1 {

    public MenuManagerTest1() {
        Display display = new Display();
        Shell shell = new Shell(display, SWT.SHELL_TRIM);
        shell.setLayout(new FillLayout());

        ToolBarManager tbm = new ToolBarManager(SWT.FLAT | SWT.RIGHT);
        tbm.add(new OneAction());
        ToolBar tb = tbm.createControl(shell);

        MenuManager mm = new MenuManager("Test Menu");
        mm.add(new OneAction());
        tbm.setContextMenuManager(mm);

        // dispose shell, this will dispose all sub-widgets as well
        shell.dispose();

        // check status of our stuff
        System.out.println("Toolbar is disposed? - " + tb.isDisposed());
        System.out.println("Menu is disposed? - " + mm.getMenu().isDisposed());
        System.out.println("ToolbarManager contains what? " + tbm.getItems());
        System.out.println("MenuManager contains what? " + mm.getItems());
    }

    class OneAction extends Action {
        public OneAction() {
            super("Test Action");
        }

        @Override
        public void run() {
            System.out.println("Action was run");
        }
    }

    public static void main(String[] args) {
        new MenuManagerTest1();
    }

}

The printout when running this will look like;

Toolbar is disposed? - true
Menu is disposed? - true
ToolbarManager contains what? [Lorg.eclipse.jface.action.IContributionItem;@16fe0f4
MenuManager contains what? [Lorg.eclipse.jface.action.IContributionItem;@19d0a1

Ok, so that's not the most intriguing printout ever, but it's mostly to show that the items are still there. The reason a manager doesn't get disposed should be pretty obvious: It's not a widget, it has no disposable parent (which would also have to be a widget), thus, it exists as long as you let it exist. There is of course a reference to the actual Menu inside the MenuManager, which is disposed as you can see above.

So why aren't MenuManagers auto-disposed in some other fashion, like when the menu is disposed? For good reason. You can create them without a parent widget which means you can keep them around and re-use them like a Factory and because of this, there is no auto-dispose when the menu is hidden. It's actually to make life easier for you even if it sometimes doesn't feel that way.

Disposing MenuManagers

There's two ways you can go about dealing with disposing your MenuManagers. Here's the first one (which might be obvious);

Assume you have a few MenuManagers in your ViewParts or EditParts. If you are creating them inside methods, you will want to refactor them out and create them only once for your class. They should be accessible and re-used throughout the life of the ViewPart and disposed when the ViewPart is disposed. This is very important! If you do not dispose it, it will not be gone, and you will most likely end up with a memory leak (as the above snippet showed). Here's a typical ViewPart dispose() override:

@Override
public void dispose() {
        try {
            if (_toolbarMenu != null) {
                _toolbarMenu.dispose();
                _toolbarMenu = null;
            }

        }
        catch (Exception err) {
            // do something with exception
        }

	    // remember to keep it going!
        super.dispose();
}

The second option is for the "show once" menus that you may have. How do you dispose those? Ideally the same as #1 but if you really have to dispose them differently, the easiest way to do it is to "auto dispose" them once the user has seen the menu and closed it or selected something from it. You will most likely create your Menu object from a call to menuManager.createContextMenu(...), so after this when you have the Menu object available, you do something like this;

final MenuManager mm = new MenuManager("Menu");

// ... various manager-related code here

// create menu object on widget
Menu menu = mm.createContextMenu(parentWidget);

// add a listener that disposes the MenuManager once user has seen the menu
menu.addMenuListener(new MenuListener() {

	 @Override
	 public void menuHidden(MenuEvent e) {
	     // dispose asynchronously or it'll dispose too fast and menu item clicked won't be run
	     Display.getDefault().asyncExec(new Runnable() {

		 @Override
		 public void run() {
		     mm.dispose();
		 }

	     });
	 }

	 @Override
	 public void menuShown(MenuEvent e) {
	 }

});

It's not the prettiest solution, but it works. Ok, so the disposal is covered now, no more leaks. Yay for us!

There is one more thing to bring up that you may/may not encounter, but if you do, it would probably be a head-scratcher.

Dynamic Menu Items

Here's a scenario that happened to me and at first seemed to be a bug:

We have a lot of tables in our application. We also have a section where users can create their own custom filters. These filters show up in our menus when you right click a table or open a menu on the toolbar. When a filter is in use, the corresponding menu item for that filter is checked. Each filter is wrapped in an IAction that overrides equals() and hashCode() that both check to see if the wrapped Filter object is the same. Thus, any action using the same filter object will return true in the equals() check.

We have an IMenuListener on the menu, so whenever the user opens the menu, just as it's being shown, we remove all old menu items in the "Filters" menu and re-create them (as filters may have changed by the user without our knowledge). We then "check" (checkbox) them according to what filters are active in the table. All this works fine, and here's where the issue arose;

We also have a MenuManager on our toolbar (ToolbarManager) associated with the table. This toolbar menu shows the exact same filter menu when popped up, through the same methods. The issue is easiest described through this chain of events...

1. User opens right click filter menu, selects a filter, item is checked, filter is applied to table.
2. User looks at toolbar menu, sees filter checked (do note this is the first time the toolbar menu was opened). Closes menu without selecting anything.
3. User opens right click filter menu again, selects same filter as before (the now checked one). Item is no longer checked, filter is removed from table.
4. User opens toolbar menu again, sees the recently cleared filter still checked(!). At this point it will always stay checked. Had we looked at the menu when stuff was unchecked, it would forever be unchecked.

I debugged this for a long time until I to my horror realized that despite me calling removeAll() and re-creating all MenuManager items in the "Filters" menu on each popup, internally, in the Menu widget itself, the removeAll() on the MenuManager removed nothing on the Menu widget. All MenuItems were still there! Hmm, what was going on? what was I missing? Why didn't removeAll() remove all MenuItems as well?

I debugged some more and noticed some code inside the MenuManager source code that looks like this:

for (int i = 0; i < mi.length; i++) {
    Object data = mi[i].getData();

    if (data == null || !clean.contains(data)) {
	mi[i].dispose();
    } else if (data instanceof IContributionItem
	    &amp;&amp; ((IContributionItem) data).isDynamic()
	    &amp;&amp; ((IContributionItem) data).isDirty()) {
	mi[i].dispose();
    }
}

This loop first of all removes all items that are not the same as before (that are in the clean array), that's good. But as I checked what got disposed, our filter items were never removed... as they are the same as before! Aha! So the second part of the if-statement checks to see if the item is Dirty or Dynamic. This was the first I had heard of Dynamic menu items and it felt like I had read the API through and through many times. The API for isDynamic() says:

Returns whether this contribution item is dynamic. A dynamic contribution item contributes items conditionally, dependent on some internal state.

Cryptic to say the least, and easily overlooked as it's very non-descriptive, but with the above looping code and how we deal with these special actions - it kind of fits. We depend on the items having checkmarks which depend on an internal state (which is; if the table filter is active or not). The MenuManager can't figure that out by itself of course, so we need to help it. The solution to this problem is obvious at this point. If isDynamic() returns true it will be disposed for us, so we need to implement that and return true.

In our code more or less everything is an IAction, for our FilterAction we create a DynamicFilterAction class that extends ActionContributionItem and then we override isDynamic() and return true.

This ensures that there is no "old" items in the menu (as now our Dynamic menu items get disposed) when we create the new one, and as such, the "checkbox issue" goes away and everything works as intended. The final code looks something like this;

class DynamicFilterAction extends ActionContributionItem {

        private FilterAction _action;

        public DynamicFilterAction(DataFilter df) {
            super(new FilterAction(df));
            _action = (FilterAction)super.getAction();
        }

        public void setChecked(boolean checked) {
            _action.setChecked(checked);
        }

        @Override
        public boolean isDynamic() {
            return true;
        }
}

Final Words

Hopefully this gives you a better insight into the rather simple but tricky world of MenuManagers and how they work. Taking good care of them will ensure that you at least don't have to worry about them sticking around in memory or doing things you didn't expect.

Happy menu managing!

Related posts:

1. Glazed Lists + SWT Tables == True If you haven’t heard about Glazed Lists until now, it’s...
2. RCP Workspaces An “Eclipse workspace” is basically a directory where all your...
3. Printing SWT Tables with PaperClips In this article I will explain how to print a...
4. The Secondary ID In many RCP applications your views (ViewParts) will not be...
5. XAML in SWT / Where The Ribbon Is I get loads of traffic and email thanks the Ribbon....