honor the fact that write-unsafe modality nesting doesn't necessarily correspond to transaction nesting

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

@@ -27,10 +27,8 @@ 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;
 
@@ -41,27 +39,25 @@ 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());
+
+  /**
+   * Remembers the value of {@link #myWritingAllowed} at the start of each modality. If writing wasn't allowed at that moment
+   * (e.g. inside SwingUtilities.invokeLater), it won't be allowed for all dialogs inside such modality, even from user activity.
+   */
+  private final Map<ModalityState, Boolean> myWriteSafeModalities = ContainerUtil.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);
+    myWriteSafeModalities.put(ModalityState.NON_MODAL, true);
   }
 
   @NotNull
   private AccessToken startTransactionUnchecked() {
-    final Object prevUnsafeModality = myUnsafeModality;
     final boolean wasWritingAllowed = myWritingAllowed;
 
     myWritingAllowed = true;
     myCurrentTransaction = new TransactionIdImpl(myCurrentTransaction);
-    myUnsafeModality = null;
 
     return new AccessToken() {
       @Override
@@ -75,7 +71,6 @@ public class TransactionGuardImpl extends TransactionGuard {
         myWritingAllowed = wasWritingAllowed;
         myCurrentTransaction.myFinished = true;
         myCurrentTransaction = myCurrentTransaction.myParent;
-        myUnsafeModality = prevUnsafeModality;
       }
     };
   }
@@ -218,7 +213,7 @@ public class TransactionGuardImpl extends TransactionGuard {
    */
   @NotNull
   public AccessToken startActivity(boolean userActivity) {
-    boolean allowWriting = userActivity && myUnsafeModality == null;
+    boolean allowWriting = userActivity && isWriteSafeModality(ModalityState.current());
     if (myWritingAllowed == allowWriting) {
       return AccessToken.EMPTY_ACCESS_TOKEN;
     }
@@ -233,11 +228,15 @@ public class TransactionGuardImpl extends TransactionGuard {
     };
   }
 
+  private boolean isWriteSafeModality(ModalityState state) {
+    return Boolean.TRUE.equals(myWriteSafeModalities.get(state));
+  }
+
   public void assertWriteActionAllowed() {
     if (Registry.is("ide.require.transaction.for.model.changes", false) && !myWritingAllowed) {
       String message = "Write access is allowed from model transactions only, see TransactionGuard documentation for details";
       if (ApplicationManager.getApplication().isUnitTestMode()) {
-        message += "; current modality=" + ModalityState.current() + "; unsafe modality=" + myUnsafeModality;
+        message += "; current modality=" + ModalityState.current() + "; known modalities=" + myWriteSafeModalities;
       }
       // please assign exceptions here to Peter
       LOG.error(message);
@@ -266,23 +265,12 @@ public class TransactionGuardImpl extends TransactionGuard {
     return myWritingAllowed ? myCurrentTransaction : null;
   }
 
-  public void enteredModality(@NotNull ModalityState modality, @NotNull Object modalityObject) {
+  public void enteredModality(@NotNull ModalityState modality) {
     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);
-    }
+    myWriteSafeModalities.put(modality, myWritingAllowed);
   }
 
   @Nullable
@@ -292,7 +280,7 @@ public class TransactionGuardImpl extends TransactionGuard {
 
   @NotNull
   public Runnable wrapLaterInvocation(@NotNull final Runnable runnable, @NotNull ModalityState modalityState) {
-    if (myWriteSafeModalities.contains(modalityState)) {
+    if (isWriteSafeModality(modalityState)) {
       return new Runnable() {
         @Override
         public void run() {

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

@@ -190,7 +190,7 @@ public class LaterInvocator {
 
     TransactionGuardImpl guard = IdeaApplication.isLoaded() ? (TransactionGuardImpl)TransactionGuard.getInstance() : null;
     if (guard != null) {
-      guard.enteredModality(ourModalityStack.peek(), modalEntity);
+      guard.enteredModality(ourModalityStack.peek());
     }
   }
 
@@ -211,11 +211,6 @@ public class LaterInvocator {
       ((ModalityStateEx)ourModalityStack.get(i)).removeModality(modalEntity);
     }
 
-    TransactionGuardImpl guard = IdeaApplication.isLoaded() ? (TransactionGuardImpl)TransactionGuard.getInstance() : null;
-    if (guard != null) {
-      guard.leftModality(modalEntity);
-    }
-
     ourQueueSkipCount = 0;
     requestFlush();
   }

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

@@ -273,6 +273,7 @@ class TransactionTest extends LightPlatformTestCase {
   }
 
   public void "test no synchronous transactions inside invokeLater"() {
+    LoggedErrorProcessor.instance.disableStderrDumping(testRootDisposable)
     SwingUtilities.invokeLater {
       log << '1'
       try {
@@ -286,4 +287,18 @@ class TransactionTest extends LightPlatformTestCase {
     assert log == ['1', 'assert']
   }
 
+  public void "test write-unsafe modality ends inside a transaction"() {
+    LaterInvocator.enterModal(new Object())
+    guard.performUserActivity { assertWritingProhibited() }
+    TransactionGuard.submitTransaction testRootDisposable, {
+      LaterInvocator.leaveAllModals()
+      log << '1'
+    }
+    UIUtil.dispatchAllInvocationEvents()
+    assert log == ['1']
+    assert ModalityState.current() == ModalityState.NON_MODAL
+    guard.performUserActivity { app.runWriteAction { log << '2' } }
+    assert log == ['1', '2']
+  }
+
 }