MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

check for cyclic Disposable registrations for IDEA-147098

Browse Source
This commit is contained in:
Alexey Kudravtsev
2015-11-06 12:12:42 +03:00
parent a7b28ab901
commit 84d9b0e5a6
5 changed files with 76 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java
+7 -1
View File
@@ -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();
platform/util/src/com/intellij/openapi/util/Disposer.java
+6 -1
View File
@@ -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() {
platform/util/src/com/intellij/openapi/util/objectTree/ObjectNode.java
+5 -5
View File
@@ -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<T> {
ObjectNode(@NotNull ObjectTree<T> tree,
@Nullable ObjectNode<T> 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<T> {
@Override
@NonNls
public String toString() {
return "Node: " + myObject.toString();
return "Node: " + myObject;
}
Throwable getTrace() {
platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java
+18 -32
View File
@@ -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<T> {
public final void register(@NotNull T parent, @NotNull T child) {
synchronized (treeLock) {
ObjectNode<T> parentNode = getOrCreateNodeFor(parent, null);
ObjectNode<T> parentNode = getNode(parent);
if (parentNode == null) parentNode = createNodeFor(parent, null);
ObjectNode<T> childNode = getNode(child);
if (childNode == null) {
childNode = createNodeFor(child, parentNode, Disposer.isDebugMode() ? new Throwable() : null);
childNode = createNodeFor(child, parentNode);
}
else {
ObjectNode<T> oldParent = childNode.getParent();
@@ -72,38 +72,26 @@ public final class ObjectTree<T> {
}
}
myRootObjects.remove(child);
checkWasNotAddedAlready(childNode, child);
checkWasNotAddedAlready(parentNode, childNode);
parentNode.addChild(childNode);
fireRegistered(childNode.getObject());
}
}
private void checkWasNotAddedAlready(@NotNull ObjectNode<T> 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<T> childNode, @NotNull ObjectNode<T> parentNode) {
for (ObjectNode<T> 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<T> getOrCreateNodeFor(@NotNull T object, @Nullable ObjectNode<T> defaultParent) {
final ObjectNode<T> node = getNode(object);
if (node != null) return node;
return createNodeFor(object, defaultParent, Disposer.isDebugMode() ? new Throwable() : null);
}
@NotNull
private ObjectNode<T> createNodeFor(@NotNull T object, @Nullable ObjectNode<T> parentNode, @Nullable final Throwable trace) {
final ObjectNode<T> newNode = new ObjectNode<T>(this, parentNode, object, getNextModification(), trace);
private ObjectNode<T> createNodeFor(@NotNull T object, @Nullable ObjectNode<T> parentNode) {
final ObjectNode<T> newNode = new ObjectNode<T>(this, parentNode, object, getNextModification());
if (parentNode == null) {
myRootObjects.add(object);
}
@@ -125,9 +113,7 @@ public final class ObjectTree<T> {
executeUnregistered(object, action);
return true;
}
else {
return false;
}
return false;
}
node.execute(disposeTree, action);
return true;
@@ -138,7 +124,7 @@ public final class ObjectTree<T> {
@NotNull List<T> recursiveGuard,
@NotNull final ObjectTreeAction<T> action) {
synchronized (recursiveGuard) {
if (ArrayUtil.indexOf(recursiveGuard, object, Equality.IDENTITY) != -1) return;
if (ArrayUtil.indexOf(recursiveGuard, object, ContainerUtil.<T>identityStrategy()) != -1) return;
recursiveGuard.add(object);
}
@@ -147,7 +133,7 @@ public final class ObjectTree<T> {
}
finally {
synchronized (recursiveGuard) {
int i = ArrayUtil.lastIndexOf(recursiveGuard, object, Equality.IDENTITY);
int i = ArrayUtil.lastIndexOf(recursiveGuard, object, ContainerUtil.<T>identityStrategy());
assert i != -1;
recursiveGuard.remove(i);
}
@@ -216,7 +202,7 @@ public final class ObjectTree<T> {
}
}
}
@TestOnly
public boolean isEmpty() {
synchronized (treeLock) {
platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java
+40 -4
View File
@@ -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;
}
};
}
}