diff --git a/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java b/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java index 800add4a9ac1..1122bcfe9bbb 100644 --- a/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java +++ b/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 myQueue = new LinkedBlockingQueue(); private final Map myModality2Transaction = ContainerUtil.createConcurrentWeakMap(); - private final Set myWriteSafeModalities = Collections.newSetFromMap(ContainerUtil.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 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() { diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/LaterInvocator.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/LaterInvocator.java index 1fcb451d0a6d..c2630ada42d3 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/LaterInvocator.java +++ b/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(); } diff --git a/platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy b/platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy index c87b7d79f7c0..747bb66e3e61 100644 --- a/platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy +++ b/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'] + } + }