diff --git a/platform/core-api/src/com/intellij/openapi/application/ThreadingSupport.kt b/platform/core-api/src/com/intellij/openapi/application/ThreadingSupport.kt index dc56b12f5b04..e0a69c564131 100644 --- a/platform/core-api/src/com/intellij/openapi/application/ThreadingSupport.kt +++ b/platform/core-api/src/com/intellij/openapi/application/ThreadingSupport.kt @@ -307,11 +307,9 @@ interface ThreadingSupport { */ @ApiStatus.Internal @Throws(LockAccessDisallowed::class) - fun prohibitTakingLocksInsideAndRun(action: Runnable) + fun prohibitTakingLocksInsideAndRun(action: Runnable, failSoftly: Boolean) - /** - * DO NOT USE - */ + /** DO NOT USE */ @ApiStatus.Internal fun isInsideUnlockedWriteIntentLock(): Boolean diff --git a/platform/core-impl/src/com/intellij/openapi/application/ex/ApplicationEx.java b/platform/core-impl/src/com/intellij/openapi/application/ex/ApplicationEx.java index 85a6662613fa..b4b735d3be47 100644 --- a/platform/core-impl/src/com/intellij/openapi/application/ex/ApplicationEx.java +++ b/platform/core-impl/src/com/intellij/openapi/application/ex/ApplicationEx.java @@ -225,7 +225,7 @@ public interface ApplicationEx extends Application { default void addSuspendingWriteActionListener(@NotNull SuspendingWriteActionListener listener, @NotNull Disposable parentDisposable) { } @ApiStatus.Internal - default void prohibitTakingLocksInsideAndRun(@NotNull Runnable runnable) { + default void prohibitTakingLocksInsideAndRun(@NotNull Runnable runnable, boolean failSoftly) { runnable.run(); } } diff --git a/platform/core-impl/src/com/intellij/openapi/application/impl/EdtCoroutineDispatcher.kt b/platform/core-impl/src/com/intellij/openapi/application/impl/EdtCoroutineDispatcher.kt index 01c2281f7524..03c0e696d20a 100644 --- a/platform/core-impl/src/com/intellij/openapi/application/impl/EdtCoroutineDispatcher.kt +++ b/platform/core-impl/src/com/intellij/openapi/application/impl/EdtCoroutineDispatcher.kt @@ -8,7 +8,9 @@ import com.intellij.openapi.application.ThreadingSupport import com.intellij.openapi.application.WriteIntentReadAction import com.intellij.openapi.application.contextModality import com.intellij.openapi.application.ex.ApplicationManagerEx +import com.intellij.openapi.application.impl.EdtDispatcherKind.LockBehavior import com.intellij.openapi.application.isCoroutineWILEnabled +import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.progress.isRunBlockingUnderReadAction import com.intellij.util.concurrency.ThreadingAssertions import com.intellij.util.ui.EDT @@ -27,7 +29,7 @@ import kotlin.coroutines.CoroutineContext * [ModalityState.any]. See IJPL-166436 */ internal sealed class EdtCoroutineDispatcher( - val type: EdtDispatcherKind, + protected val type: EdtDispatcherKind, ) : MainCoroutineDispatcher() { override val immediate: MainCoroutineDispatcher @@ -60,10 +62,10 @@ internal sealed class EdtCoroutineDispatcher( } private fun wrapWithLocking(runnable: Runnable): Runnable { - if (!type.allowReadWriteLock) { + if (!type.allowLocks()) { return Runnable { try { - ApplicationManagerEx.getApplicationEx().prohibitTakingLocksInsideAndRun(runnable) + ApplicationManagerEx.getApplicationEx().prohibitTakingLocksInsideAndRun(runnable, type.lockBehavior == LockBehavior.LOCKS_DISALLOWED_FAIL_SOFT) } catch (e: ThreadingSupport.LockAccessDisallowed) { throw IllegalStateException("You are attempting to use the RW lock inside `$this`.\n" + @@ -112,7 +114,7 @@ private class ImmediateEdtCoroutineDispatcher(type: EdtDispatcherKind) : EdtCoro if (!ModalityState.current().accepts(contextModality)) { return true } - if (type.allowReadWriteLock) { + if (type.allowLocks()) { // this dispatcher requires RW lock => if EDT does not the hold lock, then we need to reschedule to avoid blocking return !ApplicationManager.getApplication().isWriteIntentLockAcquired } @@ -124,14 +126,14 @@ private class ImmediateEdtCoroutineDispatcher(type: EdtDispatcherKind) : EdtCoro } } -enum class EdtDispatcherKind( +internal enum class EdtDispatcherKind( /** - * Historically, all runnables executed on EDT were wrapped into a Write-Intent lock. We want to move away from this contract, so here we - * operate with two versions of EDT-thread dispatcher. If [allowReadWriteLock] == `true`, then all runnables are automatically wrapped - * into write-intent lock If [allowReadWriteLock] == `false`, then runnables are executed without the lock, and requesting lock is - * forbidden for them. + * Historically, all runnables executed on EDT were wrapped into a Write-Intent lock. We want to move away from this contract, so here + * we operate with two versions of EDT-thread dispatcher. If [lockBehavior] == [LockBehavior.LOCKS_ALLOWED], then all runnables are + * automatically wrapped into write-intent lock If [lockBehavior] != [LockBehavior.LOCKS_ALLOWED], then runnables are executed without the + * lock, and requesting lock is forbidden for them. */ - val allowReadWriteLock: Boolean, + val lockBehavior: LockBehavior, /** * Sometimes coroutines get dispatched without an explicit modality in their contexts. In this case, we differentiate two cases: * 1. Platform-relevant dispatchers that may access the IJ Platform model (`Dispatchers.UI` and `Dispatchers.EDT`) These dispatchers @@ -144,11 +146,12 @@ enum class EdtDispatcherKind( val fallbackModality: DefaultModality, ) { - LEGACY_EDT(true, DefaultModality.NON_MODAL), - MODERN_UI(false, DefaultModality.NON_MODAL), + LEGACY_EDT(LockBehavior.LOCKS_ALLOWED, DefaultModality.NON_MODAL), - MAIN(false, DefaultModality.ANY); + MODERN_UI(LockBehavior.LOCKS_DISALLOWED_FAIL_HARD, DefaultModality.NON_MODAL), + + MAIN(LockBehavior.LOCKS_DISALLOWED_FAIL_SOFT, DefaultModality.ANY); enum class DefaultModality { ANY, @@ -160,9 +163,18 @@ enum class EdtDispatcherKind( } } - fun presentableName(): String = when (allowReadWriteLock) { - true -> "Dispatchers.EDT" - false -> if (fallbackModality.toModalityState() === ModalityState.nonModal()) "Dispatchers.UI" else "Dispatchers.UI.Main" + enum class LockBehavior { + LOCKS_ALLOWED, + LOCKS_DISALLOWED_FAIL_SOFT, + LOCKS_DISALLOWED_FAIL_HARD, + } + + fun allowLocks(): Boolean = lockBehavior == LockBehavior.LOCKS_ALLOWED + + fun presentableName(): String = when (this) { + LEGACY_EDT -> "Dispatchers.EDT" + MODERN_UI -> "Dispatchers.UI" + MAIN -> "Dispatchers.UI.Main" } } diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/AnyThreadWriteThreadingSupport.kt b/platform/platform-impl/src/com/intellij/openapi/application/impl/AnyThreadWriteThreadingSupport.kt index 41cba3225949..48c22866b913 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/AnyThreadWriteThreadingSupport.kt +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/AnyThreadWriteThreadingSupport.kt @@ -130,7 +130,7 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { // We approximate "on stack" permits with "thread local" permits for shared main lock private val mySecondaryPermits = ThreadLocal.withInitial { ArrayList() } private val myReadActionsInThread = ThreadLocal.withInitial { 0 } - private val myLockingProhibited = ThreadLocal.withInitial { false } + private val myLockingProhibited: ThreadLocal = ThreadLocal.withInitial { null } // todo: reimplement with listeners in IJPL-177760 private val myTopmostReadAction = ThreadLocal.withInitial { false } @@ -222,9 +222,7 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { // @Throws(E::class) override fun runWriteIntentReadAction(computation: ThrowableComputable): T { - if (myLockingProhibited.get()) { - throw ThreadingSupport.LockAccessDisallowed("Attempt to take write-intent lock was prevented") - } + handleLockAccess("write-intent lock") val listener = myWriteIntentActionListener fireBeforeWriteIntentReadActionStart(listener, computation.javaClass) @@ -396,10 +394,21 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { return permit } - private fun runReadAction(clazz: Class<*>, block: ThrowableComputable): T { - if (myLockingProhibited.get()) { - throw ThreadingSupport.LockAccessDisallowed("Attempt to take the read lock for `runReadAction` was prevented") + private fun handleLockAccess(message: String) { + val softAssertOnLockProhibition = myLockingProhibited.get() + if (softAssertOnLockProhibition != null) { + val exception = ThreadingSupport.LockAccessDisallowed("Attempt to take $message was prevented") + if (softAssertOnLockProhibition) { + logger.error(exception) + } + else { + throw exception + } } + } + + private fun runReadAction(clazz: Class<*>, block: ThrowableComputable): T { + handleLockAccess("read lock") val listener = myReadActionListener fireBeforeReadActionStart(listener, clazz) @@ -458,9 +467,7 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { } override fun tryRunReadAction(action: Runnable): Boolean { - if (myLockingProhibited.get()) { - throw ThreadingSupport.LockAccessDisallowed("Attempt to take the read lock for `tryRunReadAction` was prevented") - } + handleLockAccess("fail-fast read lock") val listener = myReadActionListener fireBeforeReadActionStart(listener, action.javaClass) @@ -635,9 +642,7 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { throwCannotWriteException() } - if (myLockingProhibited.get()) { - throw ThreadingSupport.LockAccessDisallowed("Attempt to take write lock was prevented") - } + handleLockAccess("write lock") myWriteActionPending.incrementAndGet() if (myWriteActionsStack.isEmpty()) { @@ -787,9 +792,9 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { } } - override fun prohibitTakingLocksInsideAndRun(action: Runnable) { + override fun prohibitTakingLocksInsideAndRun(action: Runnable, failSoftly: Boolean) { val currentValue = myLockingProhibited.get() - myLockingProhibited.set(true) + myLockingProhibited.set(failSoftly) try { action.run() } diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java index cbc514e1d53b..7ead30b3a471 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java @@ -1311,8 +1311,8 @@ public final class ApplicationImpl extends ClientAwareComponentManager implement } @Override - public void prohibitTakingLocksInsideAndRun(@NotNull Runnable runnable) { - getThreadingSupport().prohibitTakingLocksInsideAndRun(runnable); + public void prohibitTakingLocksInsideAndRun(@NotNull Runnable runnable, boolean failSoftly) { + getThreadingSupport().prohibitTakingLocksInsideAndRun(runnable, failSoftly); } @Override diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/EdtCoroutineDispatcherTest.kt b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/EdtCoroutineDispatcherTest.kt index ef26f0d92d30..65acf89d966b 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/EdtCoroutineDispatcherTest.kt +++ b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/EdtCoroutineDispatcherTest.kt @@ -4,9 +4,11 @@ package com.intellij.openapi.application.impl import com.intellij.openapi.application.* import com.intellij.openapi.application.ex.ApplicationManagerEx import com.intellij.openapi.progress.runBlockingMaybeCancellable +import com.intellij.openapi.util.registry.Registry import com.intellij.platform.ide.progress.ModalTaskOwner import com.intellij.platform.ide.progress.runWithModalProgressBlocking import com.intellij.testFramework.LeakHunter +import com.intellij.testFramework.assertErrorLogged import com.intellij.testFramework.common.timeoutRunBlocking import com.intellij.testFramework.junit5.TestApplication import com.intellij.util.application @@ -21,6 +23,7 @@ import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertNotEquals import org.junit.jupiter.api.Assertions.assertSame import org.junit.jupiter.api.Assertions.fail +import org.junit.jupiter.api.Assumptions import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows @@ -30,6 +33,7 @@ import org.junit.jupiter.params.provider.MethodSource import java.util.concurrent.atomic.AtomicInteger import kotlin.coroutines.ContinuationInterceptor import kotlin.coroutines.CoroutineContext +import kotlin.test.assertTrue @TestApplication class EdtCoroutineDispatcherTest { @@ -379,6 +383,33 @@ class EdtCoroutineDispatcherTest { } } + @Test + fun `main dispatcher fails softly on locking actions`(): Unit = timeoutRunBlocking(context = Dispatchers.Main) { + Assumptions.assumeTrue(Registry.`is`("ide.install.ui.dispatcher.as.main.coroutine.dispatcher")) + val counter = AtomicInteger() + assertErrorLogged { + application.runWriteAction { + counter.incrementAndGet() + } + } + assertErrorLogged { + application.runReadAction { + counter.incrementAndGet() + } + } + assertErrorLogged { + assertTrue(ApplicationManagerEx.getApplicationEx().tryRunReadAction { + counter.incrementAndGet() + }) + } + assertErrorLogged { + application.runWriteIntentReadAction { + counter.incrementAndGet() + } + } + assertThat(counter.get()).isEqualTo(4) + } + @Suppress("ForbiddenInSuspectContextMethod") @Test fun `ui dispatcher preserves modality`() = timeoutRunBlocking(context = Dispatchers.EDT) {