I have been trying to track down a memory leak in my application. Using JProfiler, I traced the issue to a reference to my Dockable from the SimpleDockAction$DockableKeyForwarder class, which stayed registered with the DefaultKeyboardController after removal. I dug some more and verified that the references were my action guards registered with the dock controller. Using the Notes example program delivered with DockFrames, I was able to replicate the problem by creating and deleting notes and verifying the NoteView was leaked with the same reference chain.
Digging in to the DockFrames code, I was able to figure out that the problem occurred during the final drag to remove the dockable from the dock station. The firing of events causes the last attempt to unbind the listeners to be ignored. The problem is that AbstractDockable.setController is being called with a null value and then the SimpleDockAction.unbound method is being called. This latter attempts to get the controller from the dockable so that it can unregister, but as this has already been set to null, the references are leaked.
I tried to hack about to change the order that things fire in AbstractDockable, but this only made things worse.
If anyone has any suggestions on how to fix or work around this, I’d appreciate it.
Uh, that is bad. I’ll fix this in the framework itself, but I have first to reproduce and see this bug myself.
Thanks for the detailed analysis.
[Edit: I’ll let the DockableKeyForwarder store the DockController to which it added its listener, that way the DockableKeyForwarder will never find a null-controller.]
I implemented your suggested fix and it reduced the number of leaked DockableKeyForwarders to 1. I was able to track down a race condition during a “Controller Changed” event where a DockableKeyForwarder was destroyed while the DockHiearchyObserver was updating all of its listeners and so still called controllerChanged on the destroyed forwarder. This resulted in the orphaned DockableKeyForwarder which re-registered with the Keyboard Controller as part of the controllerChanged method after it had unregistered as part of the destroy method. Following is my code change to DockableKeyForwarder. There is probably a better way to implement this, but I did not have time to track down the chain of events when the controller changed. However it works and I have verified AbstractDockables are no longer leaked.
/**
* Listens to all {@link KeyEvent}s concerning one {@link Dockable}.
* @author Benjamin Sigg
*
*/
private class DockableKeyForwarder implements KeyboardListener, DockHierarchyListener{
/** the element which is observed by this listener */
private Dockable dockable;
/**
* The dock controller owned by the <code>dockable</code>. It is
* cached to handle removing dockables at which point they no longer
* have a controller, but still need to be removed as keyboard
* controller listeners
*/
private DockController controller;
/**
* Flag indicating that this <code>DockableKeyForwarder</code> is in the
* process of being destroyed. It is needed to work around a race
* condition (see controllerChanged)
*/
private boolean destroyCalled = false;
/**
* Creates a new forwarder.
* @param dockable the element for which the calls will be forwarded
*/
public DockableKeyForwarder( Dockable dockable ){
this.dockable = dockable;
dockable.addDockHierarchyListener( this );
controller = dockable.getController();
if(controller != null )
controller.getKeyboardController().addListener( this );
}
public void hierarchyChanged( DockHierarchyEvent event ){
// do nothing
}
public void controllerChanged( DockHierarchyEvent event ){
// This works around a race condition where a controllerChanged
// event is received after destroy has been called. When that
// happens, without this guard, the below add will cause this to
// be orphaned
if (destroyCalled)
return;
if( event.getController() != null )
event.getController().getKeyboardController().removeListener( this );
controller = dockable.getController();
if(controller != null )
controller.getKeyboardController().addListener( this );
}
/**
* Removes all listeners added by this forwarder.
*/
public void destroy(){
destroyCalled = true;
if(controller != null )
controller.getKeyboardController().removeListener( this );
dockable.removeDockHierarchyListener( this );
}
...snip...
[QUOTE=Unregistered]I implemented your suggested fix and it reduced the number of leaked DockableKeyForwarders to 1. I was able to track down a race condition during a “Controller Changed” event where a DockableKeyForwarder was destroyed while the DockHiearchyObserver was updating all of its listeners and so still called controllerChanged on the destroyed forwarder. This resulted in the orphaned DockableKeyForwarder which re-registered with the Keyboard Controller as part of the controllerChanged method after it had unregistered as part of the destroy method. Following is my code change to DockableKeyForwarder. There is probably a better way to implement this, but I did not have time to track down the chain of events when the controller changed. However it works and I have verified AbstractDockables are no longer leaked.
[/QUOTE]
Wow, many thanks for this info.