diff --git a/java/execution/impl/src/com/intellij/compiler/options/CompileStepBeforeRun.java b/java/execution/impl/src/com/intellij/compiler/options/CompileStepBeforeRun.java index 06862f7b66d3..5cbaf8aaabec 100644 --- a/java/execution/impl/src/com/intellij/compiler/options/CompileStepBeforeRun.java +++ b/java/execution/impl/src/com/intellij/compiler/options/CompileStepBeforeRun.java @@ -26,6 +26,7 @@ import com.intellij.execution.remote.RemoteConfiguration; import com.intellij.execution.runners.ExecutionEnvironment; import com.intellij.icons.AllIcons; import com.intellij.openapi.actionSystem.DataContext; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.compiler.CompileContext; import com.intellij.openapi.compiler.CompileScope; import com.intellij.openapi.compiler.CompileStatusNotification; @@ -140,7 +141,7 @@ public class CompileStepBeforeRun extends BeforeRunTaskProvider + * + * A transaction ensures that IntelliJ model (PSI, documents, VFS, project roots etc.) isn't modified in an unexpected way + * while working with it, with either read or write access. The main property of transactions is isolation: at most one transaction + * can be running at any given time. The code inside transaction can perform read or write actions and, more importantly, show dialogs + * and process UI events in other ways: it's guaranteed that no one will be able to sneak in with an unexpected model change using + * {@link javax.swing.SwingUtilities#invokeLater(Runnable)} or analogs.

+ * + * Transactions are run on UI thread. They have read access by default.

+ * + * The recommended way to perform a transaction is to invoke {@link #submitTransaction(Runnable)}. It either runs the transaction immediately + * (if on UI thread and there's no other transaction running) or queues it to invoke at some later moment, when it becomes possible.

+ * + * Sometimes transactions need to be processed immediately, even if another transaction is already running. Example: + * outer transaction has shown a dialog with an editor, and typing into that editor (which requires a transaction for changing document) + * should be allowed. For such cases, the framework should be notified which transaction kinds are allowed to merged into + * the main transaction and executed immediately. Use {@link #acceptNestedTransactions(TransactionKind...)} for that. Inner transactions + * should be given a non-null kind in such circumstances: {@link #submitMergeableTransaction(TransactionKind, Runnable)}. + * + * @see Application#runReadAction(Runnable) + * @see Application#runWriteAction(Runnable) + * @since 146.* + * @author peter + */ +public abstract class TransactionGuard { + public static TransactionGuard getInstance() { + return ServiceManager.getService(TransactionGuard.class); + } + + /** + * Ensures that some code will be run in a transaction. It's guaranteed that no other transactions are run at the same time. + * The code will be run on Swing thread immediately or after all other queued transactions (if any) have been completed.

+ * + * For more advanced version, see {@link #submitMergeableTransaction(TransactionKind, Runnable)} + * @param transaction code to execute inside a transaction. + */ + public static void submitTransaction(@NotNull Runnable transaction) { + getInstance().submitMergeableTransaction(null, transaction); + } + + /** + * Schedules a transaction and waits for it to be completed. Only allowed to be invoked on non-UI thread and outside read action. + * @see #submitMergeableTransaction(TransactionKind, Runnable) + * @param kind + * @param transaction + * @throws ProcessCanceledException if current thread is interrupted + */ + public abstract void submitTransactionAndWait(@Nullable TransactionKind kind, @NotNull Runnable transaction) throws ProcessCanceledException; + + /** + * A synchronous version of {@link #submitMergeableTransaction(TransactionKind, Runnable)}. + * @return a token object for this transaction. Call {@link AccessToken#finish()} (inside finally) when the transaction is complete. + */ + @NotNull + public abstract AccessToken startSynchronousTransaction(@Nullable TransactionKind kind); + + /** + * @return whether there's a transaction currently running + */ + public abstract boolean isInsideTransaction(); + + /** + * When on UI thread and there's no other transaction running, executes the given runnable. If there is a transaction running, + * but the given {@code kind} is allowed via {@link #acceptNestedTransactions(TransactionKind...)}, merges two transactions + * and executes the provided code immediately. Otherwise + * adds the runnable to a queue. When all transactions scheduled before this one are finished, executes the given + * runnable under a transaction. + * @param kind a kind object to enable transaction merging or null, if no merging is required. + * @param transaction code to execute inside a transaction. + */ + public abstract void submitMergeableTransaction(@Nullable TransactionKind kind, @NotNull Runnable transaction); + + /** + * Allow incoming transactions of the specified kinds to be executed immediately, instead of being queued until the current transaction is finished.

+ * + * Example: outer transaction has shown a dialog with an editor, and typing into that editor (which requires a transaction for changing document) + * should be allowed. + * @param kinds kinds of transactions to allow + * @return a token object for this session. Please call {@link AccessToken#finish()} (inside finally clause) when you don't want + * nested transactions anymore. + */ + @NotNull + public abstract AccessToken acceptNestedTransactions(TransactionKind... kinds); + + public static final class TransactionKind { + /** + * This kind represents document modifications via editor actions, code completion and document->PSI commit. + * @see com.intellij.psi.PsiDocumentManager#commitDocument(Document) + */ + public static final TransactionKind TEXT_EDITING = new TransactionKind("TEXT_EDITING"); + + /** + * This kind represents any model modifications: + *

  • PSI or document changes + *
  • Virtual file system changes, e.g. files created/deleted/renamed/content-changed, + * caused by refresh process or explicit operations. + *
  • Project root set change + *
  • Dumb mode (reindexing) start/finish, (see {@link com.intellij.openapi.project.DumbService}). + */ + public static final TransactionKind ANY_CHANGE = new TransactionKind("ANY_CHANGE"); + + private final String myName; + + public TransactionKind(@NotNull String name) { + myName = name; + } + + @Override + public String toString() { + return myName; + } + } +} diff --git a/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java b/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java new file mode 100644 index 000000000000..c445fd7357b0 --- /dev/null +++ b/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java @@ -0,0 +1,175 @@ +/* + * Copyright 2000-2016 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.openapi.application; + +import com.intellij.openapi.progress.ProcessCanceledException; +import com.intellij.util.concurrency.Semaphore; +import com.intellij.util.containers.ContainerUtil; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.List; +import java.util.Queue; +import java.util.Set; +import java.util.concurrent.LinkedBlockingQueue; + +/** + * @author peter + */ +public class TransactionGuardImpl extends TransactionGuard { + private final Queue myQueue = new LinkedBlockingQueue(); + private final Set myMergeableKinds = ContainerUtil.newHashSet(); + private boolean myInsideTransaction; + + @Override + @NotNull + public AccessToken startSynchronousTransaction(@Nullable TransactionKind kind) throws IllegalStateException { + ApplicationManager.getApplication().assertIsDispatchThread(); + if (kind != null && myMergeableKinds.contains(kind)) { + return AccessToken.EMPTY_ACCESS_TOKEN; + } + if (myInsideTransaction) { + throw new IllegalStateException("Nested transactions are not allowed"); + } + myInsideTransaction = true; + return new AccessToken() { + @Override + public void finish() { + myInsideTransaction = false; + if (!myQueue.isEmpty()) { + pollQueueLater(); + } + } + }; + } + + private void pollQueueLater() { + //todo replace with SwingUtilities when write actions are required to run under a guard + final Application app = ApplicationManager.getApplication(); + app.invokeLater(new Runnable() { + @Override + public void run() { + if (myInsideTransaction) return; + + Runnable next = myQueue.poll(); + if (next != null) { + runSyncTransaction(null, next); + } + } + }, app.getDisposed()); + } + + private void runSyncTransaction(@Nullable TransactionKind kind, @NotNull Runnable code) { + AccessToken token = startSynchronousTransaction(kind); + try { + code.run(); + } + finally { + token.finish(); + } + } + + @Override + public boolean isInsideTransaction() { + ApplicationManager.getApplication().assertIsDispatchThread(); + return myInsideTransaction; + } + + @Override + public void submitMergeableTransaction(@Nullable final TransactionKind kind, @NotNull final Runnable transaction) { + submitTransaction(kind, transaction, ModalityState.defaultModalityState()); + } + + public void submitTransaction(@Nullable final TransactionKind kind, + @NotNull final Runnable transaction, + ModalityState modalityState) { + Runnable runnable = new Runnable() { + @Override + public void run() { + if (!myInsideTransaction || kind != null && myMergeableKinds.contains(kind)) { + runSyncTransaction(kind, transaction); + } + else { + myQueue.offer(transaction); + pollQueueLater(); + } + } + }; + + //todo replace with SwingUtilities when write actions are required to run under a guard + final Application app = ApplicationManager.getApplication(); + if (app.isDispatchThread()) { + runnable.run(); + } else { + app.invokeLater(runnable, modalityState, app.getDisposed()); + } + } + + @Override + @NotNull + public AccessToken acceptNestedTransactions(TransactionKind... kinds) { + if (!isInsideTransaction()) { + throw new IllegalStateException("acceptNestedTransactions must be called inside a transaction"); + } + final List toRemove = ContainerUtil.newArrayList(); + for (TransactionKind kind : kinds) { + if (myMergeableKinds.add(kind)) { + toRemove.add(kind); + } + } + return new AccessToken() { + @Override + public void finish() { + myMergeableKinds.removeAll(toRemove); + } + }; + } + + @Override + public void submitTransactionAndWait(@Nullable TransactionKind kind, @NotNull final Runnable transaction) throws ProcessCanceledException { + submitTransactionAndWait(kind, transaction, ModalityState.defaultModalityState()); + } + + public void submitTransactionAndWait(@Nullable TransactionKind kind, + @NotNull final Runnable transaction, + ModalityState modalityState) { + Application app = ApplicationManager.getApplication(); + assert !app.isDispatchThread() : "submitTransactionAndWait should not be invoked on dispatch thread"; + assert !app.isReadAccessAllowed() : "submitTransactionAndWait should not be invoked from a read action"; + + final Semaphore semaphore = new Semaphore(); + semaphore.down(); + final Throwable[] exception = {null}; + submitTransaction(kind, new Runnable() { + @Override + public void run() { + try { + transaction.run(); + } + catch (Throwable e) { + exception[0] = e; + } + finally { + semaphore.up(); + } + } + }, modalityState); + semaphore.waitFor(); + if (exception[0] != null) { + throw new RuntimeException(exception[0]); + } + } +} diff --git a/platform/lang-impl/src/com/intellij/codeInsight/completion/CompletionProgressIndicator.java b/platform/lang-impl/src/com/intellij/codeInsight/completion/CompletionProgressIndicator.java index eeb288df46e4..c43de979179d 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/completion/CompletionProgressIndicator.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/completion/CompletionProgressIndicator.java @@ -33,6 +33,7 @@ import com.intellij.openapi.Disposable; import com.intellij.openapi.actionSystem.IdeActions; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.Result; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.command.CommandProcessor; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.diagnostic.Logger; @@ -482,12 +483,8 @@ public class CompletionProgressIndicator extends ProgressIndicatorBase implement void disposeIndicator() { // our offset map should be disposed under write action, so that duringCompletion (read action) won't access it after disposing - ApplicationManager.getApplication().runWriteAction(new Runnable() { - @Override - public void run() { - Disposer.dispose(CompletionProgressIndicator.this); - } - }); + TransactionGuard.getInstance().submitMergeableTransaction(TransactionGuard.TransactionKind.TEXT_EDITING, () -> + ApplicationManager.getApplication().runWriteAction(() -> Disposer.dispose(this))); } @TestOnly diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DefaultHighlightInfoProcessor.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DefaultHighlightInfoProcessor.java index d83d69b45453..fd9b231d677c 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DefaultHighlightInfoProcessor.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DefaultHighlightInfoProcessor.java @@ -17,6 +17,7 @@ package com.intellij.codeInsight.daemon.impl; import com.intellij.codeHighlighting.Pass; import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.editor.colors.EditorColorsScheme; @@ -53,7 +54,7 @@ public class DefaultHighlightInfoProcessor extends HighlightInfoProcessor { final TextRange priorityIntersection = priorityRange.intersection(restrictRange); final Editor editor = session.getEditor(); - UIUtil.invokeLaterIfNeeded(new Runnable() { + TransactionGuard.submitTransaction(new Runnable() { @Override public void run() { if (project.isDisposed() || modificationStamp != document.getModificationStamp()) return; diff --git a/platform/platform-impl/src/com/intellij/ide/SaveAndSyncHandlerImpl.java b/platform/platform-impl/src/com/intellij/ide/SaveAndSyncHandlerImpl.java index dd0908afd837..40abb126da78 100644 --- a/platform/platform-impl/src/com/intellij/ide/SaveAndSyncHandlerImpl.java +++ b/platform/platform-impl/src/com/intellij/ide/SaveAndSyncHandlerImpl.java @@ -18,6 +18,7 @@ package com.intellij.ide; import com.intellij.openapi.Disposable; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ModalityState; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.application.impl.LaterInvocator; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.fileEditor.FileDocumentManager; @@ -100,10 +101,12 @@ public class SaveAndSyncHandlerImpl extends SaveAndSyncHandler implements Dispos @Override public void onFrameDeactivated() { LOG.debug("save(): enter"); - if (canSyncOrSave()) { - saveProjectsAndDocuments(); - } - LOG.debug("save(): exit"); + TransactionGuard.submitTransaction(() -> { + if (canSyncOrSave()) { + saveProjectsAndDocuments(); + } + LOG.debug("save(): exit"); + }); } @Override diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java index d8f73d67d1fc..b00595645c10 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java @@ -57,6 +57,7 @@ import com.intellij.openapi.ui.Messages; import com.intellij.openapi.util.*; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.util.io.FileUtilRt; +import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.CharsetToolkit; import com.intellij.openapi.wm.WindowManager; @@ -1225,6 +1226,10 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App private void startWrite(/*@NotNull*/ Class clazz) { assertIsDispatchThread(getStatus(), "Write access is allowed from event dispatch thread only"); HeavyProcessLatch.INSTANCE.stopThreadPrioritizing(); // let non-cancellable read actions complete faster, if present + if (!TransactionGuard.getInstance().isInsideTransaction() && Registry.is("ide.require.transaction.for.model.changes", false)) { + LOG.error("Write access is allowed from model transactions only, see TransactionGuard documentation for details"); + //todo throw new IllegalStateException("Write access is allowed from model transactions only, see TransactionGuard documentation for details"); + } boolean writeActionPending = myWriteActionPending; myWriteActionPending = true; if (gatherWriteActionStatistics && myWriteActionsStack.isEmpty()) { diff --git a/platform/platform-resources/src/META-INF/PlatformExtensions.xml b/platform/platform-resources/src/META-INF/PlatformExtensions.xml index 5c87ca020e05..44e4a5f7dde2 100644 --- a/platform/platform-resources/src/META-INF/PlatformExtensions.xml +++ b/platform/platform-resources/src/META-INF/PlatformExtensions.xml @@ -33,6 +33,8 @@ serviceImplementation="com.intellij.openapi.fileChooser.impl.FileChooserFactoryImpl"/> + diff --git a/platform/util/resources/misc/registry.properties b/platform/util/resources/misc/registry.properties index bae7bcc64dce..aa40668a7186 100644 --- a/platform/util/resources/misc/registry.properties +++ b/platform/util/resources/misc/registry.properties @@ -716,5 +716,8 @@ idea.io.safe.sync.description=When "Safe Write" is enabled, sync() is invoked af ide.prioritize.ui.thread=false ide.prioritize.ui.thread.description=In presence of UI activity, deprioritizes all other threads for the activity to complete ASAP. Changing requires restart. +ide.require.transaction.for.model.changes=false +ide.require.transaction.for.model.changes.description=Whether write action can only happen under TransactionGuard + dumb.aware.run.configurations=false dumb.aware.run.configurations.description=Enable executing run configurations in dumb mode 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 6d33c46ab480..5a7784641455 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 @@ -19,6 +19,7 @@ import com.intellij.diff.util.DiffUtil; import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationAdapter; import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.TransactionGuard; import com.intellij.openapi.command.undo.UndoConstants; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; @@ -387,7 +388,7 @@ public class LineStatusTracker { } private void markFileUnchanged() { - ApplicationManager.getApplication().invokeLater(new Runnable() { + TransactionGuard.submitTransaction(new Runnable() { @Override public void run() { FileDocumentManager.getInstance().saveDocument(myDocument);