prohibit write actions from user activity inside unsafe invokeLater; run transactions ASAP in any modality state

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

@@ -16,6 +16,7 @@
 package com.intellij.openapi.application;
 
 import com.intellij.openapi.Disposable;
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.progress.ProcessCanceledException;
 import com.intellij.openapi.progress.ProgressIndicator;
 import com.intellij.openapi.progress.ProgressIndicatorProvider;
@@ -37,11 +38,17 @@ import java.util.concurrent.atomic.AtomicLong;
  * @author peter
  */
 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> myModality2Transaction = ContainerUtil.createConcurrentWeakMap();
   private final Set<ModalityState> myWriteSafeModalities = Collections.newSetFromMap(ContainerUtil.<ModalityState, Boolean>createConcurrentWeakMap());
   private TransactionIdImpl myCurrentTransaction;
   private boolean myWritingAllowed;
+  /**
+   * A modality started when writing is not allowed (e.g. SwingUtilities.invokeLater). Writing isn't allowed for
+   * all dialogs inside this modality, even from user activity.
+   */
+  private Object myUnsafeModality;
 
   public TransactionGuardImpl() {
     myWriteSafeModalities.add(ModalityState.NON_MODAL);
@@ -50,10 +57,12 @@ public class TransactionGuardImpl extends TransactionGuard {
   @NotNull
   private AccessToken startTransactionUnchecked() {
     final TransactionIdImpl prevTransaction = myCurrentTransaction;
+    final Object prevUnsafeModality = myUnsafeModality;
     final boolean wasWritingAllowed = myWritingAllowed;
 
     myWritingAllowed = true;
     myCurrentTransaction = new TransactionIdImpl();
+    myUnsafeModality = null;
 
     return new AccessToken() {
       @Override
@@ -67,6 +76,7 @@ public class TransactionGuardImpl extends TransactionGuard {
 
         myWritingAllowed = wasWritingAllowed;
         myCurrentTransaction = prevTransaction;
+        myUnsafeModality = prevUnsafeModality;
       }
     };
   }
@@ -77,7 +87,6 @@ public class TransactionGuardImpl extends TransactionGuard {
   }
 
   private void pollQueueLater() {
-    //todo replace with SwingUtilities when write actions are required to run under a guard
     ApplicationManager.getApplication().invokeLater(new Runnable() {
       @Override
       public void run() {
@@ -88,7 +97,7 @@ public class TransactionGuardImpl extends TransactionGuard {
           runSyncTransaction(next);
         }
       }
-    });
+    }, ModalityState.any());
   }
 
   private void runSyncTransaction(@NotNull Transaction transaction) {
@@ -125,8 +134,7 @@ public class TransactionGuardImpl extends TransactionGuard {
     if (isDispatchThread) {
       runnable.run();
     } else {
-      //todo add ModalityState.any() when write actions are required to run under a guard
-      app.invokeLater(runnable);
+      app.invokeLater(runnable, ModalityState.any());
     }
   }
 
@@ -208,12 +216,13 @@ public class TransactionGuardImpl extends TransactionGuard {
    */
   @NotNull
   public AccessToken startActivity(boolean userActivity) {
-    if (myWritingAllowed == userActivity) {
+    boolean allowWriting = userActivity && myUnsafeModality == null;
+    if (myWritingAllowed == allowWriting) {
       return AccessToken.EMPTY_ACCESS_TOKEN;
     }
 
     final boolean prev = myWritingAllowed;
-    myWritingAllowed = userActivity;
+    myWritingAllowed = allowWriting;
     return new AccessToken() {
       @Override
       public void finish() {
@@ -235,7 +244,7 @@ public class TransactionGuardImpl extends TransactionGuard {
       public void run() {
         submitTransaction(parentDisposable, id, transaction);
       }
-    });
+    }, ModalityState.any());
   }
 
   @Override
@@ -248,13 +257,22 @@ public class TransactionGuardImpl extends TransactionGuard {
     return myWritingAllowed ? myCurrentTransaction : null;
   }
 
-  public void enteredModality(@NotNull ModalityState modality) {
+  public void enteredModality(@NotNull ModalityState modality, @NotNull Object modalityObject) {
     TransactionIdImpl contextTransaction = getContextTransaction();
     if (contextTransaction != null) {
       myModality2Transaction.put(modality, contextTransaction);
     }
     if (myWritingAllowed) {
       myWriteSafeModalities.add(modality);
+    } else if (myUnsafeModality == null) {
+      myUnsafeModality = modalityObject;
+    }
+  }
+
+  public void leftModality(@NotNull Object modalityObject) {
+    if (myUnsafeModality == modalityObject) {
+      myUnsafeModality = null;
+      LOG.assertTrue(!myWritingAllowed, modalityObject);
     }
   }
 

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

@@ -193,7 +193,7 @@ public class LaterInvocator {
 
     TransactionGuardImpl guard = IdeaApplication.isLoaded() ? (TransactionGuardImpl)TransactionGuard.getInstance() : null;
     if (guard != null) {
-      guard.enteredModality(ourModalityStack.peek());
+      guard.enteredModality(ourModalityStack.peek(), modalEntity);
     }
   }
 
@@ -215,6 +215,12 @@ public class LaterInvocator {
     }
     LOG.assertTrue(removed, modalEntity);
     LOG.assertTrue(!ourModalityStack.isEmpty());
+
+    TransactionGuardImpl guard = IdeaApplication.isLoaded() ? (TransactionGuardImpl)TransactionGuard.getInstance() : null;
+    if (guard != null) {
+      guard.leftModality(modalEntity);
+    }
+
     cleanupQueueForModal(modalEntity);
     ourQueueSkipCount = 0;
     requestFlush();
@@ -237,9 +243,8 @@ public class LaterInvocator {
 
   @TestOnly
   public static void leaveAllModals() {
-    ourModalEntities.clear();
-    while (ourModalityStack.size() > 1) {
-      ourModalityStack.pop();
+    while (!ourModalEntities.isEmpty()) {
+      leaveModal(ourModalEntities.get(ourModalEntities.size() - 1));
     }
     LOG.assertTrue(getCurrentModalityState() == ModalityState.NON_MODAL, getCurrentModalityState());
     ourQueueSkipCount = 0;

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

@@ -40,14 +40,32 @@ class TransactionTest extends LightPlatformTestCase {
     super.tearDown()
   }
 
-  public void "test write action in invokeLater requires transaction"() {
+  public void "test write action without transaction prohibited"() {
     assert app.isDispatchThread()
     assert !app.isWriteAccessAllowed()
 
+    assertWritingProhibited()
     SwingUtilities.invokeLater { assertWritingProhibited() }
     UIUtil.dispatchAllInvocationEvents()
   }
 
+  public void "test write action allowed inside user activity but not in modal dialog shown from non-modal invokeLater"() {
+    SwingUtilities.invokeLater {
+      guard.performUserActivity { app.runWriteAction { log << '1' } }
+
+      LaterInvocator.enterModal(new Object())
+      guard.performUserActivity {
+        assertWritingProhibited()
+        log << '2'
+      }
+      LaterInvocator.leaveAllModals()
+
+      guard.performUserActivity { app.runWriteAction { log << '3' } }
+    }
+    UIUtil.dispatchAllInvocationEvents()
+    assert log == ['1', '2', '3']
+  }
+
   private void assertWritingProhibited() {
     boolean writeActionFailed = false
     def disposable = Disposer.newDisposable('assertWritingProhibited')
@@ -193,7 +211,8 @@ class TransactionTest extends LightPlatformTestCase {
     TransactionGuard.submitTransaction testRootDisposable, {
       log << '1'
 
-      LaterInvocator.enterModal(new Object())
+      def innerModal = new Object()
+      LaterInvocator.enterModal(innerModal)
       def safeModality = ModalityState.current()
       app.executeOnPooledThread({
         app.invokeLater({
@@ -210,12 +229,27 @@ class TransactionTest extends LightPlatformTestCase {
         app.invokeLater({ app.runWriteAction { log << '5' } }, ModalityState.NON_MODAL)
       }).get()
       UIUtil.dispatchAllInvocationEvents()
+      LaterInvocator.leaveModal(innerModal)
+      UIUtil.dispatchAllInvocationEvents()
     }
     LaterInvocator.leaveAllModals()
     UIUtil.dispatchAllInvocationEvents()
     assert log == ['1', '2', '3', '4', '5']
   }
 
+  public void "test submitTransactionLater happens ASAP regardless of modality bounds"() {
+    TransactionGuard.submitTransaction testRootDisposable, {
+      log << '1'
+      guard.submitTransactionLater testRootDisposable, { log << '2' }
+      LaterInvocator.enterModal(new Object())
+      UIUtil.dispatchAllInvocationEvents()
+      LaterInvocator.leaveAllModals()
+      log << '3'
+    }
+    UIUtil.dispatchAllInvocationEvents()
+    assert log == ['1', '2', '3']
+  }
+
   public void "test don't add transaction to outdated queue"() {
     TransactionGuard.submitTransaction testRootDisposable, {
       log << '1'