diff --git a/platform/util/src/com/intellij/openapi/util/Disposer.java b/platform/util/src/com/intellij/openapi/util/Disposer.java index 68e699ac9556..bc79e86dae08 100644 --- a/platform/util/src/com/intellij/openapi/util/Disposer.java +++ b/platform/util/src/com/intellij/openapi/util/Disposer.java @@ -129,10 +129,6 @@ public class Disposer { ourTree.executeAll(disposable, ourDisposeAction, processUnregistered); } - public static void disposeChildAndReplace(@NotNull Disposable toDispose, @NotNull Disposable toReplace) { - ourTree.executeChildAndReplace(toDispose, toReplace, ourDisposeAction); - } - @NotNull public static ObjectTree getTree() { return ourTree; diff --git a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java index abf485e868f3..bd3843b2f926 100644 --- a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java +++ b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java @@ -103,7 +103,7 @@ final class ObjectNode { } } - void execute(@NotNull final ObjectTreeAction action) { + void execute(@NotNull final ObjectTreeAction action, @NotNull final List exceptions) { ObjectTree.executeActionWithRecursiveGuard(this, myTree.getNodesInExecution(), new ObjectTreeAction>() { @Override public void execute(@NotNull ObjectNode each) { @@ -117,23 +117,22 @@ final class ObjectNode { ObjectNode[] childrenArray; synchronized (myTree.treeLock) { childrenArray = getChildrenArray(); + myChildren = null; } - List exceptions = new SmartList(); - for (int i = childrenArray.length - 1; i >= 0; i--) { try { - childrenArray[i].execute(action); + ObjectNode childNode = childrenArray[i]; + childNode.execute(action, exceptions); + synchronized (myTree.treeLock) { + childNode.myParent = null; + } } catch (Throwable e) { exceptions.add(e); } } - synchronized (myTree.treeLock) { - myChildren = null; - } - try { action.execute(myObject); myTree.fireExecuted(myObject); @@ -141,15 +140,13 @@ final class ObjectNode { catch (Throwable e) { exceptions.add(e); } - - remove(); + removeFromObjectTree(); handleExceptions(exceptions); } @Override public void beforeTreeExecution(@NotNull ObjectNode parent) { - } }); } @@ -166,18 +163,16 @@ final class ObjectNode { if (pce != null) { throw pce; } + exceptions.clear(); } } - private void remove() { + private void removeFromObjectTree() { synchronized (myTree.treeLock) { myTree.putNode(myObject, null); if (myParent == null) { myTree.removeRootObject(myObject); } - else { - myParent.removeChild(this); - } } } diff --git a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java index ec1f92f237c5..46ce75b15358 100644 --- a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java +++ b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java @@ -20,6 +20,7 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Disposer; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -38,10 +39,11 @@ public final class ObjectTree { // identity used here to prevent problems with hashCode/equals overridden by not very bright minds private final Set myRootObjects = ContainerUtil.newIdentityTroveSet(); // guarded by treeLock private final Map> myObject2NodeMap = ContainerUtil.newIdentityTroveMap(); // guarded by treeLock - private final Map myDisposedObjects = ContainerUtil.createWeakMap(100, 0.5f, ContainerUtil.identityStrategy()); // guarded by treeLock + // Disposable to trace or boolean marker (if trace unavailable) + private final Map myDisposedObjects = ContainerUtil.createWeakMap(100, 0.5f, ContainerUtil.identityStrategy()); // guarded by treeLock private final List> myExecutedNodes = new ArrayList>(); // guarded by myExecutedNodes - private final List myExecutedUnregisteredNodes = new ArrayList(); // guarded by myExecutedUnregisteredNodes + private final List myExecutedUnregisteredObjects = new ArrayList(); // guarded by myExecutedUnregisteredObjects final Object treeLock = new Object(); @@ -103,9 +105,9 @@ public final class ObjectTree { } } - public Object getDisposalInfo(@NotNull T parent) { + public Object getDisposalInfo(@NotNull T object) { synchronized (treeLock) { - return myDisposedObjects.get(parent); + return myDisposedObjects.get(object); } } @@ -148,7 +150,16 @@ public final class ObjectTree { } } else { - node.execute(action); + ObjectNode parent; + synchronized (treeLock) { + parent = node.getParent(); + } + node.execute(action, new SmartList()); + if (parent != null) { + synchronized (treeLock) { + parent.removeChild(node); + } + } } } finally { @@ -169,10 +180,10 @@ public final class ObjectTree { return false; } - @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter") static void executeActionWithRecursiveGuard(@NotNull T object, @NotNull List recursiveGuard, @NotNull final ObjectTreeAction action) { + //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized (recursiveGuard) { if (ArrayUtil.indexOf(recursiveGuard, object, ContainerUtil.identityStrategy()) != -1) return; recursiveGuard.add(object); @@ -182,6 +193,7 @@ public final class ObjectTree { action.execute(object); } finally { + //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized (recursiveGuard) { int i = ArrayUtil.lastIndexOf(recursiveGuard, object, ContainerUtil.identityStrategy()); assert i != -1; @@ -191,39 +203,16 @@ public final class ObjectTree { } private void executeUnregistered(@NotNull final T object, @NotNull final ObjectTreeAction action) { - executeActionWithRecursiveGuard(object, myExecutedUnregisteredNodes, action); - } - - public final void executeChildAndReplace(@NotNull T toExecute, - @NotNull T toReplace, - @NotNull ObjectTreeAction action) { - final ObjectNode toExecuteNode; - T parentObject; - synchronized (treeLock) { - toExecuteNode = getNode(toExecute); - if (toExecuteNode == null) throw new IllegalArgumentException("Object " + toExecute + " wasn't registered or already disposed"); - - final ObjectNode parent = toExecuteNode.getParent(); - if (parent == null) throw new IllegalArgumentException("Object " + toExecute + " is not connected to the tree - doesn't have parent"); - parentObject = parent.getObject(); - } - - toExecuteNode.execute(action); - register(parentObject, toReplace); - } - - public boolean containsKey(@NotNull T object) { - synchronized (treeLock) { - return getNode(object) != null; - } + executeActionWithRecursiveGuard(object, myExecutedUnregisteredObjects, action); } @TestOnly - // public for Upsource - public void assertNoReferenceKeptInTree(@NotNull T disposable) { + void assertNoReferenceKeptInTree(@NotNull T disposable) { synchronized (treeLock) { - Collection> nodes = myObject2NodeMap.values(); - for (ObjectNode node : nodes) { + for (Map.Entry> entry : myObject2NodeMap.entrySet()) { + T key = entry.getKey(); + assert key != disposable; + ObjectNode node = entry.getValue(); node.assertNoReferencesKept(disposable); } } @@ -233,7 +222,6 @@ public final class ObjectTree { myRootObjects.remove(object); } - @SuppressWarnings({"HardCodedStringLiteral"}) public void assertIsEmpty(boolean throwError) { synchronized (treeLock) { for (T object : myRootObjects) { @@ -277,20 +265,20 @@ public final class ObjectTree { myListeners.remove(listener); } - private void fireRegistered(@NotNull Object object) { + private void fireRegistered(@NotNull T object) { for (ObjectTreeListener each : myListeners) { each.objectRegistered(object); } } - void fireExecuted(@NotNull Object object) { + void fireExecuted(@NotNull T object) { for (ObjectTreeListener each : myListeners) { each.objectExecuted(object); } rememberDisposedTrace(object); } - private void rememberDisposedTrace(@NotNull Object object) { + private void rememberDisposedTrace(@NotNull T object) { synchronized (treeLock) { Throwable trace = ourTopmostDisposeTrace.get(); myDisposedObjects.put(object, trace != null ? trace : Boolean.TRUE); diff --git a/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java b/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java index f413398463c0..129c0112d64e 100644 --- a/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java +++ b/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java @@ -24,6 +24,7 @@ import junit.framework.TestCase; import org.jetbrains.annotations.NonNls; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import static com.intellij.openapi.util.Disposer.newDisposable; @@ -90,17 +91,11 @@ public class DisposerTest extends TestCase { Disposer.dispose(myRoot); - List expected = new ArrayList<>(); - expected.add(myFolder2); - expected.add(myLeaf1); - expected.add(myFolder1); - expected.add(myRoot); - - assertEquals(expected, myDisposedObjects); + assertEquals(Arrays.asList(myFolder2, myLeaf1, myFolder1, myRoot), myDisposedObjects); } public void testDirectCallOfDisposable() { - SelDisposable selfDisposable = new SelDisposable("root"); + SelDisposable selfDisposable = new SelDisposable("selfDisposable"); Disposer.register(myRoot, selfDisposable); Disposer.register(selfDisposable, myFolder1); Disposer.register(myFolder1, myFolder2); @@ -121,17 +116,6 @@ public class DisposerTest extends TestCase { selfDisposable.dispose(); } - public void testDisposeAndReplace() { - Disposer.register(myRoot, myFolder1); - - Disposer.disposeChildAndReplace(myFolder1, myFolder2); - assertDisposed(myFolder1); - - Disposer.dispose(myRoot); - assertDisposed(myRoot); - assertDisposed(myFolder2); - } - public void testPostponedParentRegistration() { Disposer.register(myFolder1, myLeaf1); Disposer.register(myLeaf1, myLeaf2); @@ -212,10 +196,8 @@ public class DisposerTest extends TestCase { assertEquals(toString(expected), toString(myDisposeActions)); } - private void assertDisposed(MyDisposable disposable) { assertTrue(disposable.isDisposed()); - assertFalse(disposable.toString(), Disposer.getTree().containsKey(disposable)); Disposer.getTree().assertNoReferenceKeptInTree(disposable); } @@ -239,6 +221,7 @@ public class DisposerTest extends TestCase { return myDisposed; } + @Override public String toString() { return myName; }