Description Dirk Baeumer 2001-10-10 22:21:48 EDT

The SelectionProviderAction adds itself as a listener to the passed selection provider. But the action never removes itself
	from the provider nor does this anybody else. As a result we are leaking selection provider actions that are no longer needed

	Example: OpenToActions which get created everytime we open a context menu in the navigator or packages view. The
	onces created the last time don't get removed as a selection provider listener.

NOTES:
	AK (3/21/01 1:33:28 PM)
	via the actions that do not get deregistered we leak tons of objects - lots of actions know their editor, that knows documents etc. etc.
	all this cannot be gc'd

NE (05/01/01 2:34:14 PM)
	ResourceNavigator now uses OpenNewPageMenu, OpenNewWindowMenu and OpenWithMenu,
	which don't have this problem.  I assume packages view does the same now too.
	But this is still a problem for SelectionProviderActions in general.
	The action has no way of knowing when to release itself.
	The view or editor which creates the action should just create a single instance,
	and unhook it when the view or editor closes.
	However, there is currently no API for doing this.
	They must do: action.getSelectionProvider().removeSelectionChangedListener(action)

NE (05/01/01 2:41:22 PM)
	Should add API:
/**
 * Disposes this selection provider action.
 * The default implementation removes this action from the selection changed listener list 
 * of its selection provider.
 * Subclasses may extend this method.
 */
public void dispose() {
	if (provider != null) {
		provider.removeSelectionChangedListener(this);
	}
}
		
NE (05/01/01 4:48:24 PM)
	Change in SelectionProviderAction 102 [ne] 01, but not released.

NE (5/3/01 2:03:08 PM)
	DG says:
This is going to make a major impact to anybody with a nontrivial UI.
A typical UI has a lot of actions, and adding code to 'dispose()' them
is going to be a nontrivial exersize. As a meta comment, I don't
like how we are introducing C++ (where you had to 'delete' everything
you new'ed).

NE (5/3/01 2:21:29 PM)
I agree with your concern re dispose(), but I don't see a better solution.
The action has no way of knowing the lifecycle of the selection provider.
Normally the client, e.g. a view, would create the action once, and unhook it when closed.
But SelectionProviderAction has no means of unhooking itself, which is why I proposed adding the dispose() method.
Otherwise, they have to do the current workaround of: action.getSelectionProvider().removeSelectionListener(action).

Do you see a better alternative?

NE (5/3/01 4:10:01 PM) 
DG replies:
I have to reluctantly agree with you that 'dispose' is the cleanest
solution. My whine was more of a meta-comment that we are proving in our
daily life that 'new/delete' scheme of C++ is more alive than was
originally thought (in the glory days of garbage collection :-).

jeem (5/4/01 8:20:00 PM) - 
Adding SelectionProviderAction.dispose() is a breaking API change, but the problem of
leakage is serious enough to justify it. It should be documented as a breaking API change
that would only break subclassing clients that happen to have already implemented their 
own public or protected dispose() method (not very likely).  The instantiation contract
for SelectionProviderAction (and its subclasses) should spell out who's responsible
for calling dispose().