fix memory leak in XDebugSessionImpl — ensure that listeners are removed on reset

It is advantage and one of the reason why we migrated to message bus — such leaks discovered by Disposer and easily discoverable. Not silent as EventDispatcher.
This commit is contained in:
Vladimir Krivosheev
2018-12-07 19:44:44 +01:00
parent 50ce4b644e
commit 5c6509d270
5 changed files with 40 additions and 63 deletions
@@ -34,6 +34,7 @@ import com.intellij.ui.AppUIUtil;
import com.intellij.util.EventDispatcher;
import com.intellij.util.SmartList;
import com.intellij.util.containers.SmartHashSet;
import com.intellij.util.messages.MessageBusConnection;
import com.intellij.util.ui.UIUtil;
import com.intellij.xdebugger.*;
import com.intellij.xdebugger.breakpoints.*;
@@ -86,7 +87,6 @@ public class XDebugSessionImpl implements XDebugSession {
private boolean myIsTopFrame;
private volatile XSourcePosition myTopFramePosition;
private final AtomicBoolean myPaused = new AtomicBoolean();
private MyDependentBreakpointListener myDependentBreakpointListener;
private XValueMarkers<?, ?> myValueMarkers;
private final String mySessionName;
private @Nullable XDebugSessionTab mySessionTab;
@@ -301,6 +301,7 @@ public class XDebugSessionImpl implements XDebugSession {
public void reset() {
breakpointsInitialized = false;
removeBreakpointListeners();
unsetPaused();
clearPausedData();
rebuildViews();
@@ -312,18 +313,14 @@ public class XDebugSessionImpl implements XDebugSession {
LOG.assertTrue(!breakpointsInitialized);
breakpointsInitialized = true;
XBreakpointManagerImpl breakpointManager = myDebuggerManager.getBreakpointManager();
XDependentBreakpointManager dependentBreakpointManager = breakpointManager.getDependentBreakpointManager();
disableSlaveBreakpoints(dependentBreakpointManager);
disableSlaveBreakpoints();
processAllBreakpoints(true, false);
if (myBreakpointListenerDisposable == null) {
myBreakpointListenerDisposable = Disposer.newDisposable();
myProject.getMessageBus().connect(myBreakpointListenerDisposable).subscribe(XBreakpointListener.TOPIC, new MyBreakpointListener());
}
if (myDependentBreakpointListener == null) {
myDependentBreakpointListener = new MyDependentBreakpointListener();
dependentBreakpointManager.addListener(myDependentBreakpointListener);
MessageBusConnection busConnection = myProject.getMessageBus().connect(myBreakpointListenerDisposable);
busConnection.subscribe(XBreakpointListener.TOPIC, new MyBreakpointListener());
busConnection.subscribe(XDependentBreakpointListener.TOPIC, new MyDependentBreakpointListener());
}
}
@@ -354,8 +351,8 @@ public class XDebugSessionImpl implements XDebugSession {
return mySessionData;
}
private void disableSlaveBreakpoints(final XDependentBreakpointManager dependentBreakpointManager) {
Set<XBreakpoint<?>> slaveBreakpoints = dependentBreakpointManager.getAllSlaveBreakpoints();
private void disableSlaveBreakpoints() {
Set<XBreakpoint<?>> slaveBreakpoints = myDebuggerManager.getBreakpointManager().getDependentBreakpointManager().getAllSlaveBreakpoints();
if (slaveBreakpoints.isEmpty()) {
return;
}
@@ -890,17 +887,7 @@ public class XDebugSessionImpl implements XDebugSession {
}
try {
if (breakpointsInitialized) {
XBreakpointManagerImpl breakpointManager = myDebuggerManager.getBreakpointManager();
Disposable breakpointListenerDisposable = myBreakpointListenerDisposable;
if (breakpointListenerDisposable != null) {
myBreakpointListenerDisposable = null;
Disposer.dispose(breakpointListenerDisposable);
}
if (myDependentBreakpointListener != null) {
breakpointManager.getDependentBreakpointManager().removeListener(myDependentBreakpointListener);
}
}
removeBreakpointListeners();
}
finally {
//noinspection unchecked
@@ -943,6 +930,14 @@ public class XDebugSessionImpl implements XDebugSession {
}
}
private void removeBreakpointListeners() {
Disposable breakpointListenerDisposable = myBreakpointListenerDisposable;
if (breakpointListenerDisposable != null) {
myBreakpointListenerDisposable = null;
Disposer.dispose(breakpointListenerDisposable);
}
}
public boolean isInactiveSlaveBreakpoint(final XBreakpoint<?> breakpoint) {
return myInactiveSlaveBreakpoints.contains(breakpoint);
}
@@ -54,7 +54,7 @@ public class XBreakpointManagerImpl implements XBreakpointManager {
myProject = project;
myDebuggerManager = debuggerManager;
myDependentBreakpointManager = new XDependentBreakpointManager(this);
myLineBreakpointManager = new XLineBreakpointManager(project, myDependentBreakpointManager);
myLineBreakpointManager = new XLineBreakpointManager(project);
if (!project.isDefault()) {
if (!ApplicationManager.getApplication().isUnitTestMode()) {
HttpFileSystem.getInstance().addFileListener(this::updateBreakpointInFile, project);
@@ -1,20 +1,7 @@
/*
* Copyright 2000-2009 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.xdebugger.impl.breakpoints;
import com.intellij.util.messages.Topic;
import com.intellij.xdebugger.breakpoints.XBreakpoint;
import org.jetbrains.annotations.NotNull;
@@ -24,6 +11,7 @@ import java.util.EventListener;
* @author nik
*/
public interface XDependentBreakpointListener extends EventListener {
Topic<XDependentBreakpointListener> TOPIC = new Topic<>("XBreakpointManager events", XDependentBreakpointListener.class);
void dependencySet(@NotNull XBreakpoint<?> slave, @NotNull XBreakpoint<?> master);
@@ -2,9 +2,10 @@
package com.intellij.xdebugger.impl.breakpoints;
import com.intellij.openapi.util.MultiValuesMap;
import com.intellij.util.EventDispatcher;
import com.intellij.util.SmartList;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.messages.MessageBus;
import com.intellij.util.messages.MessageBusConnection;
import com.intellij.xdebugger.breakpoints.XBreakpoint;
import com.intellij.xdebugger.breakpoints.XBreakpointListener;
import gnu.trove.THashMap;
@@ -20,12 +21,14 @@ public class XDependentBreakpointManager {
private final Map<XBreakpoint<?>, XDependentBreakpointInfo> mySlave2Info = new HashMap<>();
private final MultiValuesMap<XBreakpointBase, XDependentBreakpointInfo> myMaster2Info = new MultiValuesMap<>();
private final XBreakpointManagerImpl myBreakpointManager;
private final EventDispatcher<XDependentBreakpointListener> myDispatcher;
private final XDependentBreakpointListener myEventPublisher;
public XDependentBreakpointManager(@NotNull XBreakpointManagerImpl breakpointManager) {
myBreakpointManager = breakpointManager;
myDispatcher = EventDispatcher.create(XDependentBreakpointListener.class);
breakpointManager.getProject().getMessageBus().connect().subscribe(XBreakpointListener.TOPIC, new XBreakpointListener<XBreakpoint<?>>() {
MessageBus messageBus = breakpointManager.getProject().getMessageBus();
myEventPublisher = messageBus.syncPublisher(XDependentBreakpointListener.TOPIC);
MessageBusConnection busConnection = messageBus.connect();
busConnection.subscribe(XBreakpointListener.TOPIC, new XBreakpointListener<XBreakpoint<?>>() {
@Override
public void breakpointRemoved(@NotNull final XBreakpoint<?> breakpoint) {
XDependentBreakpointInfo info = mySlave2Info.remove(breakpoint);
@@ -38,7 +41,7 @@ public class XDependentBreakpointManager {
for (XDependentBreakpointInfo breakpointInfo : infos) {
XDependentBreakpointInfo removed = mySlave2Info.remove(breakpointInfo.mySlaveBreakpoint);
if (removed != null) {
myDispatcher.getMulticaster().dependencyCleared(breakpointInfo.mySlaveBreakpoint);
myEventPublisher.dependencyCleared(breakpointInfo.mySlaveBreakpoint);
}
}
}
@@ -46,14 +49,6 @@ public class XDependentBreakpointManager {
});
}
public void addListener(final XDependentBreakpointListener listener) {
myDispatcher.addListener(listener);
}
public void removeListener(final XDependentBreakpointListener listener) {
myDispatcher.removeListener(listener);
}
public void loadState() {
mySlave2Info.clear();
myMaster2Info.clear();
@@ -118,14 +113,14 @@ public class XDependentBreakpointManager {
info.myLeaveEnabled = leaveEnabled;
myMaster2Info.put((XBreakpointBase)master, info);
}
myDispatcher.getMulticaster().dependencySet(slave, master);
myEventPublisher.dependencySet(slave, master);
}
public void clearMasterBreakpoint(@NotNull XBreakpoint<?> slave) {
XDependentBreakpointInfo info = mySlave2Info.remove(slave);
if (info != null) {
myMaster2Info.remove(info.myMasterBreakpoint, info);
myDispatcher.getMulticaster().dependencyCleared(slave);
myEventPublisher.dependencyCleared(slave);
}
}
@@ -23,17 +23,18 @@ import com.intellij.openapi.fileEditor.TextEditor;
import com.intellij.openapi.project.DumbAwareRunnable;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.startup.StartupManager;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.util.io.FileUtil;
import com.intellij.openapi.util.registry.Registry;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.VirtualFileEvent;
import com.intellij.openapi.vfs.VirtualFileManager;
import com.intellij.openapi.vfs.VirtualFileUrlChangeAdapter;
import com.intellij.openapi.vfs.impl.BulkVirtualFileListenerAdapter;
import com.intellij.psi.PsiDocumentManager;
import com.intellij.util.SmartList;
import com.intellij.util.containers.BidirectionalMap;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.messages.MessageBusConnection;
import com.intellij.util.ui.update.MergingUpdateQueue;
import com.intellij.util.ui.update.Update;
import com.intellij.xdebugger.XDebuggerManager;
@@ -61,11 +62,11 @@ public class XLineBreakpointManager {
private final BidirectionalMap<XLineBreakpointImpl, String> myBreakpoints = new BidirectionalMap<>();
private final MergingUpdateQueue myBreakpointsUpdateQueue;
private final Project myProject;
private final XDependentBreakpointManager myDependentBreakpointManager;
public XLineBreakpointManager(@NotNull Project project, final XDependentBreakpointManager dependentBreakpointManager) {
public XLineBreakpointManager(@NotNull Project project) {
myProject = project;
myDependentBreakpointManager = dependentBreakpointManager;
MessageBusConnection busConnection = project.getMessageBus().connect();
if (!myProject.isDefault()) {
EditorEventMulticaster editorEventMulticaster = EditorFactory.getInstance().getEventMulticaster();
@@ -73,10 +74,8 @@ public class XLineBreakpointManager {
editorEventMulticaster.addEditorMouseListener(new MyEditorMouseListener(), project);
editorEventMulticaster.addEditorMouseMotionListener(new MyEditorMouseMotionListener(), project);
final MyDependentBreakpointListener myDependentBreakpointListener = new MyDependentBreakpointListener();
myDependentBreakpointManager.addListener(myDependentBreakpointListener);
Disposer.register(project, () -> myDependentBreakpointManager.removeListener(myDependentBreakpointListener));
VirtualFileManager.getInstance().addVirtualFileListener(new VirtualFileUrlChangeAdapter() {
busConnection.subscribe(XDependentBreakpointListener.TOPIC, new MyDependentBreakpointListener());
busConnection.subscribe(VirtualFileManager.VFS_CHANGES, new BulkVirtualFileListenerAdapter(new VirtualFileUrlChangeAdapter() {
@Override
protected void fileUrlChanged(String oldUrl, String newUrl) {
breakpoints().forEach(breakpoint -> {
@@ -92,12 +91,12 @@ public class XLineBreakpointManager {
List<XLineBreakpointImpl> breakpoints = myBreakpoints.getKeysByValue(event.getFile().getUrl());
removeBreakpoints(breakpoints != null ? new ArrayList<>(breakpoints) : null); // safe copy
}
}, project);
}));
}
myBreakpointsUpdateQueue = new MergingUpdateQueue("XLine breakpoints", 300, true, null, project);
// Update breakpoints colors if global color schema was changed
project.getMessageBus().connect().subscribe(EditorColorsManager.TOPIC, new MyEditorColorsListener());
busConnection.subscribe(EditorColorsManager.TOPIC, new MyEditorColorsListener());
}
void updateBreakpointsUI() {