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 237df5160ca3..17c520c6e906 100644 --- a/platform/core-impl/src/com/intellij/openapi/application/TransactionGuardImpl.java +++ b/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 myQueue = new LinkedBlockingQueue(); private final Map myModality2Transaction = ContainerUtil.createConcurrentWeakMap(); private final Set myWriteSafeModalities = Collections.newSetFromMap(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); @@ -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); } } 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 981b9f101712..79327119cb9a 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 @@ -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; diff --git a/platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy b/platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy index 1b68f58338bd..aa702edf29a6 100644 --- a/platform/platform-tests/testSrc/com/intellij/application/TransactionTest.groovy +++ b/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'