better diagnostics for rogue after commit listeners (which go and register another after commit handler inside)

This commit is contained in:
Alexey Kudravtsev
2015-10-26 14:05:04 +03:00
parent 38a0721d62
commit 09852a929b
2 changed files with 94 additions and 53 deletions
@@ -40,10 +40,7 @@ import com.intellij.openapi.progress.ProgressIndicator;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.roots.FileIndexFacade;
import com.intellij.openapi.util.Computable;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.*;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.*;
import com.intellij.psi.impl.smartPointers.SmartPointerManagerImpl;
@@ -75,7 +72,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
protected final Set<Document> myUncommittedDocuments = ContainerUtil.newConcurrentSet();
private final Map<Document, UncommittedInfo> myUncommittedInfos = ContainerUtil.newConcurrentMap();
protected boolean myStopTrackingDocuments;
protected boolean myPerformBackgroundCommit = true;
private boolean myPerformBackgroundCommit = true;
private volatile boolean myIsCommitInProgress;
private final PsiToDocumentSynchronizer mySynchronizer;
@@ -164,7 +161,6 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
return ((PsiManagerEx)myPsiManager).getFileManager().findFile(virtualFile);
}
@Nullable
@Override
public Document getDocument(@NotNull PsiFile file) {
if (file instanceof PsiBinaryFile) return null;
@@ -263,6 +259,9 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
}
return true;
}
checkWeAreOutsideAfterCommitHandler();
actionsWhenAllDocumentsAreCommitted.put(key, action);
return false;
}
@@ -484,6 +483,8 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
@Override
public boolean performWhenAllCommitted(@NotNull final Runnable action) {
ApplicationManager.getApplication().assertIsDispatchThread();
checkWeAreOutsideAfterCommitHandler();
assert !myProject.isDisposed() : "Already disposed: " + myProject;
if (myUncommittedDocuments.isEmpty()) {
action.run();
@@ -525,22 +526,30 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
}
if (!hasUncommitedDocuments() && !actionsWhenAllDocumentsAreCommitted.isEmpty()) {
List<Object> keys = new ArrayList<Object>(actionsWhenAllDocumentsAreCommitted.keySet());
for (Object key : keys) {
Runnable action = actionsWhenAllDocumentsAreCommitted.remove(key);
List<Map.Entry<Object, Runnable>> entries = new ArrayList<Map.Entry<Object, Runnable>>(new LinkedHashMap<Object, Runnable>(actionsWhenAllDocumentsAreCommitted).entrySet());
weAreInsideAfterCommitHandler();
for (Map.Entry<Object, Runnable> entry : entries) {
Runnable action = entry.getValue();
Object key = entry.getKey();
try {
myDocumentCommitProcessor.log("Running after commit runnable: ", null, false, key, action);
int before = actionsWhenAllDocumentsAreCommitted.size();
action.run();
int after = actionsWhenAllDocumentsAreCommitted.size();
if (before != after) {
LOG.error("You must not call performWhenAllCommitted() from within after-commit handler " + action + ": "+action.getClass());
}
}
catch (Throwable e) {
LOG.error("During running "+action, e);
}
}
actionsWhenAllDocumentsAreCommitted.clear();
}
}
private void weAreInsideAfterCommitHandler() {
actionsWhenAllDocumentsAreCommitted.put(PERFORM_ALWAYS_KEY, EmptyRunnable.getInstance()); // to prevent listeners from registering new actions during firing
}
private void checkWeAreOutsideAfterCommitHandler() {
if (actionsWhenAllDocumentsAreCommitted.get(PERFORM_ALWAYS_KEY) == EmptyRunnable.getInstance()) {
throw new IncorrectOperationException("You must not call performWhenAllCommitted()/cancelAndRunWhenCommitted() from within after-commit handler");
}
}
@@ -929,12 +938,12 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
}
private static class UncommittedInfo extends DocumentAdapter implements PrioritizedInternalDocumentListener {
final DocumentImpl myOriginal;
final FrozenDocument myFrozen;
final List<DocumentEvent> myEvents = ContainerUtil.newArrayList();
final ConcurrentMap<DocumentWindow, DocumentWindow> myFrozenWindows = ContainerUtil.newConcurrentMap();
private final DocumentImpl myOriginal;
private final FrozenDocument myFrozen;
private final List<DocumentEvent> myEvents = ContainerUtil.newArrayList();
private final ConcurrentMap<DocumentWindow, DocumentWindow> myFrozenWindows = ContainerUtil.newConcurrentMap();
public UncommittedInfo(DocumentImpl original) {
private UncommittedInfo(DocumentImpl original) {
myOriginal = original;
myFrozen = original.freeze();
myOriginal.addDocumentListener(this);
@@ -45,8 +45,10 @@ import com.intellij.testFramework.LeakHunter;
import com.intellij.testFramework.LightVirtualFile;
import com.intellij.testFramework.PlatformTestCase;
import com.intellij.testFramework.PlatformTestUtil;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.concurrency.Semaphore;
import com.intellij.util.ui.UIUtil;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
@@ -94,7 +96,7 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
VirtualFile vFile = createFile();
final PsiFile file = new MockPsiFile(vFile, getPsiManager());
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
assertNotNull(document);
assertSame(document, FileDocumentManager.getInstance().getDocument(vFile));
}
@@ -111,7 +113,7 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
public void testDocumentGced() throws Exception {
VirtualFile vFile = getVirtualFile(createTempFile("txt", "abc"));
PsiDocumentManagerImpl documentManager = getPsiDocumentManager();
long id = System.identityHashCode(documentManager.getDocument(getPsiManager().findFile(vFile)));
long id = System.identityHashCode(documentManager.getDocument(findFile(vFile)));
documentManager.commitAllDocuments();
UIUtil.dispatchAllInvocationEvents();
@@ -121,28 +123,31 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
LeakHunter.checkLeak(documentManager, DocumentImpl.class);
LeakHunter.checkLeak(documentManager, PsiFileImpl.class,
psiFile -> psiFile.getViewProvider().getVirtualFile().getFileSystem() instanceof LocalFileSystem);
//Class.forName("com.intellij.util.ProfilingUtil").getDeclaredMethod("forceCaptureMemorySnapshot").invoke(null);
for (int i = 0; i < 1000; i++) {
PlatformTestUtil.tryGcSoftlyReachableObjects();
UIUtil.dispatchAllInvocationEvents();
if (documentManager.getCachedDocument(getPsiManager().findFile(vFile)) == null) break;
if (documentManager.getCachedDocument(findFile(vFile)) == null) break;
System.gc();
}
assertNull(documentManager.getCachedDocument(getPsiManager().findFile(vFile)));
assertNull(documentManager.getCachedDocument(findFile(vFile)));
Document newDoc = documentManager.getDocument(getPsiManager().findFile(vFile));
Document newDoc = documentManager.getDocument(findFile(vFile));
assertTrue(id != System.identityHashCode(newDoc));
}
private PsiFile findFile(@NotNull VirtualFile vFile) {
return getPsiManager().findFile(vFile);
}
public void testGetUncommittedDocuments_noDocuments() throws Exception {
assertEquals(0, getPsiDocumentManager().getUncommittedDocuments().length);
}
public void testGetUncommittedDocuments_documentChanged_DontProcessEvents() throws Exception {
final PsiFile file = getPsiManager().findFile(createFile());
final PsiFile file = findFile(createFile());
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
WriteCommandAction.runWriteCommandAction(null, () -> {
getPsiDocumentManager().getSynchronizer().performAtomically(file, () -> changeDocument(document, getPsiDocumentManager()));
@@ -164,19 +169,22 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
}
public void testCommitDocument_RemovesFromUncommittedList() throws Exception {
PsiFile file = getPsiManager().findFile(createFile());
PsiFile file = findFile(createFile());
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
WriteCommandAction.runWriteCommandAction(null, () -> {
changeDocument(document, getPsiDocumentManager());
});
getPsiDocumentManager().commitDocument(document);
assertEquals(0, getPsiDocumentManager().getUncommittedDocuments().length);
}
private Document getDocument(PsiFile file) {
return getPsiDocumentManager().getDocument(file);
}
private static void changeDocument(Document document, PsiDocumentManagerImpl manager) {
DocumentEventImpl event = new DocumentEventImpl(document, 0, "", "", document.getModificationStamp(), false);
manager.beforeDocumentChange(event);
@@ -184,9 +192,9 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
}
public void testCommitAllDocument_RemovesFromUncommittedList() throws Exception {
PsiFile file = getPsiManager().findFile(createFile());
PsiFile file = findFile(createFile());
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
WriteCommandAction.runWriteCommandAction(null, () -> {
changeDocument(document, getPsiDocumentManager());
@@ -198,9 +206,9 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
}
public void testDocumentFromAlienProjectDoesNotEndUpInMyUncommittedList() throws Exception {
PsiFile file = getPsiManager().findFile(createFile());
PsiFile file = findFile(createFile());
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
File temp = createTempDirectory();
final Project alienProject = createProject(temp + "/alien.ipr", DebugUtil.currentStackTrace());
@@ -216,7 +224,6 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
final PsiFile alienFile = alienManager.findFile(alienVirt);
final PsiDocumentManagerImpl alienDocManager = (PsiDocumentManagerImpl)PsiDocumentManager.getInstance(alienProject);
final Document alienDocument = alienDocManager.getDocument(alienFile);
//alienDocument.putUserData(CACHED_VIEW_PROVIDER, new MockFileViewProvider(alienFile));
assertEquals(0, alienDocManager.getUncommittedDocuments().length);
assertEquals(0, getPsiDocumentManager().getUncommittedDocuments().length);
@@ -244,10 +251,10 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
}
public void testCommitInBackground() {
PsiFile file = getPsiManager().findFile(createFile());
PsiFile file = findFile(createFile());
assertNotNull(file);
assertTrue(file.isPhysical());
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
assertNotNull(document);
final Semaphore semaphore = new Semaphore();
@@ -316,9 +323,9 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
public void testDocumentFromAlienProjectGetsCommittedInBackground() throws Exception {
LightVirtualFile virtualFile = createFile();
PsiFile file = getPsiManager().findFile(virtualFile);
PsiFile file = findFile(virtualFile);
final Document document = getPsiDocumentManager().getDocument(file);
final Document document = getDocument(file);
File temp = createTempDirectory();
final Project alienProject = createProject(temp + "/alien.ipr", DebugUtil.currentStackTrace());
@@ -380,26 +387,26 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
public void testFileChangesToText() throws IOException {
VirtualFile vFile = getVirtualFile(createTempFile("a.txt", "abc"));
PsiFile psiFile = getPsiManager().findFile(vFile);
Document document = getPsiDocumentManager().getDocument(psiFile);
PsiFile psiFile = findFile(vFile);
Document document = getDocument(psiFile);
rename(vFile, "a.xml");
assertFalse(psiFile.isValid());
assertNotSame(psiFile, getPsiManager().findFile(vFile));
psiFile = getPsiManager().findFile(vFile);
assertNotSame(psiFile, findFile(vFile));
psiFile = findFile(vFile);
assertSame(document, FileDocumentManager.getInstance().getDocument(vFile));
assertSame(document, getPsiDocumentManager().getDocument(psiFile));
assertSame(document, getDocument(psiFile));
}
public void testFileChangesToBinary() throws IOException {
VirtualFile vFile = getVirtualFile(createTempFile("a.txt", "abc"));
PsiFile psiFile = getPsiManager().findFile(vFile);
Document document = getPsiDocumentManager().getDocument(psiFile);
PsiFile psiFile = findFile(vFile);
Document document = getDocument(psiFile);
rename(vFile, "a.zip");
assertFalse(psiFile.isValid());
psiFile = getPsiManager().findFile(vFile);
psiFile = findFile(vFile);
assertInstanceOf(psiFile, PsiBinaryFile.class);
assertNoFileDocumentMapping(vFile, psiFile, document);
@@ -408,12 +415,12 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
public void testFileBecomesTooLarge() throws Exception {
VirtualFile vFile = getVirtualFile(createTempFile("a.txt", "abc"));
PsiFile psiFile = getPsiManager().findFile(vFile);
Document document = getPsiDocumentManager().getDocument(psiFile);
PsiFile psiFile = findFile(vFile);
Document document = getDocument(psiFile);
makeFileTooLarge(vFile);
assertFalse(psiFile.isValid());
psiFile = getPsiManager().findFile(vFile);
psiFile = findFile(vFile);
assertInstanceOf(psiFile, PsiLargeFile.class);
assertNoFileDocumentMapping(vFile, psiFile, document);
@@ -424,7 +431,7 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
assertNull(FileDocumentManager.getInstance().getDocument(vFile));
assertNull(FileDocumentManager.getInstance().getFile(document));
assertNull(getPsiDocumentManager().getPsiFile(document));
assertNull(getPsiDocumentManager().getDocument(psiFile));
assertNull(getDocument(psiFile));
}
private void makeFileTooLarge(final VirtualFile vFile) throws Exception {
@@ -437,8 +444,8 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
public void testCommitDocumentInModalDialog() throws IOException {
VirtualFile vFile = getVirtualFile(createTempFile("a.txt", "abc"));
PsiFile psiFile = getPsiManager().findFile(vFile);
final Document document = getPsiDocumentManager().getDocument(psiFile);
PsiFile psiFile = findFile(vFile);
final Document document = getDocument(psiFile);
final DialogWrapper dialog = new DialogWrapper(getProject()) {
@Nullable
@@ -513,4 +520,29 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase {
FileDocumentManager.getInstance().saveDocument(document);
assertEquals("-1\n2\n3\n", VfsUtilCore.loadText(file));
}
public void testPerformWhenAllCommittedMustNotNest() {
PsiFile file = findFile(createFile());
assertNotNull(file);
assertTrue(file.isPhysical());
final Document document = getDocument(file);
assertNotNull(document);
WriteCommandAction.runWriteCommandAction(null, () -> {
document.insertString(0, "class X {}");
});
getPsiDocumentManager().performWhenAllCommitted(() -> {
try {
getPsiDocumentManager().performWhenAllCommitted(() -> {
});
fail("Must fail");
}
catch (IncorrectOperationException ignored) {
}
});
getPsiDocumentManager().commitAllDocuments();
assertTrue(getPsiDocumentManager().isCommitted(document));
}
}