From 1480f62e0bb86c68edd68631392ca2c5982f7dbe Mon Sep 17 00:00:00 2001 From: peter Date: Thu, 10 Mar 2016 14:40:59 +0100 Subject: [PATCH] allow to commit non-physical psi outside write action --- .../intellij/pom/core/impl/PomModelImpl.java | 2 +- .../psi/impl/CommitToPsiFileAction.java | 33 ------- .../psi/impl/DocumentCommitThread.java | 91 +++++++++++++------ .../psi/impl/PsiDocumentManagerBase.java | 24 ++++- .../psi/impl/PsiToDocumentSynchronizer.java | 2 +- .../psi/impl/PsiDocumentManagerImpl.java | 4 +- .../psi/impl/PsiDocumentManagerImplTest.java | 39 ++++++++ 7 files changed, 128 insertions(+), 67 deletions(-) delete mode 100644 platform/core-impl/src/com/intellij/psi/impl/CommitToPsiFileAction.java diff --git a/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java b/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java index a45b88b060e9..d28b1c3241ed 100644 --- a/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java +++ b/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java @@ -251,7 +251,7 @@ public class PomModelImpl extends UserDataHolderBase implements PomModel { } if (containingFileByTree != null) { boolean isFromCommit = ApplicationManager.getApplication().isDispatchThread() && - ApplicationManager.getApplication().hasWriteAction(CommitToPsiFileAction.class); + ((PsiDocumentManagerBase)PsiDocumentManager.getInstance(myProject)).isCommitInProgress(); if (!isFromCommit && !synchronizer.isIgnorePsiEvents()) { reparseParallelTrees(containingFileByTree); if (docSynced) { diff --git a/platform/core-impl/src/com/intellij/psi/impl/CommitToPsiFileAction.java b/platform/core-impl/src/com/intellij/psi/impl/CommitToPsiFileAction.java deleted file mode 100644 index f3966bdbb38d..000000000000 --- a/platform/core-impl/src/com/intellij/psi/impl/CommitToPsiFileAction.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * 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. - */ - -package com.intellij.psi.impl; - -import com.intellij.openapi.editor.Document; -import com.intellij.openapi.editor.DocumentRunnable; -import com.intellij.openapi.project.Project; -import com.intellij.psi.IgnorePsiEventsMarker; - - -public abstract class CommitToPsiFileAction extends DocumentRunnable implements IgnorePsiEventsMarker { - protected CommitToPsiFileAction(Document document, Project project) { - super(document,project); - } -} - - - - diff --git a/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java b/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java index 881e5a29bf49..cd50b88fb056 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java +++ b/platform/core-impl/src/com/intellij/psi/impl/DocumentCommitThread.java @@ -19,9 +19,7 @@ import com.intellij.diagnostic.ThreadDumper; import com.intellij.lang.ASTNode; import com.intellij.lang.FileASTNode; import com.intellij.openapi.Disposable; -import com.intellij.openapi.application.ApplicationAdapter; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.application.*; import com.intellij.openapi.application.ex.ApplicationEx; import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.diagnostic.Attachment; @@ -75,6 +73,8 @@ import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; public class DocumentCommitThread implements Runnable, Disposable, DocumentCommitProcessor { private static final Logger LOG = Logger.getInstance("#com.intellij.psi.impl.DocumentCommitThread"); @@ -471,14 +471,29 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi throw new RuntimeException(s); } - CommitTask task = createNewTaskAndCancelSimilar(project, document, getAllFileNodes(psiFile), "Sync commit", ModalityState.current()); - assert !task.indicator.isCanceled(); - Pair result = commitUnderProgress(task, true); - Runnable finish = result.first; - log(project, "Committed sync", task, finish, task.indicator); - assert finish != null; + List> allFileNodes = getAllFileNodes(psiFile); - finish.run(); + Lock documentLock = getDocumentLock(document); + + CommitTask task; + synchronized (lock) { + // synchronized to ensure no new similar tasks can start before we hold the document's lock + task = createNewTaskAndCancelSimilar(project, document, allFileNodes, "Sync commit", ModalityState.current()); + documentLock.lock(); + } + + try { + assert !task.indicator.isCanceled(); + Pair result = commitUnderProgress(task, true); + Runnable finish = result.first; + log(project, "Committed sync", task, finish, task.indicator); + assert finish != null; + + finish.run(); + } + finally { + documentLock.unlock(); + } // will wake itself up on write action end } @@ -525,29 +540,40 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi myApplication.assertReadAccessAllowed(); if (project.isDisposed()) return; - if (documentManager.isCommitted(document)) return; - - if (!task.isStillValid()) { - task.cancel("Task invalidated", DocumentCommitThread.this); + Lock lock = getDocumentLock(document); + if (!lock.tryLock()) { + task.cancel("Can't obtain document lock", DocumentCommitThread.this); return; } - FileViewProvider viewProvider = documentManager.getCachedViewProvider(document); - if (viewProvider == null) { - finishProcessors.add(handleCommitWithoutPsi(documentManager, task)); - return; - } + try { + if (documentManager.isCommitted(document)) return; - for (Pair pair : task.myOldFileNodes) { - PsiFileImpl file = pair.first; - if (file.isValid()) { - FileASTNode oldFileNode = pair.second; - Processor finishProcessor = doCommit(task, file, oldFileNode); - if (finishProcessor != null) { - finishProcessors.add(finishProcessor); + if (!task.isStillValid()) { + task.cancel("Task invalidated", DocumentCommitThread.this); + return; + } + + FileViewProvider viewProvider = documentManager.getCachedViewProvider(document); + if (viewProvider == null) { + finishProcessors.add(handleCommitWithoutPsi(documentManager, task)); + return; + } + + for (Pair pair : task.myOldFileNodes) { + PsiFileImpl file = pair.first; + if (file.isValid()) { + FileASTNode oldFileNode = pair.second; + Processor finishProcessor = doCommit(task, file, oldFileNode); + if (finishProcessor != null) { + finishProcessors.add(finishProcessor); + } } } } + finally { + lock.unlock(); + } } }; if (synchronously) { @@ -742,7 +768,9 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi return new Processor() { @Override public boolean process(Document document) { - ApplicationManager.getApplication().assertWriteAccessAllowed(); + if (file.isPhysical()) { + ApplicationManager.getApplication().assertWriteAccessAllowed(); + } if (!task.isStillValid() || ((PsiDocumentManagerBase)PsiDocumentManager.getInstance(file.getProject())).getCachedViewProvider(document) != file.getViewProvider()) { return false; // optimistic locking failed @@ -887,4 +915,13 @@ public class DocumentCommitThread implements Runnable, Disposable, DocumentCommi } } } + + /** + * @return an internal lock object to prevent read & write phases of commit from running simultaneously for free-threaded PSI + */ + private static Lock getDocumentLock(Document document) { + Lock lock = document.getUserData(DOCUMENT_LOCK); + return lock != null ? lock : ((UserDataHolderEx)document).putUserDataIfAbsent(DOCUMENT_LOCK, new ReentrantLock()); + } + private static final Key DOCUMENT_LOCK = Key.create("DOCUMENT_LOCK"); } diff --git a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java index b896505433a3..dbfb1a2909ce 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java @@ -27,6 +27,7 @@ import com.intellij.openapi.application.impl.ApplicationInfoImpl; import com.intellij.openapi.components.ProjectComponent; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; +import com.intellij.openapi.editor.DocumentRunnable; import com.intellij.openapi.editor.event.DocumentAdapter; import com.intellij.openapi.editor.event.DocumentEvent; import com.intellij.openapi.editor.event.DocumentListener; @@ -153,6 +154,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen return ((PsiManagerEx)myPsiManager).getFileManager().findCachedViewProvider(virtualFile); } + @Nullable private static VirtualFile getVirtualFile(@NotNull Document document) { final VirtualFile virtualFile = FileDocumentManager.getInstance().getFile(document); if (virtualFile == null || !virtualFile.isValid()) return null; @@ -261,7 +263,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen return true; } if (myUncommittedDocuments.isEmpty()) { - if (!ApplicationManager.getApplication().hasWriteAction(CommitToPsiFileAction.class)) { + if (!isCommitInProgress()) { // in case of fireWriteActionFinished() we didn't execute 'actionsWhenAllDocumentsAreCommitted' yet assert actionsWhenAllDocumentsAreCommitted.isEmpty() : actionsWhenAllDocumentsAreCommitted; } @@ -300,12 +302,18 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen @NotNull final Object reason) { assert !myProject.isDisposed() : "Already disposed"; final boolean[] ok = {true}; - ApplicationManager.getApplication().runWriteAction(new CommitToPsiFileAction(document, myProject) { + Runnable runnable = new DocumentRunnable(document, myProject) { @Override public void run() { ok[0] = finishCommitInWriteAction(document, finishProcessors, synchronously); } - }); + }; + if (synchronously) { + runnable.run(); + } + else { + ApplicationManager.getApplication().runWriteAction(runnable); + } if (ok[0]) { // otherwise changes maybe not synced to the document yet, and injectors will crash @@ -408,7 +416,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen } }; - if (Boolean.TRUE.equals(psiFile.getViewProvider().getVirtualFile().getUserData(SingleRootFileViewProvider.FREE_THREADED))) { + if (isFreeThreaded(psiFile.getViewProvider().getVirtualFile())) { runnable.run(); } else { @@ -416,6 +424,14 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen } } + private static boolean isFreeThreaded(@NotNull VirtualFile file) { + return Boolean.TRUE.equals(file.getUserData(SingleRootFileViewProvider.FREE_THREADED)); + } + + public boolean isCommitInProgress() { + return myIsCommitInProgress; + } + @Override public T commitAndRunReadAction(@NotNull final Computable computation) { final Ref ref = Ref.create(null); diff --git a/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java b/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java index 5c34ed2c4b95..391df3ab683d 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java @@ -209,7 +209,7 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { } public boolean toProcessPsiEvent() { - return !myIgnorePsiEvents && !ApplicationManager.getApplication().hasWriteAction(IgnorePsiEventsMarker.class); + return !myIgnorePsiEvents && !myPsiDocumentManager.isCommitInProgress() && !ApplicationManager.getApplication().hasWriteAction(IgnorePsiEventsMarker.class); } @TestOnly diff --git a/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java b/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java index d60418926487..51c34633c526 100644 --- a/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java +++ b/platform/lang-impl/src/com/intellij/psi/impl/PsiDocumentManagerImpl.java @@ -136,7 +136,9 @@ public class PsiDocumentManagerImpl extends PsiDocumentManagerBase implements Se protected boolean finishCommitInWriteAction(@NotNull Document document, @NotNull List> finishProcessors, boolean synchronously) { - EditorWindowImpl.disposeInvalidEditors(); // in write action + if (ApplicationManager.getApplication().isWriteAccessAllowed()) { // can be false for non-physical PSI + EditorWindowImpl.disposeInvalidEditors(); + } return super.finishCommitInWriteAction(document, finishProcessors, synchronously); } diff --git a/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java b/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java index e9f5629de441..89cb0d6f4b77 100644 --- a/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/psi/impl/PsiDocumentManagerImplTest.java @@ -660,4 +660,43 @@ public class PsiDocumentManagerImplTest extends PlatformTestCase { System.out.println("i = " + i); } } + + public void testCommitNonPhysicalPsiWithoutWriteAction() throws IOException { + assertFalse(ApplicationManager.getApplication().isWriteAccessAllowed()); + + PsiFile original = getPsiManager().findFile(getVirtualFile(createTempFile("X.txt", ""))); + assertNotNull(original); + assertTrue(original.getViewProvider().isEventSystemEnabled()); + + long modCount = getPsiManager().getModificationTracker().getModificationCount(); + + PsiFile copy = (PsiFile)original.copy(); + assertFalse(copy.getViewProvider().isEventSystemEnabled()); + + Document document = copy.getViewProvider().getDocument(); + assertNotNull(document); + document.setText("class A{}"); + + PsiDocumentManager.getInstance(myProject).commitDocument(document); + assertEquals(modCount, getPsiManager().getModificationTracker().getModificationCount()); + assertEquals(document.getText(), copy.getText()); + assertTrue(PsiDocumentManager.getInstance(myProject).isCommitted(document)); + } + + public void testCommitNonPhysicalCopyOnPerformWhenAllCommitted() throws Exception { + assertFalse(ApplicationManager.getApplication().isWriteAccessAllowed()); + + PsiFile original = getPsiManager().findFile(getVirtualFile(createTempFile("X.txt", ""))); + assertNotNull(original); + PsiFile copy = (PsiFile)original.copy(); + assertEquals("", copy.getText()); + Document document = copy.getViewProvider().getDocument(); + assertNotNull(document); + + document.setText("class A{}"); + PsiDocumentManager.getInstance(myProject).performWhenAllCommitted(() -> assertEquals(document.getText(), copy.getText())); + DocumentCommitThread.getInstance().waitForAllCommits(); + assertTrue(PsiDocumentManager.getInstance(myProject).isCommitted(document)); + } + }