}
public static boolean isDisposing(@NotNull Disposable disposable) {
- return ourTree.isDisposing(disposable);
+ return isDisposed(disposable);
}
public static Disposable get(@NotNull String key) {
package com.intellij.openapi.util;
import com.intellij.openapi.Disposable;
-import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.util.objectTree.ThrowableInterner;
import com.intellij.util.SmartList;
import org.jetbrains.annotations.NonNls;
import java.util.List;
final class ObjectNode {
- private static final ObjectNode[] EMPTY_ARRAY = new ObjectNode[0];
-
- private static final Logger LOG = Logger.getInstance(ObjectNode.class);
-
private final ObjectTree myTree;
ObjectNode myParent; // guarded by myTree.treeLock
}
}
- @Nullable
- List<Throwable> executeChildren() {
- return myTree.executeActionWithRecursiveGuard(myObject, () -> {
- if (myTree.getDisposalInfo(myObject) != null) {
- return null; // already disposed. may happen when someone does `register(obj, ()->Disposer.dispose(t));` abomination
- }
-
- ObjectNode[] children = getAndClearChildren();
- List<Throwable> result = null;
-
- for (int i = children.length - 1; i >= 0; i--) {
- try {
- ObjectNode childNode = children[i];
- result = childNode.execute(result);
- }
- catch (Throwable e) {
- if (result == null) result = new SmartList<>();
- result.add(e);
- }
+ void getAndRemoveRecursively(@NotNull List<? super Disposable> result) {
+ if (myChildren != null) {
+ for (int i = myChildren.size() - 1; i >= 0; i--) {
+ ObjectNode childNode = myChildren.get(i);
+ childNode.getAndRemoveRecursively(result);
}
-
- return result;
- });
+ }
+ myTree.removeObjectFromTree(this);
+ // already disposed. may happen when someone does `register(obj, ()->Disposer.dispose(t));` abomination
+ if (myTree.rememberDisposedTrace(myObject) == null) {
+ result.add(myObject);
+ }
+ myChildren = null;
+ myParent = null;
}
-
- @Nullable
- List<Throwable> execute(@Nullable List<Throwable> exceptions) {
- return myTree.executeActionWithRecursiveGuard(myObject, () -> {
- List<Throwable> result = exceptions;
- if (myTree.rememberDisposedTrace(myObject) != null) {
- return result; // already disposed. may happen when someone does `register(obj, ()->Disposer.dispose(t));` abomination
- }
-
- if (myObject instanceof Disposable.Parent) {
- try {
- ((Disposable.Parent)myObject).beforeTreeDispose();
- }
- catch (Throwable t) {
- LOG.error(t);
- }
- }
-
- ObjectNode[] children = getAndClearChildren();
-
- for (int i = children.length - 1; i >= 0; i--) {
- try {
- ObjectNode childNode = children[i];
- result = childNode.execute(result);
- }
- catch (Throwable e) {
- if (result == null) result = new SmartList<>();
- result.add(e);
- }
- }
-
- try {
- //noinspection SSBasedInspection
- myObject.dispose();
- }
- catch (Throwable e) {
- if (result == null) result = new SmartList<>();
- result.add(e);
+ void getAndRemoveChildrenRecursively(@NotNull List<? super Disposable> result) {
+ if (myChildren != null) {
+ for (int i = myChildren.size() - 1; i >= 0; i--) {
+ ObjectNode childNode = myChildren.get(i);
+ childNode.getAndRemoveRecursively(result);
}
- myTree.removeObjectFromTree(this);
- return result;
- });
- }
-
- private ObjectNode @NotNull [] getAndClearChildren() {
- synchronized (myTree.treeLock) {
- List<ObjectNode> children = myChildren;
- myChildren = null;
- return children == null || children.isEmpty() ? EMPTY_ARRAY : children.toArray(EMPTY_ARRAY);
}
}
import com.intellij.openapi.progress.ProcessCanceledException;
import com.intellij.openapi.util.objectTree.ThrowableInterner;
import com.intellij.util.IncorrectOperationException;
+import com.intellij.util.SmartList;
import com.intellij.util.containers.ContainerUtil;
import it.unimi.dsi.fastutil.objects.Reference2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet;
// Disposable -> trace or boolean marker (if trace unavailable)
private final Map<Disposable, Object> myDisposedObjects = ContainerUtil.createWeakMap(100, 0.5f, ContainerUtil.identityStrategy()); // guarded by treeLock
- // disposables for which Disposer.dispose() is currently running
- final List<Disposable> myObjectsBeingDisposed = new ArrayList<>(); // guarded by myObjectsBeingDisposed
-
final Object treeLock = new Object();
private ObjectNode getNode(@NotNull Disposable object) {
return newNode;
}
- private static void runWithTrace(@NotNull Supplier<? extends List<Throwable>> action) {
+ private void runWithTrace(@NotNull Supplier<? extends List<Disposable>> removeFromTreeAction) {
boolean needTrace = Disposer.isDebugMode() && ourTopmostDisposeTrace.get() == null;
if (needTrace) {
ourTopmostDisposeTrace.set(ThrowableInterner.intern(new Throwable()));
}
- try {
- List<Throwable> exceptions = action.get();
- if (exceptions != null) {
- handleExceptions(exceptions);
+ // first, atomically remove disposables from the tree to avoid "register during dispose" race conditions
+ List<Disposable> disposables;
+ synchronized (treeLock) {
+ disposables = removeFromTreeAction.get();
+ }
+
+ // second, call "beforeTreeDispose" in pre-order (some clients are hardcoded to see parents-then-children order in "beforeTreeDispose")
+ List<Throwable> exceptions = null;
+ for (int i = disposables.size() - 1; i >= 0; i--) {
+ Disposable disposable = disposables.get(i);
+ if (disposable instanceof Disposable.Parent) {
+ try {
+ ((Disposable.Parent)disposable).beforeTreeDispose();
+ }
+ catch (Throwable t) {
+ if (exceptions == null) exceptions = new SmartList<>();
+ exceptions.add(t);
+ }
}
}
- finally {
- if (needTrace) {
- ourTopmostDisposeTrace.remove();
+
+ // third, dispose in post-order (bottom-up)
+ for (Disposable disposable : disposables) {
+ try {
+ //noinspection SSBasedInspection
+ disposable.dispose();
+ }
+ catch (Throwable e) {
+ if (exceptions == null) exceptions = new SmartList<>();
+ exceptions.add(e);
}
}
+
+ if (needTrace) {
+ ourTopmostDisposeTrace.remove();
+ }
+ if (exceptions != null) {
+ handleExceptions(exceptions);
+ }
}
void executeAllChildren(@NotNull Disposable object) {
- ObjectNode node;
- synchronized (treeLock) {
- node = getNode(object);
+ runWithTrace(() -> {
+ ObjectNode node = getNode(object);
if (node == null) {
- return;
+ return Collections.emptyList();
}
- }
- runWithTrace(() -> node.executeChildren());
+ List<Disposable> disposables = new ArrayList<>();
+ node.getAndRemoveChildrenRecursively(disposables);
+ return disposables;
+ });
}
void executeAll(@NotNull Disposable object, boolean processUnregistered) {
- ObjectNode node;
- synchronized (treeLock) {
- node = getNode(object);
+ runWithTrace(() -> {
+ ObjectNode node = getNode(object);
if (node == null && !processUnregistered) {
- return;
+ return Collections.emptyList();
}
- }
- runWithTrace(() -> {
+ List<Disposable> disposables = new ArrayList<>();
if (node == null) {
- rememberDisposedTrace(object);
- return executeUnregistered(object);
+ if (rememberDisposedTrace(object) == null) {
+ disposables.add(object);
+ }
+ }
+ else {
+ node.getAndRemoveRecursively(disposables);
}
- return node.execute(null);
+ return disposables;
});
}
}
}
- boolean isDisposing(@NotNull Disposable disposable) {
- synchronized (myObjectsBeingDisposed) {
- if (ContainerUtil.indexOfIdentity(myObjectsBeingDisposed, disposable) != -1) {
- return true;
- }
- }
- return false;
- }
-
- List<Throwable> executeActionWithRecursiveGuard(@NotNull Disposable object, @NotNull Supplier<? extends List<Throwable>> action) {
- //noinspection SynchronizationOnLocalVariableOrMethodParameter
- synchronized (myObjectsBeingDisposed) {
- int i = ContainerUtil.lastIndexOfIdentity(myObjectsBeingDisposed, object);
- if (i != -1) {
- return null;
- }
- myObjectsBeingDisposed.add(object);
- }
-
- try {
- return action.get();
- }
- finally {
- //noinspection SynchronizationOnLocalVariableOrMethodParameter
- synchronized (myObjectsBeingDisposed) {
- int i = ContainerUtil.lastIndexOfIdentity(myObjectsBeingDisposed, object);
- assert i != -1;
- myObjectsBeingDisposed.remove(i);
- }
- }
- }
-
- private List<Throwable> executeUnregistered(@NotNull Disposable disposable) {
- return executeActionWithRecursiveGuard(disposable, ()-> {
- try {
- //noinspection SSBasedInspection
- disposable.dispose();
- return null;
- }
- catch (Throwable e) {
- return Collections.singletonList(e);
- }
- });
- }
-
@TestOnly
void assertNoReferenceKeptInTree(@NotNull Disposable disposable) {
synchronized (treeLock) {
import com.intellij.openapi.diagnostic.DefaultLogger;
import com.intellij.openapi.progress.ProcessCanceledException;
import com.intellij.openapi.util.text.StringUtil;
+import com.intellij.testFramework.LeakHunter;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.concurrency.SequentialTaskExecutor;
import junit.framework.TestCase;
assertDisposed(selfDisposable);
assertDisposed(myFolder1);
assertDisposed(myFolder2);
-
- assertEquals(0, Disposer.getTree().myObjectsBeingDisposed.size());
}
public void testDirectCallOfUnregisteredSelfDisposable() {
assertFalse(future.isDone());
allowToContinueDispose.set(true);
future.get();
- assertFalse(Disposer.isDisposing(disposable));
+ assertTrue(Disposer.isDisposed(disposable));
}
public void testIsDisposingWorksForUnregisteredDisposables() throws ExecutionException, InterruptedException {
assertFalse(future.isDone());
allowToContinueDispose.set(true);
future.get();
- assertFalse(Disposer.isDisposing(disposable));
+ assertTrue(Disposer.isDisposed(disposable));
}
public void testDisposableParentNotify() {
assertTrue(Disposer.isDisposed(parent));
}
+
+ public void testNoLeaksAfterConcurrentDisposeAndRegister() throws Exception {
+ ExecutorService executor = SequentialTaskExecutor.createSequentialApplicationPoolExecutor(StringUtil.capitalize(getName()));
+
+ for (int i = 0; i < 100; i++) {
+ MyDisposable parent = new MyDisposable("parent");
+ Future<?> future = executor.submit(() -> Disposer.tryRegister(parent, new MyDisposable("child")));
+
+ Disposer.dispose(parent);
+
+ future.get();
+
+ LeakHunter.checkLeak(Disposer.getTree(), MyDisposable.class);
+ }
+ }
}
projects / idea / community.git / commitdiff