allow write actions from app invokeLater used in an orthodox way, this way most code won't need rewriting to transactions

platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java

@@ -27,8 +27,10 @@ import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.Collections;
 import java.util.Map;
 import java.util.Queue;
+import java.util.Set;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -38,10 +40,15 @@ import java.util.concurrent.atomic.AtomicLong;
 public class TransactionGuardImpl extends TransactionGuard {
   private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.application.TransactionGuardImpl");
   private final Queue<Transaction> myQueue = new LinkedBlockingQueue<Transaction>();
-  private final Map<ModalityState, TransactionIdImpl> myModalities = ContainerUtil.createConcurrentWeakMap();
+  private final Map<ModalityState, TransactionIdImpl> myModality2Transaction = ContainerUtil.createConcurrentWeakMap();
+  private final Set<ModalityState> myWriteSafeModalities = Collections.newSetFromMap(ContainerUtil.<ModalityState, Boolean>createConcurrentWeakMap());
   private TransactionIdImpl myCurrentTransaction;
   private boolean myWritingAllowed;
 
+  public TransactionGuardImpl() {
+    myWriteSafeModalities.add(ModalityState.NON_MODAL);
+  }
+
   @Override
   @NotNull
   public AccessToken startSynchronousTransaction(@NotNull TransactionKind kind) throws IllegalStateException {
@@ -263,7 +270,7 @@ public class TransactionGuardImpl extends TransactionGuard {
   }
 
   public boolean isWriteActionAllowed() {
-    return !Registry.is("ide.require.transaction.for.model.changes", false) || isInsideTransaction() || myWritingAllowed;
+    return !Registry.is("ide.require.transaction.for.model.changes", false) || myWritingAllowed;
   }
 
   @Override
@@ -282,7 +289,7 @@ public class TransactionGuardImpl extends TransactionGuard {
   public TransactionIdImpl getContextTransaction() {
     if (!ApplicationManager.getApplication().isDispatchThread()) {
       ProgressIndicator indicator = ProgressIndicatorProvider.getGlobalProgressIndicator();
-      return indicator != null ? myModalities.get(indicator.getModalityState()) : null;
+      return indicator != null ? myModality2Transaction.get(indicator.getModalityState()) : null;
     }
 
     return myWritingAllowed ? myCurrentTransaction : null;
@@ -291,13 +298,36 @@ public class TransactionGuardImpl extends TransactionGuard {
   public void enteredModality(@NotNull ModalityState modality) {
     TransactionIdImpl contextTransaction = getContextTransaction();
     if (contextTransaction != null) {
-      myModalities.put(modality, contextTransaction);
+      myModality2Transaction.put(modality, contextTransaction);
+    }
+    if (myWritingAllowed) {
+      myWriteSafeModalities.add(modality);
     }
   }
 
   @Nullable
   public TransactionIdImpl getModalityTransaction(@NotNull ModalityState modalityState) {
-    return myModalities.get(modalityState);
+    return myModality2Transaction.get(modalityState);
+  }
+
+  @NotNull
+  public Runnable wrapLaterInvocation(@NotNull final Runnable runnable, @NotNull ModalityState modalityState) {
+    if (myWriteSafeModalities.contains(modalityState)) {
+      return new Runnable() {
+        @Override
+        public void run() {
+          final boolean prev = myWritingAllowed;
+          myWritingAllowed = true;
+          try {
+            runnable.run();
+          } finally {
+            myWritingAllowed = prev;
+          }
+        }
+      };
+    }
+
+    return runnable;
   }
 
   private static class Transaction {

platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java

@@ -435,22 +435,22 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App
 
   @Override
   public void invokeLater(@NotNull final Runnable runnable) {
-    myInvokator.invokeLater(runnable);
+    invokeLater(runnable, getDisposed());
   }
 
   @Override
   public void invokeLater(@NotNull final Runnable runnable, @NotNull final Condition expired) {
-    myInvokator.invokeLater(runnable, expired);
+    invokeLater(runnable, ModalityState.defaultModalityState(), expired);
   }
 
   @Override
   public void invokeLater(@NotNull final Runnable runnable, @NotNull final ModalityState state) {
-    myInvokator.invokeLater(runnable, state);
+    invokeLater(runnable, state, getDisposed());
   }
 
   @Override
   public void invokeLater(@NotNull final Runnable runnable, @NotNull final ModalityState state, @NotNull final Condition expired) {
-    myInvokator.invokeLater(runnable, state, expired);
+    myInvokator.invokeLater(((TransactionGuardImpl)TransactionGuard.getInstance()).wrapLaterInvocation(runnable, state), state, expired);
   }
 
   @Override
@@ -737,7 +737,7 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App
       LOG.error("Calling invokeAndWait from read-action leads to possible deadlock.");
     }
 
-    LaterInvocator.invokeAndWait(runnable, modalityState);
+    LaterInvocator.invokeAndWait(((TransactionGuardImpl)TransactionGuard.getInstance()).wrapLaterInvocation(runnable, modalityState), modalityState);
   }
 
   @Override

platform/platform-impl/src/com/intellij/openapi/application/impl/LaterInvocator.java

@@ -236,7 +236,7 @@ public class LaterInvocator {
   }
 
   @TestOnly
-  static void leaveAllModals() {
+  public static void leaveAllModals() {
     ourModalEntities.clear();
     while (ourModalityStack.size() > 1) {
       ourModalityStack.pop();

platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy

@@ -1,10 +1,7 @@
 package com.intellij.application
 
-import com.intellij.openapi.application.Application
-import com.intellij.openapi.application.ApplicationManager
-import com.intellij.openapi.application.TransactionGuard
-import com.intellij.openapi.application.TransactionGuardImpl
-import com.intellij.openapi.application.TransactionId
+import com.intellij.openapi.application.*
+import com.intellij.openapi.application.impl.LaterInvocator
 import com.intellij.openapi.progress.ProgressManager
 import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.util.registry.Registry
@@ -38,6 +35,7 @@ class TransactionTest extends LightPlatformTestCase {
   protected void tearDown() throws Exception {
     Registry.get("ide.require.transaction.for.model.changes").resetToDefault()
     log.clear()
+    LaterInvocator.leaveAllModals()
     super.tearDown()
   }
 
@@ -45,18 +43,23 @@ class TransactionTest extends LightPlatformTestCase {
     assert app.isDispatchThread()
     assert !app.isWriteAccessAllowed()
 
-    SwingUtilities.invokeLater {
-      try {
-        app.runWriteAction {}
-        fail()
-      }
-      catch (AssertionError ignore) {
-        // a trace is also printed to stderr, which is expected
-      }
-    }
+    SwingUtilities.invokeLater { assertWritingProhibited() }
     UIUtil.dispatchAllInvocationEvents()
   }
 
+  private void assertWritingProhibited() {
+    boolean writeActionFailed = false
+    try {
+      app.runWriteAction { log << 'writing' }
+    }
+    catch (AssertionError ignore) {
+      writeActionFailed = true
+    }
+    if (!writeActionFailed) {
+      fail('write action should fail')
+    }
+  }
+
   public void "test parent disposable"() {
     def parent = Disposer.newDisposable()
 
@@ -176,4 +179,35 @@ class TransactionTest extends LightPlatformTestCase {
     }
   }
 
+  public void "test write access in modal invokeLater"() {
+    LaterInvocator.enterModal(new Object())
+    UIUtil.dispatchAllInvocationEvents()
+    def unsafeModality = ModalityState.current()
+
+    TransactionGuard.submitTransaction testRootDisposable, {
+      log << '1'
+
+      LaterInvocator.enterModal(new Object())
+      def safeModality = ModalityState.current()
+      app.executeOnPooledThread({
+        app.invokeLater({
+                          app.runWriteAction { log << '2' }
+                        }, safeModality)
+        app.invokeLater({
+                          assertWritingProhibited()
+                          log << '4'
+                        }, unsafeModality)
+        app.invokeLater({
+                          assertWritingProhibited()
+                          log << '3'
+                        }, ModalityState.any())
+        app.invokeLater({ app.runWriteAction { log << '5' } }, ModalityState.NON_MODAL)
+      }).get()
+      UIUtil.dispatchAllInvocationEvents()
+      LaterInvocator.leaveAllModals()
+      UIUtil.dispatchAllInvocationEvents()
+      assert log == ['1', '2', '3', '4', '5']
+    }
+  }
+
 }