From 3dc83f7651e070ced31e87eb4788b0d9c9bccb35 Mon Sep 17 00:00:00 2001 From: Aleksey Pivovarov Date: Wed, 20 May 2015 11:51:13 +0300 Subject: [PATCH] Revert "lst: replace myLock with ReadLock" this introduces deadlock because LineStatusTracker.release() acquires ReadLock holding LineStatusTrackerManager.LOCK, while the latter can be awaited in AWT This reverts commit 8e83f655d6df89c8988dfd94bb1f0ce833a6d241. --- .../openapi/vcs/ex/LineStatusTracker.java | 392 +++++++++--------- .../vcs/impl/LineStatusTrackerManager.java | 2 - 2 files changed, 206 insertions(+), 188 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.java index bc5de9fbda92..4c95033ed1bb 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/ex/LineStatusTracker.java @@ -42,7 +42,8 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.ui.EditorNotificationPanel; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.diff.FilesTooBigForDiffException; -import org.jetbrains.annotations.*; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.BitSet; @@ -62,6 +63,7 @@ public class LineStatusTracker { private static final Key PANEL_KEY = new Key("LineStatusTracker.CanNotCalculateDiffPanel"); + private final Object myLock = new Object(); private boolean myInitialized; @NotNull private final Project myProject; @@ -104,50 +106,51 @@ public class LineStatusTracker { myRanges = new ArrayList(); } - @CalledInAwt public void initialize(@NotNull final String vcsContent, @NotNull RevisionPack baseRevisionNumber) { myApplication.assertIsDispatchThread(); - if (myReleased) return; - try { - if (myBaseRevisionNumber != null && myBaseRevisionNumber.contains(baseRevisionNumber)) return; + synchronized (myLock) { + try { + if (myReleased) return; + if (myBaseRevisionNumber != null && myBaseRevisionNumber.contains(baseRevisionNumber)) return; - myBaseRevisionNumber = baseRevisionNumber; + myBaseRevisionNumber = baseRevisionNumber; - myVcsDocument.setReadOnly(false); - myVcsDocument.setText(vcsContent); - myVcsDocument.setReadOnly(true); - reinstallRanges(); + myVcsDocument.setReadOnly(false); + myVcsDocument.setText(vcsContent); + myVcsDocument.setReadOnly(true); + reinstallRanges(); - if (myDocumentListener == null) { - myDocumentListener = new MyDocumentListener(); - myDocument.addDocumentListener(myDocumentListener); + if (myDocumentListener == null) { + myDocumentListener = new MyDocumentListener(); + myDocument.addDocumentListener(myDocumentListener); + } + } + finally { + myInitialized = true; } - } - finally { - myInitialized = true; } } - @CalledInAwt private void reinstallRanges() { myApplication.assertIsDispatchThread(); - removeAnathema(); - removeHighlightersFromMarkupModel(); - try { - myRanges = new RangesBuilder(myDocument, myVcsDocument, myMode).getRanges(); - } - catch (FilesTooBigForDiffException e) { - installAnathema(); - return; - } - for (final Range range : myRanges) { - range.setHighlighter(createHighlighter(range)); + synchronized (myLock) { + removeAnathema(); + removeHighlightersFromMarkupModel(); + try { + myRanges = new RangesBuilder(myDocument, myVcsDocument, myMode).getRanges(); + } + catch (FilesTooBigForDiffException e) { + installAnathema(); + return; + } + for (final Range range : myRanges) { + range.setHighlighter(createHighlighter(range)); + } } } - @CalledInAwt private void installAnathema() { myAnathemaThrown = true; final FileEditor[] editors = myFileEditorManager.getAllEditors(myVirtualFile); @@ -161,7 +164,6 @@ public class LineStatusTracker { } } - @CalledInAwt private void removeAnathema() { if (!myAnathemaThrown) return; myAnathemaThrown = false; @@ -175,15 +177,15 @@ public class LineStatusTracker { } } - @CalledInAwt public void setMode(@NotNull Mode mode) { - if (myMode == mode) return; - myMode = mode; - reinstallRanges(); + synchronized (myLock) { + if (myMode == mode) return; + myMode = mode; + reinstallRanges(); + } } @NotNull - @CalledInAwt private RangeHighlighter createHighlighter(@NotNull Range range) { myApplication.assertIsDispatchThread(); @@ -226,31 +228,27 @@ public class LineStatusTracker { } public void release() { - myApplication.runReadAction(new Runnable() { - @Override - public void run() { - if (myReleased) return; - myReleased = true; - if (myDocumentListener != null) { - myDocument.removeDocumentListener(myDocumentListener); - } - - if (myApplication.isDispatchThread()) { - removeAnathema(); - removeHighlightersFromMarkupModel(); - } - else { - invalidateRanges(); - myApplication.invokeLater(new Runnable() { - @Override - public void run() { - removeAnathema(); - removeHighlightersFromMarkupModel(); - } - }); - } + synchronized (myLock) { + myReleased = true; + if (myDocumentListener != null) { + myDocument.removeDocumentListener(myDocumentListener); } - }); + + if (myApplication.isDispatchThread()) { + removeAnathema(); + removeHighlightersFromMarkupModel(); + } + else { + invalidateRanges(); + myApplication.invokeLater(new Runnable() { + @Override + public void run() { + removeAnathema(); + removeHighlightersFromMarkupModel(); + } + }); + } + } } @NotNull @@ -274,46 +272,51 @@ public class LineStatusTracker { } @NotNull - @CalledWithReadLock public List getRanges() { - return Collections.unmodifiableList(myRanges); + synchronized (myLock) { + return Collections.unmodifiableList(myRanges); + } } - @CalledInAwt public void startBulkUpdate() { - if (myReleased) return; + synchronized (myLock) { + if (myReleased) return; - myBulkUpdate = true; - removeAnathema(); - removeHighlightersFromMarkupModel(); + myBulkUpdate = true; + removeAnathema(); + removeHighlightersFromMarkupModel(); + } } - @CalledInAwt private void removeHighlightersFromMarkupModel() { myApplication.assertIsDispatchThread(); - for (Range range : myRanges) { - if (range.getHighlighter() != null) { - range.getHighlighter().dispose(); + synchronized (myLock) { + for (Range range : myRanges) { + if (range.getHighlighter() != null) { + range.getHighlighter().dispose(); + } + range.invalidate(); } - range.invalidate(); + myRanges.clear(); } - myRanges.clear(); } - @CalledWithReadLock private void invalidateRanges() { - for (Range range : myRanges) { - range.invalidate(); + synchronized (myLock) { + for (Range range : myRanges) { + range.invalidate(); + } } } - @CalledInAwt public void finishBulkUpdate() { - if (myReleased) return; + synchronized (myLock) { + if (myReleased) return; - myBulkUpdate = false; - reinstallRanges(); + myBulkUpdate = false; + reinstallRanges(); + } } private void markFileUnchanged() { @@ -321,7 +324,11 @@ public class LineStatusTracker { @Override public void run() { FileDocumentManager.getInstance().saveDocument(myDocument); - if (myRanges.isEmpty()) { + boolean stillEmpty; + synchronized (myLock) { + stillEmpty = myRanges.isEmpty(); + } + if (stillEmpty) { // file was modified, and now it's not -> dirty local change myVcsDirtyScopeManager.fileDirty(myVirtualFile); } @@ -342,25 +349,27 @@ public class LineStatusTracker { @Override public void beforeDocumentChange(DocumentEvent e) { myApplication.assertIsDispatchThread(); - if (myReleased) return; - if (myBulkUpdate || mySuppressUpdate || myAnathemaThrown || !myInitialized) return; - assert myDocument == e.getDocument(); + synchronized (myLock) { + if (myReleased) return; + if (myBulkUpdate || mySuppressUpdate || myAnathemaThrown || !myInitialized) return; + assert myDocument == e.getDocument(); - try { - myLine1 = myDocument.getLineNumber(e.getOffset()); - if (e.getOldLength() == 0) { - myBeforeChangedLines = 1; + try { + myLine1 = myDocument.getLineNumber(e.getOffset()); + if (e.getOldLength() == 0) { + myBeforeChangedLines = 1; + } + else { + int line1 = myLine1; + int line2 = myDocument.getLineNumber(e.getOffset() + e.getOldLength()); + myBeforeChangedLines = line2 - line1 + 1; + } + + myBeforeTotalLines = getLineCount(myDocument); } - else { - int line1 = myLine1; - int line2 = myDocument.getLineNumber(e.getOffset() + e.getOldLength()); - myBeforeChangedLines = line2 - line1 + 1; + catch (ProcessCanceledException ignore) { } - - myBeforeTotalLines = getLineCount(myDocument); - } - catch (ProcessCanceledException ignore) { } } @@ -368,30 +377,32 @@ public class LineStatusTracker { public void documentChanged(final DocumentEvent e) { myApplication.assertIsDispatchThread(); - if (myReleased) return; - if (myBulkUpdate || mySuppressUpdate || myAnathemaThrown || !myInitialized) return; - assert myDocument == e.getDocument(); + synchronized (myLock) { + if (myReleased) return; + if (myBulkUpdate || mySuppressUpdate || myAnathemaThrown || !myInitialized) return; + assert myDocument == e.getDocument(); + + int afterChangedLines; + if (e.getNewLength() == 0) { + afterChangedLines = 1; + } + else { + int line1 = myLine1; + int line2 = myDocument.getLineNumber(e.getOffset() + e.getNewLength()); + afterChangedLines = line2 - line1 + 1; + } + + int linesShift = afterChangedLines - myBeforeChangedLines; - int afterChangedLines; - if (e.getNewLength() == 0) { - afterChangedLines = 1; - } - else { int line1 = myLine1; - int line2 = myDocument.getLineNumber(e.getOffset() + e.getNewLength()); - afterChangedLines = line2 - line1 + 1; + int line2 = line1 + myBeforeChangedLines; + + int[] fixed = fixRanges(e, line1, line2); + line1 = fixed[0]; + line2 = fixed[1]; + + doUpdateRanges(line1, line2, linesShift, myBeforeTotalLines); } - - int linesShift = afterChangedLines - myBeforeChangedLines; - - int line1 = myLine1; - int line2 = line1 + myBeforeChangedLines; - - int[] fixed = fixRanges(e, line1, line2); - line1 = fixed[0]; - line2 = fixed[1]; - - doUpdateRanges(line1, line2, linesShift, myBeforeTotalLines); } } @@ -668,73 +679,77 @@ public class LineStatusTracker { } @Nullable - @CalledWithReadLock public Range getNextRange(Range range) { - final int index = myRanges.indexOf(range); - if (index == myRanges.size() - 1) return null; - return myRanges.get(index + 1); + synchronized (myLock) { + final int index = myRanges.indexOf(range); + if (index == myRanges.size() - 1) return null; + return myRanges.get(index + 1); + } } @Nullable - @CalledWithReadLock public Range getPrevRange(Range range) { - final int index = myRanges.indexOf(range); - if (index <= 0) return null; - return myRanges.get(index - 1); + synchronized (myLock) { + final int index = myRanges.indexOf(range); + if (index <= 0) return null; + return myRanges.get(index - 1); + } } @Nullable - @CalledWithReadLock public Range getNextRange(int line) { - for (Range range : myRanges) { - if (line < range.getLine2() && !range.isSelectedByLine(line)) { - return range; + synchronized (myLock) { + for (Range range : myRanges) { + if (line < range.getLine2() && !range.isSelectedByLine(line)) { + return range; + } } + return null; } - return null; } @Nullable - @CalledWithReadLock public Range getPrevRange(int line) { - for (int i = myRanges.size() - 1; i >= 0; i--) { - Range range = myRanges.get(i); - if (line > range.getLine1() && !range.isSelectedByLine(line)) { - return range; + synchronized (myLock) { + for (int i = myRanges.size() - 1; i >= 0; i--) { + Range range = myRanges.get(i); + if (line > range.getLine1() && !range.isSelectedByLine(line)) { + return range; + } } + return null; } - return null; } @Nullable - @CalledWithReadLock public Range getRangeForLine(int line) { - for (final Range range : myRanges) { - if (range.isSelectedByLine(line)) return range; + synchronized (myLock) { + for (final Range range : myRanges) { + if (range.isSelectedByLine(line)) return range; + } + return null; } - return null; } - @CalledWithWriteLock private void doRollbackRange(@NotNull Range range) { DiffUtil.applyModification(myDocument, range.getLine1(), range.getLine2(), myVcsDocument, range.getVcsLine1(), range.getVcsLine2()); } - @CalledWithWriteLock public void rollbackChanges(@NotNull Range range) { myApplication.assertWriteAccessAllowed(); - if (myBulkUpdate) return; + synchronized (myLock) { + if (myBulkUpdate) return; - if (!range.isValid()) { - LOG.warn("Rollback of invalid range"); - return; + if (!range.isValid()) { + LOG.warn("Rollback of invalid range"); + return; + } + + doRollbackRange(range); } - - doRollbackRange(range); } - @CalledWithWriteLock public void rollbackChanges(@NotNull final BitSet lines) { runBulkRollback(new Runnable() { @Override @@ -778,7 +793,6 @@ public class LineStatusTracker { }); } - @CalledWithWriteLock public void rollbackAllChanges() { runBulkRollback(new Runnable() { @Override @@ -793,69 +807,75 @@ public class LineStatusTracker { }); } - @CalledWithWriteLock private void runBulkRollback(@NotNull Runnable task) { myApplication.assertWriteAccessAllowed(); - if (myBulkUpdate) return; + synchronized (myLock) { + if (myBulkUpdate) return; - try { - mySuppressUpdate = true; + try { + mySuppressUpdate = true; - task.run(); - } - catch (Error e) { - reinstallRanges(); - throw e; - } - catch (RuntimeException e) { - reinstallRanges(); - throw e; - } - finally { - mySuppressUpdate = false; + task.run(); + } + catch (Error e) { + reinstallRanges(); + throw e; + } + catch (RuntimeException e) { + reinstallRanges(); + throw e; + } + finally { + mySuppressUpdate = false; + } } } @NotNull - @CalledWithReadLock public CharSequence getCurrentContent(@NotNull Range range) { - TextRange textRange = getCurrentTextRange(range); - final int startOffset = textRange.getStartOffset(); - final int endOffset = textRange.getEndOffset(); - return myDocument.getImmutableCharSequence().subSequence(startOffset, endOffset); + synchronized (myLock) { + TextRange textRange = getCurrentTextRange(range); + final int startOffset = textRange.getStartOffset(); + final int endOffset = textRange.getEndOffset(); + return myDocument.getImmutableCharSequence().subSequence(startOffset, endOffset); + } } @NotNull - @CalledWithReadLock public CharSequence getVcsContent(@NotNull Range range) { - TextRange textRange = getVcsTextRange(range); - final int startOffset = textRange.getStartOffset(); - final int endOffset = textRange.getEndOffset(); - return myVcsDocument.getImmutableCharSequence().subSequence(startOffset, endOffset); + synchronized (myLock) { + TextRange textRange = getVcsTextRange(range); + final int startOffset = textRange.getStartOffset(); + final int endOffset = textRange.getEndOffset(); + return myVcsDocument.getImmutableCharSequence().subSequence(startOffset, endOffset); + } } @NotNull - @CalledWithReadLock public TextRange getCurrentTextRange(@NotNull Range range) { - if (!range.isValid()) { - LOG.warn("Current TextRange of invalid range"); - } + myApplication.assertReadAccessAllowed(); - return DiffUtil.getLinesRange(myDocument, range.getLine1(), range.getLine2()); + synchronized (myLock) { + if (!range.isValid()) { + LOG.warn("Current TextRange of invalid range"); + } + + return DiffUtil.getLinesRange(myDocument, range.getLine1(), range.getLine2()); + } } @NotNull - @CalledWithReadLock public TextRange getVcsTextRange(@NotNull Range range) { - if (!range.isValid()) { - LOG.warn("Vcs TextRange of invalid range"); - } + synchronized (myLock) { + if (!range.isValid()) { + LOG.warn("Vcs TextRange of invalid range"); + } - return DiffUtil.getLinesRange(myVcsDocument, range.getVcsLine1(), range.getVcsLine2()); + return DiffUtil.getLinesRange(myVcsDocument, range.getVcsLine1(), range.getVcsLine2()); + } } - @NotNull public static LineStatusTracker createOn(@NotNull VirtualFile virtualFile, @NotNull final Document document, final Project project, @NotNull Mode mode) { final Document vcsDocument = new DocumentImpl("", true); diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.java index 265e7fe38871..6de1143b6f8a 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/LineStatusTrackerManager.java @@ -62,7 +62,6 @@ import com.intellij.util.containers.HashMap; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.CalledInAwt; import java.util.Map; @@ -209,7 +208,6 @@ public class LineStatusTrackerManager implements ProjectComponent, LineStatusTra } } - @CalledInAwt public void updateSettings() { synchronized (myLock) { LineStatusTracker.Mode mode = getMode();