mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
[threading] IJPL-149317: Generate soft error on lock access in Dispatchers.Main
GitOrigin-RevId: 2655e7273eed74476c0445c1cadccea0e365bbb1
This commit is contained in:
committed by
intellij-monorepo-bot
parent
9d2825f651
commit
f04299129d
@@ -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
|
||||
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
+28
-16
@@ -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"
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+20
-15
@@ -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<Permit>() }
|
||||
private val myReadActionsInThread = ThreadLocal.withInitial { 0 }
|
||||
private val myLockingProhibited = ThreadLocal.withInitial { false }
|
||||
private val myLockingProhibited: ThreadLocal<Boolean?> = 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 <T, E : Throwable?> runWriteIntentReadAction(computation: ThrowableComputable<T, E>): 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 <T, E : Throwable?> runReadAction(clazz: Class<*>, block: ThrowableComputable<T, E>): 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 <T, E : Throwable?> runReadAction(clazz: Class<*>, block: ThrowableComputable<T, E>): 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()
|
||||
}
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
+31
@@ -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<ThreadingSupport.LockAccessDisallowed> {
|
||||
application.runWriteAction {
|
||||
counter.incrementAndGet()
|
||||
}
|
||||
}
|
||||
assertErrorLogged<ThreadingSupport.LockAccessDisallowed> {
|
||||
application.runReadAction {
|
||||
counter.incrementAndGet()
|
||||
}
|
||||
}
|
||||
assertErrorLogged<ThreadingSupport.LockAccessDisallowed> {
|
||||
assertTrue(ApplicationManagerEx.getApplicationEx().tryRunReadAction {
|
||||
counter.incrementAndGet()
|
||||
})
|
||||
}
|
||||
assertErrorLogged<ThreadingSupport.LockAccessDisallowed> {
|
||||
application.runWriteIntentReadAction<Unit, Exception> {
|
||||
counter.incrementAndGet()
|
||||
}
|
||||
}
|
||||
assertThat(counter.get()).isEqualTo(4)
|
||||
}
|
||||
|
||||
@Suppress("ForbiddenInSuspectContextMethod")
|
||||
@Test
|
||||
fun `ui dispatcher preserves modality`() = timeoutRunBlocking(context = Dispatchers.EDT) {
|
||||
|
||||
Reference in New Issue
Block a user