diff --git a/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java b/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java index 0cbe7c600eee..3d5a24c1f03f 100644 --- a/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java @@ -127,6 +127,8 @@ public abstract class UsefulTestCase extends TestCase { } } + private boolean oldDisposerDebug; + protected boolean shouldContainTempFiles() { return true; } @@ -142,11 +144,15 @@ public abstract class UsefulTestCase extends TestCase { myTempDir = new File(ORIGINAL_TEMP_DIR, TEMP_DIR_MARKER + testName).getPath(); FileUtil.resetCanonicalTempPathCache(myTempDir); } - ApplicationInfoImpl.setInPerformanceTest(isPerformanceTest()); + boolean isPerformanceTest = isPerformanceTest(); + ApplicationInfoImpl.setInPerformanceTest(isPerformanceTest); + // turn off Disposer debugging for performance tests + oldDisposerDebug = Disposer.setDebugMode(Disposer.isDebugMode() && !isPerformanceTest); } @Override protected void tearDown() throws Exception { + Disposer.setDebugMode(oldDisposerDebug); try { Disposer.dispose(myTestRootDisposable); cleanupSwingDataStructures(); diff --git a/platform/util/src/com/intellij/openapi/util/Disposer.java b/platform/util/src/com/intellij/openapi/util/Disposer.java index 6307017967ac..4d597880f38d 100644 --- a/platform/util/src/com/intellij/openapi/util/Disposer.java +++ b/platform/util/src/com/intellij/openapi/util/Disposer.java @@ -23,6 +23,7 @@ import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; import java.lang.reflect.Field; import java.lang.reflect.Modifier; @@ -126,12 +127,16 @@ public class Disposer { } } + @TestOnly public static boolean isEmpty() { return ourDebugMode && ourTree.isEmpty(); } - public static void setDebugMode(final boolean debugMode) { + // returns old value + public static boolean setDebugMode(final boolean debugMode) { + boolean oldValue = ourDebugMode; ourDebugMode = debugMode; + return oldValue; } public static boolean isDebugMode() { 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 5ce40516f200..4f356c6d8801 100644 --- a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java +++ b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2015 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package com.intellij.openapi.util.objectTree; import com.intellij.openapi.Disposable; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProcessCanceledException; +import com.intellij.openapi.util.Disposer; import com.intellij.util.SmartList; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -46,13 +47,12 @@ final class ObjectNode { ObjectNode(@NotNull ObjectTree tree, @Nullable ObjectNode parentNode, @NotNull T object, - long modification, - @Nullable final Throwable trace) { + long modification) { myTree = tree; myParent = parentNode; myObject = object; - myTrace = trace; + myTrace = Disposer.isDebugMode() ? new Throwable() : null; myOwnModification = modification; } @@ -171,7 +171,7 @@ final class ObjectNode { @Override @NonNls public String toString() { - return "Node: " + myObject.toString(); + return "Node: " + myObject; } Throwable getTrace() { 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 11212543a1d2..6340aafb237a 100644 --- a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java +++ b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2015 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,10 +17,9 @@ package com.intellij.openapi.util.objectTree; import com.intellij.openapi.Disposable; 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.containers.ContainerUtil; -import gnu.trove.Equality; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -59,11 +58,12 @@ public final class ObjectTree { public final void register(@NotNull T parent, @NotNull T child) { synchronized (treeLock) { - ObjectNode parentNode = getOrCreateNodeFor(parent, null); + ObjectNode parentNode = getNode(parent); + if (parentNode == null) parentNode = createNodeFor(parent, null); ObjectNode childNode = getNode(child); if (childNode == null) { - childNode = createNodeFor(child, parentNode, Disposer.isDebugMode() ? new Throwable() : null); + childNode = createNodeFor(child, parentNode); } else { ObjectNode oldParent = childNode.getParent(); @@ -72,38 +72,26 @@ public final class ObjectTree { } } myRootObjects.remove(child); - checkWasNotAddedAlready(childNode, child); + + checkWasNotAddedAlready(parentNode, childNode); + parentNode.addChild(childNode); fireRegistered(childNode.getObject()); } } - private void checkWasNotAddedAlready(@NotNull ObjectNode childNode, @NotNull T child) { - ObjectNode parent = childNode.getParent(); - boolean childIsInTree = parent != null; - if (!childIsInTree) return; - - while (parent != null) { - if (parent.getObject() == child) { - LOG.error(child + " was already added as a child of: " + parent); + private void checkWasNotAddedAlready(ObjectNode childNode, @NotNull ObjectNode parentNode) { + for (ObjectNode node = childNode; node != null; node = node.getParent()) { + if (node == parentNode) { + throw new IncorrectOperationException("'"+childNode.getObject() + "' was already added as a child of '" + parentNode.getObject()+"'"); } - parent = parent.getParent(); } } @NotNull - private ObjectNode getOrCreateNodeFor(@NotNull T object, @Nullable ObjectNode defaultParent) { - final ObjectNode node = getNode(object); - - if (node != null) return node; - - return createNodeFor(object, defaultParent, Disposer.isDebugMode() ? new Throwable() : null); - } - - @NotNull - private ObjectNode createNodeFor(@NotNull T object, @Nullable ObjectNode parentNode, @Nullable final Throwable trace) { - final ObjectNode newNode = new ObjectNode(this, parentNode, object, getNextModification(), trace); + private ObjectNode createNodeFor(@NotNull T object, @Nullable ObjectNode parentNode) { + final ObjectNode newNode = new ObjectNode(this, parentNode, object, getNextModification()); if (parentNode == null) { myRootObjects.add(object); } @@ -125,9 +113,7 @@ public final class ObjectTree { executeUnregistered(object, action); return true; } - else { - return false; - } + return false; } node.execute(disposeTree, action); return true; @@ -138,7 +124,7 @@ public final class ObjectTree { @NotNull List recursiveGuard, @NotNull final ObjectTreeAction action) { synchronized (recursiveGuard) { - if (ArrayUtil.indexOf(recursiveGuard, object, Equality.IDENTITY) != -1) return; + if (ArrayUtil.indexOf(recursiveGuard, object, ContainerUtil.identityStrategy()) != -1) return; recursiveGuard.add(object); } @@ -147,7 +133,7 @@ public final class ObjectTree { } finally { synchronized (recursiveGuard) { - int i = ArrayUtil.lastIndexOf(recursiveGuard, object, Equality.IDENTITY); + int i = ArrayUtil.lastIndexOf(recursiveGuard, object, ContainerUtil.identityStrategy()); assert i != -1; recursiveGuard.remove(i); } @@ -216,7 +202,7 @@ public final class ObjectTree { } } } - + @TestOnly public boolean isEmpty() { synchronized (treeLock) { 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 ce4132496aea..84aeea82e008 100644 --- a/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java +++ b/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java @@ -17,6 +17,7 @@ package com.intellij.openapi.util.objectTree; import com.intellij.openapi.Disposable; import com.intellij.openapi.util.Disposer; +import com.intellij.util.IncorrectOperationException; import junit.framework.TestCase; import org.jetbrains.annotations.NonNls; @@ -216,11 +217,10 @@ public class DisposerTest extends TestCase { } private class MyDisposable implements Disposable { - - private boolean myDisposed = false; + private boolean myDisposed; protected String myName; - public MyDisposable(@NonNls String aName) { + MyDisposable(@NonNls String aName) { myName = aName; } @@ -252,7 +252,7 @@ public class DisposerTest extends TestCase { } private class SelDisposable extends MyDisposable { - public SelDisposable(@NonNls String aName) { + private SelDisposable(@NonNls String aName) { super(aName); } @@ -276,4 +276,40 @@ public class DisposerTest extends TestCase { return result.toString(); } + + public void testIncest() { + Disposable parent = newDisposable("parent"); + Disposable child = newDisposable("child"); + Disposer.register(parent, child); + + Disposable grand = newDisposable("grand"); + Disposer.register(child, grand); + + try { + Disposer.register(grand, parent); + fail("must not allow"); + } + catch (IncorrectOperationException e) { + assertEquals("'grand' was already added as a child of 'parent'", e.getMessage()); + } + finally { + Disposer.dispose(grand); + Disposer.dispose(child); + Disposer.dispose(parent); + } + } + + private static Disposable newDisposable(final String name) { + return new Disposable() { + @Override + public void dispose() { + + } + + @Override + public String toString() { + return name; + } + }; + } }