Hi Francis,
You sync the nodeState table on each call of the setSelectionPath()
method. However, this would not cover the case where an application
calls the selection model directly.
If I understand the problem correctly, we don't have a problem here when
the selection changes, but when the actual tree structure changes, that
is, when we have entries left in the nodeStates table that shouldn't
exist anymore, and thus can't be garbage collected.
I have implemented the necessary cleanup for the nodeState table how I
think it should work. Can you try if the attached patch helps?
/Roman
> OK, slightly more elegant fix for the JTree leak. I'm not sure if this
> covers *all* possible cases where we'd need to remove an entry out of
> nodeStates, but it seems to cover the ones that I was testing for.
>
> How's this look?
>
> Thanks,
> Francis
>
> On Thu, 2006-11-02 at 22:29 +0100, Roman Kennke wrote:
> > Hi Francis,
> >
> >
> > > The attached patch fixes a memory leak in the JTree, changing a
> > > Hashtable into a WeakHashMap.
> > >
> > > I'm not too familiar with this class, so I don't know if this is the
> > > best way to fix it (or if this even breaks something!), and I'd
> > > appreciate any comments or approval for this patch.
> >
> > This would probably not break things. But using a WeakHashMap doesn't
> > seem like the best solution here. We should rather make sure that no
> > entries are left in the table. I'm thinking that in the TreeModelHandler
> > inner class we can take care of that cleanly without the need to use
> > WeakHashMap.
> >
> > /Roman
> >
> >
Index: javax/swing/JTree.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/JTree.java,v
retrieving revision 1.77
diff -u -1 -5 -r1.77 JTree.java
--- javax/swing/JTree.java 18 Oct 2006 18:46:25 -0000 1.77
+++ javax/swing/JTree.java 6 Nov 2006 15:47:35 -0000
@@ -1218,46 +1218,99 @@
*/
public void treeNodesInserted(TreeModelEvent ev)
{
// nothing to do here
}
/**
* Notifies when a node is removed from the tree.
*
* This method is called after the actual change occured.
*
* @param ev the TreeModelEvent describing the change
*/
public void treeNodesRemoved(TreeModelEvent ev)
{
- // TODO: The API docs suggest that this method should do something
- // but I cannot really see what has to be done here ...
+ if (ev != null)
+ {
+ TreePath parent = ev.getTreePath();
+ Object[] children = ev.getChildren();
+ TreeSelectionModel sm = getSelectionModel();
+ if (children != null)
+ {
+ TreePath path;
+ Vector toRemove = new Vector();
+ // Collect items that we must remove.
+ for (int i = children.length - 1; i >= 0; i--)
+ {
+ path = parent.pathByAddingChild(children[i]);
+ if (nodeStates.containsKey(path))
+ toRemove.add(path);
+ // Clear selection while we are at it.
+ if (sm != null)
+ removeDescendantSelectedPaths(path, true);
+ }
+ if (toRemove.size() > 0)
+ removeDescendantToggledPaths(toRemove.elements());
+ TreeModel model = getModel();
+ if (model == null || model.isLeaf(parent.getLastPathComponent()))
+ nodeStates.remove(parent);
+ }
+ }
}
/**
* Notifies when the structure of the tree is changed.
*
* This method is called after the actual change occured.
*
* @param ev the TreeModelEvent describing the change
*/
public void treeStructureChanged(TreeModelEvent ev)
{
- // Set state of new path.
- TreePath path = ev.getTreePath();
- setExpandedState(path, isExpanded(path));
+ if (ev != null)
+ {
+ TreePath parent = ev.getTreePath();
+ if (parent != null)
+ {
+ if (parent.getPathCount() == 1)
+ {
+ // We have a new root, clear everything.
+ clearToggledPaths();
+ Object root = treeModel.getRoot();
+ if (root != null && treeModel.isLeaf(root))
+ nodeStates.put(parent, Boolean.TRUE);
+ }
+ else if (nodeStates.containsKey(parent))
+ {
+ Vector toRemove = new Vector();
+ boolean expanded = isExpanded(parent);
+ toRemove.add(parent);
+ removeDescendantToggledPaths(toRemove.elements());
+ if (expanded)
+ {
+ TreeModel model = getModel();
+ if (model != null
+ || model.isLeaf(parent.getLastPathComponent()))
+ collapsePath(parent);
+ else
+ nodeStates.put(parent, Boolean.TRUE);
+ }
+ }
+ removeDescendantSelectedPaths(parent, false);
+ }
+ }
}
}
/**
* This redirects TreeSelectionEvents and rewrites the source of it to be
* this JTree. This is typically done when the tree model generates an
* event, but the JTree object associated with that model should be listed
* as the actual source of the event.
*/
protected class TreeSelectionRedirector implements TreeSelectionListener,
Serializable
{
/** The serial version UID. */
private static final long serialVersionUID = -3505069663646241664L;
@@ -1387,32 +1440,34 @@
private static final Object EXPANDED = Boolean.TRUE;
private static final Object COLLAPSED = Boolean.FALSE;
private boolean dragEnabled;
private boolean expandsSelectedPaths;
private TreePath anchorSelectionPath;
/**
* This contains the state of all nodes in the tree. Al/ entries map the
* TreePath of a note to to its state. Valid states are EXPANDED and
* COLLAPSED. Nodes not in this Hashtable are assumed state COLLAPSED.
+ *
+ * This is package private to avoid accessor methods.
*/
- private Hashtable nodeStates = new Hashtable();
+ Hashtable nodeStates = new Hashtable();
protected transient TreeCellEditor cellEditor;
protected transient TreeCellRenderer cellRenderer;
protected boolean editable;
protected boolean invokesStopCellEditing;
protected boolean largeModel;
protected boolean rootVisible;
protected int rowHeight;
