From fa3714af83e3aeb23d62645ebbc5f34b4c677b94 Mon Sep 17 00:00:00 2001 From: Aleksey Pivovarov Date: Fri, 15 May 2015 17:24:19 +0300 Subject: [PATCH] diff: dispose viewers in EDT avoid 'half-disposed' state with two-step disposing (on pooled and EDT threads) --- .../src/com/intellij/diff/FrameDiffTool.java | 4 +++ .../diff/impl/DiffRequestProcessor.java | 4 +++ .../diff/tools/binary/BinaryDiffViewer.java | 3 ++ .../tools/fragmented/OnesideDiffViewer.java | 2 ++ .../diff/tools/simple/SimpleDiffViewer.java | 6 ++-- .../simple/SimpleThreesideDiffViewer.java | 6 ++-- .../diff/tools/util/base/DiffViewerBase.java | 31 ++++++++++--------- .../threeside/ThreesideTextDiffViewer.java | 7 ++--- .../util/twoside/TwosideTextDiffViewer.java | 7 ++--- 9 files changed, 41 insertions(+), 29 deletions(-) diff --git a/platform/diff-api/src/com/intellij/diff/FrameDiffTool.java b/platform/diff-api/src/com/intellij/diff/FrameDiffTool.java index 446a33a25204..86f354889cc2 100644 --- a/platform/diff-api/src/com/intellij/diff/FrameDiffTool.java +++ b/platform/diff-api/src/com/intellij/diff/FrameDiffTool.java @@ -40,6 +40,10 @@ public interface FrameDiffTool extends DiffTool { @NotNull @CalledInAwt ToolbarComponents init(); + + @Override + @CalledInAwt + void dispose(); } class ToolbarComponents { diff --git a/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java b/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java index 74674c4f83ab..7244faf8aff3 100644 --- a/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java +++ b/platform/diff-impl/src/com/intellij/diff/impl/DiffRequestProcessor.java @@ -968,6 +968,7 @@ public abstract class DiffRequestProcessor implements Disposable { private interface ViewerState { void init(); + @CalledInAwt void destroy(); @Nullable @@ -1008,6 +1009,7 @@ public abstract class DiffRequestProcessor implements Disposable { } @Override + @CalledInAwt public void destroy() { Disposer.dispose(myViewer); } @@ -1058,6 +1060,7 @@ public abstract class DiffRequestProcessor implements Disposable { } @Override + @CalledInAwt public void destroy() { Disposer.dispose(myViewer); } @@ -1130,6 +1133,7 @@ public abstract class DiffRequestProcessor implements Disposable { } @Override + @CalledInAwt public void destroy() { Disposer.dispose(myViewer); Disposer.dispose(myWrapperViewer); diff --git a/platform/diff-impl/src/com/intellij/diff/tools/binary/BinaryDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/binary/BinaryDiffViewer.java index 9079242200e0..dd96392a2f9e 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/binary/BinaryDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/binary/BinaryDiffViewer.java @@ -55,6 +55,7 @@ import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.Pair; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.ui.UIUtil; +import org.jetbrains.annotations.CalledInAwt; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -118,12 +119,14 @@ public class BinaryDiffViewer extends ListenerDiffViewerBase { } @Override + @CalledInAwt protected void onInit() { super.onInit(); processContextHints(); } @Override + @CalledInAwt public void onDispose() { updateContextHints(); destroyEditorListeners(); diff --git a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/OnesideDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/OnesideDiffViewer.java index 652904780f43..0a897967f03f 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/fragmented/OnesideDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/fragmented/OnesideDiffViewer.java @@ -146,6 +146,7 @@ public class OnesideDiffViewer extends TextDiffViewerBase { } @Override + @CalledInAwt protected void onInit() { super.onInit(); processContextHints(); @@ -154,6 +155,7 @@ public class OnesideDiffViewer extends TextDiffViewerBase { } @Override + @CalledInAwt protected void onDispose() { updateContextHints(); EditorFactory.getInstance().releaseEditor(myEditor); diff --git a/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleDiffViewer.java index 4f007437da89..b564ca219ea8 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleDiffViewer.java @@ -92,6 +92,7 @@ public class SimpleDiffViewer extends TwosideTextDiffViewer { } @Override + @CalledInAwt protected void onInit() { super.onInit(); myContentPanel.setPainter(new MyDividerPainter()); @@ -99,10 +100,11 @@ public class SimpleDiffViewer extends TwosideTextDiffViewer { } @Override - protected void onDisposeAwt() { + @CalledInAwt + protected void onDispose() { myModifierProvider.destroy(); destroyChangedBlocks(); - super.onDisposeAwt(); + super.onDispose(); } @NotNull diff --git a/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleThreesideDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleThreesideDiffViewer.java index 77d828f49316..ec42e74ab28f 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleThreesideDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/simple/SimpleThreesideDiffViewer.java @@ -86,6 +86,7 @@ public class SimpleThreesideDiffViewer extends ThreesideTextDiffViewer { } @Override + @CalledInAwt protected void onInit() { super.onInit(); myContentPanel.setPainter(new MyDividerPainter(Side.LEFT), Side.LEFT); @@ -94,9 +95,10 @@ public class SimpleThreesideDiffViewer extends ThreesideTextDiffViewer { } @Override - protected void onDisposeAwt() { + @CalledInAwt + protected void onDispose() { destroyChangedBlocks(); - super.onDisposeAwt(); + super.onDispose(); } @NotNull diff --git a/platform/diff-impl/src/com/intellij/diff/tools/util/base/DiffViewerBase.java b/platform/diff-impl/src/com/intellij/diff/tools/util/base/DiffViewerBase.java index 5f41070aa91f..836f8b0ac338 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/util/base/DiffViewerBase.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/util/base/DiffViewerBase.java @@ -37,7 +37,6 @@ import org.jetbrains.annotations.*; import javax.swing.*; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; public abstract class DiffViewerBase implements DiffViewer, DataProvider { protected static final Logger LOG = Logger.getInstance(DiffViewerBase.class); @@ -47,7 +46,7 @@ public abstract class DiffViewerBase implements DiffViewer, DataProvider { @NotNull protected final ContentDiffRequest myRequest; @NotNull private final DiffTaskQueue myTaskExecutor = new DiffTaskQueue(); - @NotNull private final AtomicBoolean myDisposed = new AtomicBoolean(false); + private volatile boolean myDisposed; public DiffViewerBase(@NotNull DiffContext context, @NotNull ContentDiffRequest request) { myProject = context.getProject(); @@ -69,22 +68,27 @@ public abstract class DiffViewerBase implements DiffViewer, DataProvider { } @Override + @CalledInAwt public final void dispose() { - if (!myDisposed.compareAndSet(false, true)) return; + if (myDisposed) return; - onDispose(); - - UIUtil.invokeLaterIfNeeded(new Runnable() { + Runnable doDispose = new Runnable() { @Override public void run() { - onDisposeAwt(); + if (myDisposed) return; + myDisposed = true; + + onDispose(); } - }); + }; + + if (!ApplicationManager.getApplication().isDispatchThread()) LOG.warn(new Throwable("dispose() not from EDT")); + UIUtil.invokeLaterIfNeeded(doDispose); } @CalledInAwt public final void scheduleRediff() { - if (myDisposed.get()) return; + if (isDisposed()) return; myTaskExecutor.abortAndSchedule(new Runnable() { @Override @@ -106,7 +110,7 @@ public abstract class DiffViewerBase implements DiffViewer, DataProvider { @CalledInAwt public final void rediff(boolean trySync) { - if (myDisposed.get()) return; + if (isDisposed()) return; onBeforeRediff(); @@ -148,7 +152,7 @@ public abstract class DiffViewerBase implements DiffViewer, DataProvider { } public boolean isDisposed() { - return myDisposed.get(); + return myDisposed; } // @@ -191,14 +195,11 @@ public abstract class DiffViewerBase implements DiffViewer, DataProvider { @NotNull protected abstract Runnable performRediff(@NotNull ProgressIndicator indicator); + @CalledInAwt protected void onDispose() { Disposer.dispose(myTaskExecutor); } - @CalledInAwt - protected void onDisposeAwt() { - } - @Nullable protected OpenFileDescriptor getOpenFileDescriptor() { return null; diff --git a/platform/diff-impl/src/com/intellij/diff/tools/util/threeside/ThreesideTextDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/util/threeside/ThreesideTextDiffViewer.java index a1f81110d4d1..0346a753215e 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/util/threeside/ThreesideTextDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/util/threeside/ThreesideTextDiffViewer.java @@ -111,21 +111,18 @@ public abstract class ThreesideTextDiffViewer extends TextDiffViewerBase { } @Override + @CalledInAwt protected void onInit() { super.onInit(); processContextHints(); } @Override + @CalledInAwt protected void onDispose() { updateContextHints(); super.onDispose(); - } - - @Override - protected void onDisposeAwt() { destroyEditors(); - super.onDisposeAwt(); } protected void processContextHints() { diff --git a/platform/diff-impl/src/com/intellij/diff/tools/util/twoside/TwosideTextDiffViewer.java b/platform/diff-impl/src/com/intellij/diff/tools/util/twoside/TwosideTextDiffViewer.java index 0a767ff45e5e..2f0d5eb48367 100644 --- a/platform/diff-impl/src/com/intellij/diff/tools/util/twoside/TwosideTextDiffViewer.java +++ b/platform/diff-impl/src/com/intellij/diff/tools/util/twoside/TwosideTextDiffViewer.java @@ -114,21 +114,18 @@ public abstract class TwosideTextDiffViewer extends TextDiffViewerBase { } @Override + @CalledInAwt protected void onInit() { super.onInit(); processContextHints(); } @Override + @CalledInAwt protected void onDispose() { updateContextHints(); super.onDispose(); - } - - @Override - protected void onDisposeAwt() { destroyEditors(); - super.onDisposeAwt(); } protected void processContextHints() {