avoid memory leak when register disposable on already disposed parent

This commit is contained in:
Alexey Kudravtsev
2015-11-17 16:11:10 +03:00
parent 569323f391
commit cd0083b23e
3 changed files with 65 additions and 22 deletions
@@ -1,5 +1,5 @@
/*
* Copyright 2000-2013 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.
@@ -57,7 +57,7 @@ public final class ActionMenu extends JMenu {
private MenuItemSynchronizer myMenuItemSynchronizer;
private StubItem myStubItem; // A PATCH!!! Do not remove this code, otherwise you will lose all keyboard navigation in JMenuBar.
private final boolean myTopLevel;
private final Disposable myDisposable;
private Disposable myDisposable;
public ActionMenu(final DataContext context,
@NotNull final String place,
@@ -84,11 +84,6 @@ public final class ActionMenu extends JMenu {
if (UIUtil.isUnderIntelliJLaF()) {
setOpaque(true);
}
myDisposable = new Disposable() {
@Override
public void dispose() {
}
};
// Triggering initialization of private field "popupMenu" from JMenu with our own JBPopupMenu
getPopupMenu();
@@ -98,6 +93,7 @@ public final class ActionMenu extends JMenu {
myContext = context;
}
@Override
public void addNotify() {
super.addNotify();
installSynchronizer();
@@ -115,7 +111,10 @@ public final class ActionMenu extends JMenu {
public void removeNotify() {
uninstallSynchronizer();
super.removeNotify();
Disposer.dispose(myDisposable);
if (myDisposable != null) {
Disposer.dispose(myDisposable);
myDisposable = null;
}
}
private void uninstallSynchronizer() {
@@ -126,7 +125,7 @@ public final class ActionMenu extends JMenu {
}
}
private JPopupMenu mySpecialMenu = null;
private JPopupMenu mySpecialMenu;
@Override
public JPopupMenu getPopupMenu() {
if (mySpecialMenu == null) {
@@ -184,7 +183,7 @@ public final class ActionMenu extends JMenu {
}
private void init() {
boolean macSystemMenu = SystemInfo.isMacSystemMenu && myPlace == ActionPlaces.MAIN_MENU;
boolean macSystemMenu = SystemInfo.isMacSystemMenu && myPlace.equals(ActionPlaces.MAIN_MENU);
myStubItem = macSystemMenu ? null : new StubItem();
addStubItem();
@@ -252,25 +251,35 @@ public final class ActionMenu extends JMenu {
}
private class MenuListenerImpl implements MenuListener {
@Override
public void menuCanceled(MenuEvent e) {
clearItems();
addStubItem();
}
@Override
public void menuDeselected(MenuEvent e) {
Disposer.dispose(myDisposable);
if (myDisposable != null) {
Disposer.dispose(myDisposable);
myDisposable = null;
}
clearItems();
addStubItem();
}
@Override
public void menuSelected(MenuEvent e) {
new UsabilityHelper(ActionMenu.this, myDisposable);
UsabilityHelper helper = new UsabilityHelper(ActionMenu.this);
if (myDisposable == null) {
myDisposable = Disposer.newDisposable();
}
Disposer.register(myDisposable, helper);
fillMenu();
}
}
private void clearItems() {
if (SystemInfo.isMacSystemMenu && myPlace == ActionPlaces.MAIN_MENU) {
if (SystemInfo.isMacSystemMenu && myPlace.equals(ActionPlaces.MAIN_MENU)) {
for (Component menuComponent : getMenuComponents()) {
if (menuComponent instanceof ActionMenu) {
((ActionMenu)menuComponent).clearItems();
@@ -315,6 +324,7 @@ public final class ActionMenu extends JMenu {
}
private class MenuItemSynchronizer implements PropertyChangeListener {
@Override
public void propertyChange(PropertyChangeEvent e) {
String name = e.getPropertyName();
if (Presentation.PROP_VISIBLE.equals(name)) {
@@ -343,14 +353,13 @@ public final class ActionMenu extends JMenu {
private static class UsabilityHelper implements IdeEventQueue.EventDispatcher, AWTEventListener, Disposable {
private Component myComponent;
private Point myLastMousePoint = null;
private Point myUpperTargetPoint = null;
private Point myLowerTargetPoint = null;
private Point myLastMousePoint;
private Point myUpperTargetPoint;
private Point myLowerTargetPoint;
private SingleAlarm myCallbackAlarm;
private MouseEvent myEventToRedispatch = null;
private MouseEvent myEventToRedispatch;
private UsabilityHelper(Component component, @NotNull Disposable disposable) {
Disposer.register(disposable, this);
private UsabilityHelper(Component component) {
myCallbackAlarm = new SingleAlarm(new Runnable() {
@Override
public void run() {
@@ -17,9 +17,11 @@ 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 com.intellij.util.containers.WeakHashMap;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.TestOnly;
@@ -35,6 +37,7 @@ public final class ObjectTree<T> {
// identity used here to prevent problems with hashCode/equals overridden by not very bright minds
private final Set<T> myRootObjects = ContainerUtil.newIdentityTroveSet(); // guarded by treeLock
private final Map<T, ObjectNode<T>> myObject2NodeMap = ContainerUtil.newIdentityTroveMap(); // guarded by treeLock
private final Map<Object, Object> myDisposedObjects = new WeakHashMap<Object, Object>(100, 0.5f, ContainerUtil.identityStrategy()); // guarded by treeLock
private final List<ObjectNode<T>> myExecutedNodes = new ArrayList<ObjectNode<T>>(); // guarded by myExecutedNodes
private final List<T> myExecutedUnregisteredNodes = new ArrayList<T>(); // guarded by myExecutedUnregisteredNodes
@@ -58,6 +61,13 @@ public final class ObjectTree<T> {
public final void register(@NotNull T parent, @NotNull T child) {
synchronized (treeLock) {
Object wasDisposed = myDisposedObjects.get(parent);
if (wasDisposed != null) {
throw new IncorrectOperationException("Sorry but parent: " + parent + " has already been disposed " +
"(see the cause for stacktrace) so the child: "+child+" will never be disposed",
wasDisposed instanceof Throwable ? (Throwable)wasDisposed : null);
}
myDisposedObjects.remove(child); // if we dispose thing and then register it back it means it's not disposed anymore
ObjectNode<T> parentNode = getNode(parent);
if (parentNode == null) parentNode = createNodeFor(parent, null);
@@ -202,7 +212,7 @@ public final class ObjectTree<T> {
}
}
}
@TestOnly
public boolean isEmpty() {
synchronized (treeLock) {
@@ -235,6 +245,7 @@ public final class ObjectTree<T> {
for (ObjectTreeListener each : myListeners) {
each.objectExecuted(object);
}
myDisposedObjects.put(object, Disposer.isDebugMode() ? ThrowableInterner.intern(new Throwable()) : Boolean.TRUE);
}
int size() {
@@ -27,7 +27,6 @@ import java.util.List;
import static com.intellij.openapi.util.Disposer.newDisposable;
public class DisposerTest extends TestCase {
private MyDisposable myRoot;
private MyDisposable myFolder1;
@@ -222,7 +221,7 @@ public class DisposerTest extends TestCase {
private boolean myDisposed;
protected String myName;
MyDisposable(@NonNls String aName) {
private MyDisposable(@NonNls String aName) {
myName = aName;
}
@@ -301,4 +300,28 @@ public class DisposerTest extends TestCase {
}
}
public void testMustNotRegisterWithAlreadyDisposed() {
Disposable disposable = Disposer.newDisposable();
Disposer.register(myRoot, disposable);
Disposer.dispose(disposable);
try {
Disposer.register(disposable, Disposer.newDisposable());
fail("Must not be able to register with already disposed parent");
}
catch (IncorrectOperationException ignored) {
}
}
public void testRegisterThenDisposeThenRegisterAgain() {
Disposable disposable = Disposer.newDisposable();
Disposer.register(myRoot, disposable);
Disposer.dispose(disposable);
Disposer.register(myRoot, disposable);
Disposer.register(disposable, Disposer.newDisposable());
}
}