diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java index 734c8e86ac..1ad5371cec 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeProvider.java @@ -150,10 +150,7 @@ public class SymbolTreeProvider extends ComponentProviderAdapter { private SymbolGTree createTree(SymbolTreeRootNode rootNode) { if (tree != null) { - GTreeNode oldRootNode = tree.getModelRoot(); tree.setRootNode(rootNode); - - oldRootNode.removeAll();// assist in cleanup a bit return tree; } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java index f4f7d4790a..e85baaf74f 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/CoreGTreeNode.java @@ -253,6 +253,10 @@ abstract class CoreGTreeNode implements Cloneable { } public void dispose() { + disconnect(true); + } + + void disconnect(boolean dispose) { List oldChildren; synchronized (this) { oldChildren = children; @@ -262,7 +266,10 @@ abstract class CoreGTreeNode implements Cloneable { if (oldChildren != null) { for (GTreeNode node : oldChildren) { - node.dispose(); + node.disconnect(dispose); + if (dispose) { + node.dispose(); + } } oldChildren.clear(); } @@ -270,23 +277,13 @@ abstract class CoreGTreeNode implements Cloneable { /** * This is used to dispose filtered "clone" nodes. When a filter is applied to the tree, - * the nodes that matched are "shallow" cloned, so when the filter is removed, we don't - * want to do a full dispose on the nodes, just clean up the parent-child references. + * the nodes that matched are "shallow" cloned. This is effectively a shallow dispose that will + * clean up any children and disconnect this node from the true, but will *not* call dispose(), + * which would affect the original node from which this node was cloned. */ - final void disposeClones() { - List oldChildren; - synchronized (this) { - oldChildren = children; - children = null; - parent = null; - } - - if (oldChildren != null) { - for (GTreeNode node : oldChildren) { - node.disposeClones(); - } - oldChildren.clear(); - } + final void disposeClone() { + // Do not dispose, as that will affect the clone's source too + disconnect(false); } /** diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java index 054c03cbb8..f02f630aec 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTree.java @@ -285,12 +285,12 @@ public class GTree extends JPanel implements BusyListener { realModelRootNode.dispose(); } - // if there is a filter applied, clean up the filtered nodes. Note that filtered nodes + // If there is a filter applied, clean up the filtered nodes. Note that filtered nodes // are expected to be shallow clones of the model nodes, so we don't want to call full // dispose on the filtered nodes because internal clean-up should happen when the - // model nodes are disposed. The disposeClones just breaks the child-parent ties. + // model nodes are disposed. The disposeClone() just breaks the child-parent ties. if (realViewRootNode != null && realViewRootNode != realModelRootNode) { - realViewRootNode.disposeClones(); + realViewRootNode.disposeClone(); } filterProvider.dispose(); @@ -847,23 +847,40 @@ public class GTree extends JPanel implements BusyListener { Swing.runIfSwingOrRunLater(() -> { worker.clearAllJobs(); rootNode.setParent(rootParent); + GTreeNode oldModelRoot = realModelRootNode; + GTreeNode oldViewRoot = realViewRootNode; realModelRootNode = rootNode; realViewRootNode = rootNode; - GTreeNode oldRoot; - oldRoot = swingSetModelRootNode(rootNode); - oldRoot.dispose(); + swingSetModelRootNode(rootNode); + + disposeOldRoots(oldModelRoot, oldViewRoot); + if (filter != null) { filterUpdateManager.update(); } }); } + private void disposeOldRoots(GTreeNode oldModelRoot, GTreeNode oldViewRoot) { + if (oldModelRoot != null && oldModelRoot != realModelRootNode) { + oldModelRoot.dispose(); + } + + // If there is a filter applied, clean up the filtered nodes. Note that filtered nodes + // are expected to be shallow clones of the model nodes, so we don't want to call full + // dispose on the filtered nodes because internal clean-up should happen when the + // model nodes are disposed. The disposeClone() just breaks the child-parent ties. + if (oldViewRoot != null && oldViewRoot != realViewRootNode) { + oldViewRoot.disposeClone(); // safe to call even if we disposed the same node above + } + } + void swingSetFilteredRootNode(GTreeNode filteredRootNode) { filteredRootNode.setParent(rootParent); realViewRootNode = filteredRootNode; GTreeNode currentRoot = swingSetModelRootNode(filteredRootNode); if (currentRoot != realModelRootNode) { - currentRoot.disposeClones(); + currentRoot.disposeClone(); } } @@ -871,7 +888,7 @@ public class GTree extends JPanel implements BusyListener { realViewRootNode = realModelRootNode; GTreeNode currentRoot = swingSetModelRootNode(realModelRootNode); if (currentRoot != realModelRootNode && currentRoot != null) { - currentRoot.disposeClones(); + currentRoot.disposeClone(); } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeRootParentNode.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeRootParentNode.java index 4858f28f79..02742b7a78 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeRootParentNode.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/GTreeRootParentNode.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -53,4 +53,9 @@ class GTreeRootParentNode extends GTreeNode { public boolean isLeaf() { return false; } + + @Override + public String toString() { + return "GTree Root Parent Node"; + } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java index 99f499119e..542eceecd9 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/internal/GTreeModel.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -125,20 +125,25 @@ public class GTreeModel implements TreeModel { "GTreeModel.fireNodeStructuredChanged() must be " + "called from the AWT thread"); // If the tree is filtered and this is called on the original node, we have to - // translate the node to a view node (one the jtree knows). + // translate the node to a view node (one the tree knows). GTreeNode viewNode = convertToViewNode(changedNode); if (viewNode == null) { return; } if (viewNode != changedNode) { - // This means we are filtered and since the original node's children are invalid, - // then the filtered children are invalid also. So clear out the children by - // setting an empty list as we don't want to trigger the node to regenerate its - // children which happens if you set the children to null. + // The only time this can happen is when the tree is filtered. In this case, the + // view node will be a clone of the changed node. We need to update the cloned + // node to signal that the children have changed. If we set the children to null, + // then they will get reloaded when we fire the event. Instead, if we set the + // children to the empty list, the node will simply think there are no children and + // it will not get reloaded. // - // This won't cause a second event to the jtree because we are protected - // by the isFiringNodeStructureChanged variable + // After the events have been fired, there will eventually be a refilter operation + // to update the view node with the correct children for the active filter. + // + // Note: this won't cause a second event to the tree because we are protected by + // the isFiringNodeStructureChanged flag at the top of this method. viewNode.setChildren(Collections.emptyList()); }