From 0b6ba6a94d185b1c28a713cc1bb7aa4d242b29b3 Mon Sep 17 00:00:00 2001 From: Lev Serebryakov Date: Sun, 8 Sep 2024 21:54:24 +0200 Subject: [PATCH] IJPL-149317 More explicit locks. GitOrigin-RevId: 95f64ac7da5066d2efb212d4e7cfc2cb98543ac1 --- .../editor/editorComponentInlaysUtil.kt | 43 +++++++++------- platform/core-api/api-dump-unreviewed.txt | 2 - .../util/concurrency/ThreadingAssertions.java | 40 --------------- .../completion/InlineCompletionHandler.kt | 4 +- .../src/com/intellij/ide/IdeEventQueue.kt | 9 +--- .../impl/AnyThreadWriteThreadingSupport.kt | 50 +++---------------- .../openapi/editor/impl/EditorImpl.java | 2 +- .../test/showcase/JUnit5EditorFixtureTest.kt | 13 +++-- .../InlineCompletionLifecycleTestDSL.kt | 4 +- .../coverage/view/CoverageGutterTest.kt | 5 +- .../AbstractGradleMultiFileQuickFixTest.kt | 26 +++++----- .../emptyProject/PyV3EmptyProjectSettings.kt | 5 +- 12 files changed, 72 insertions(+), 131 deletions(-) diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/editorComponentInlaysUtil.kt b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/editorComponentInlaysUtil.kt index 3ca47f58f785..3b40361f72d0 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/editorComponentInlaysUtil.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/editorComponentInlaysUtil.kt @@ -11,6 +11,7 @@ import com.intellij.collaboration.ui.util.DimensionRestrictions import com.intellij.collaboration.util.HashingUtil import com.intellij.openapi.application.ModalityState import com.intellij.openapi.application.asContextElement +import com.intellij.openapi.application.writeIntentReadAction import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.diagnostic.getOrLogException import com.intellij.openapi.editor.* @@ -85,7 +86,9 @@ private suspend fun EditorEx.doRenderInlays( vmsFlow.map { createCustomHashingStrategySet(vmHashingStrategy).apply { addAll(it) } }.collect { - positionKeeper.savePosition() + writeIntentReadAction { + positionKeeper.savePosition() + } // remove missing val iter = controllersByVmKey.iterator() @@ -105,10 +108,12 @@ private suspend fun EditorEx.doRenderInlays( } } - // immediately validate the editor to recalculate the size with inlays - editor.contentComponent.validate() - positionKeeper.restorePosition(true) - editor.contentComponent.repaint() + writeIntentReadAction { + // immediately validate the editor to recalculate the size with inlays + editor.contentComponent.validate() + positionKeeper.restorePosition(true) + editor.contentComponent.repaint() + } } awaitCancellation() } @@ -128,19 +133,21 @@ private suspend fun controlInlay(vm: VM, editor: EditorEx, r combine(lineFlow, visibleFlow, ::Pair) }.distinctUntilChanged() .collect { (line, isVisible) -> - val currentInlay = inlay - if (line != null && isVisible) { - runCatching { - val offset = editor.document.getLineEndOffset(line) - if (currentInlay == null || !currentInlay.isValid || currentInlay.offset != offset) { - currentInlay?.let(Disposer::dispose) - inlay = insertComponent(vm, rendererFactory, editor, offset) - } - }.getOrLogException(LOG) - } - else if (currentInlay != null) { - Disposer.dispose(currentInlay) - inlay = null + writeIntentReadAction { + val currentInlay = inlay + if (line != null && isVisible) { + runCatching { + val offset = editor.document.getLineEndOffset(line) + if (currentInlay == null || !currentInlay.isValid || currentInlay.offset != offset) { + currentInlay?.let(Disposer::dispose) + inlay = insertComponent(vm, rendererFactory, editor, offset) + } + }.getOrLogException(LOG) + } + else if (currentInlay != null) { + Disposer.dispose(currentInlay) + inlay = null + } } } awaitCancellation() diff --git a/platform/core-api/api-dump-unreviewed.txt b/platform/core-api/api-dump-unreviewed.txt index 3c9c72c88d37..c4c2a4afe0d6 100644 --- a/platform/core-api/api-dump-unreviewed.txt +++ b/platform/core-api/api-dump-unreviewed.txt @@ -6244,8 +6244,6 @@ f:com.intellij.util.concurrency.ThreadingAssertions - s:assertReadAccess():V - s:assertWriteAccess():V - s:assertWriteIntentReadAccess():V -- s:isImplicitLockOnEDT():Z -- s:setImplicitLockOnEDT(Z):V - s:softAssertBackgroundThread():V - s:softAssertEventDispatchThread():V - s:softAssertReadAccess():V diff --git a/platform/core-api/src/com/intellij/util/concurrency/ThreadingAssertions.java b/platform/core-api/src/com/intellij/util/concurrency/ThreadingAssertions.java index b24dcd0967e9..7ba1c6b894f9 100644 --- a/platform/core-api/src/com/intellij/util/concurrency/ThreadingAssertions.java +++ b/platform/core-api/src/com/intellij/util/concurrency/ThreadingAssertions.java @@ -32,16 +32,12 @@ public final class ThreadingAssertions { @VisibleForTesting public static final String MUST_EXECUTE_IN_READ_ACTION = "Read access is allowed from inside read-action only (see Application.runReadAction())"; - private static final String MUST_EXECUTE_IN_READ_ACTION_EXPLICIT = - "Read access is allowed from inside explicit read-action only (see Application.runReadAction()). Now implicit WriteIntentReadAction.run() is used."; @Internal @VisibleForTesting public static final String MUST_NOT_EXECUTE_IN_READ_ACTION = "Must not execute inside read action"; private static final String MUST_EXECUTE_IN_WRITE_INTENT_READ_ACTION = "Access is allowed from write thread only"; - private static final String MUST_EXECUTE_IN_WRITE_INTENT_READ_ACTION_EXPLICIT = - "Access is allowed from write thread only with explicit WriteIntentReadAction.run(). Now implicit WriteIntentReadAction.run() is used."; @Internal @VisibleForTesting public static final String MUST_EXECUTE_IN_WRITE_ACTION = @@ -57,8 +53,6 @@ public final class ThreadingAssertions { private static final String DOCUMENTATION_URL = "https://jb.gg/ij-platform-threading"; - private static boolean implicitLock = false; - /** * Asserts that the current thread is the event dispatch thread. * @@ -117,17 +111,6 @@ public final class ThreadingAssertions { if (!ApplicationManager.getApplication().isReadAccessAllowed()) { throwThreadAccessException(MUST_EXECUTE_IN_READ_ACTION); } - else if (isImplicitLockOnEDT()) { - reportImplicitRead(); - } - } - - /** - * Reports message about implicit read to logger at error level - */ - @Internal - public static void reportImplicitRead() { - getLogger().error(createThreadAccessException(MUST_EXECUTE_IN_READ_ACTION_EXPLICIT)); } /** @@ -165,17 +148,6 @@ public final class ThreadingAssertions { if (!ApplicationManager.getApplication().isWriteIntentLockAcquired()) { throwWriteIntentReadAccess(); } - else if (isImplicitLockOnEDT()) { - reportImplicitWriteIntent(); - } - } - - /** - * Reports message about implicit read to logger at error level - */ - @Internal - public static void reportImplicitWriteIntent() { - getLogger().error(createThreadAccessException(MUST_EXECUTE_IN_WRITE_INTENT_READ_ACTION_EXPLICIT)); } /** @@ -216,16 +188,4 @@ public final class ThreadingAssertions { private static @NotNull String describe(@Nullable Thread o) { return o == null ? "null" : o + " " + System.identityHashCode(o); } - - public static boolean isImplicitLockOnEDT() { - if (!EDT.isCurrentThreadEdt()) - return false; - return implicitLock; - } - - public static void setImplicitLockOnEDT(boolean implicitLock) { - if (!EDT.isCurrentThreadEdt()) - return; - ThreadingAssertions.implicitLock = implicitLock; - } } diff --git a/platform/platform-impl/src/com/intellij/codeInsight/inline/completion/InlineCompletionHandler.kt b/platform/platform-impl/src/com/intellij/codeInsight/inline/completion/InlineCompletionHandler.kt index 65eca0857e31..853bd9e40b64 100644 --- a/platform/platform-impl/src/com/intellij/codeInsight/inline/completion/InlineCompletionHandler.kt +++ b/platform/platform-impl/src/com/intellij/codeInsight/inline/completion/InlineCompletionHandler.kt @@ -526,7 +526,9 @@ class InlineCompletionHandler( @RequiresEdt private fun traceBlocking(event: InlineCompletionEventType) { ThreadingAssertions.assertEventDispatchThread() - eventListeners.getMulticaster().on(event) + WriteIntentReadAction.run { + eventListeners.getMulticaster().on(event) + } } @RequiresEdt diff --git a/platform/platform-impl/src/com/intellij/ide/IdeEventQueue.kt b/platform/platform-impl/src/com/intellij/ide/IdeEventQueue.kt index 5f1fb0654157..337e1109e89e 100644 --- a/platform/platform-impl/src/com/intellij/ide/IdeEventQueue.kt +++ b/platform/platform-impl/src/com/intellij/ide/IdeEventQueue.kt @@ -1041,14 +1041,7 @@ internal fun performActivity(e: AWTEvent, needWIL: Boolean, runnable: () -> Unit else { val runnableWithWIL = if (needWIL) { - { - ThreadingAssertions.setImplicitLockOnEDT(true) - try { - WriteIntentReadAction.run(runnable) - } finally { - ThreadingAssertions.setImplicitLockOnEDT(false) - } - } + { WriteIntentReadAction.run(runnable) } } else { runnable 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 ed5625f3fdaf..d98619badf85 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 @@ -81,13 +81,10 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { is WriteIntentPermit, is WritePermit -> release = false } - val prevImplicitLock = ThreadingAssertions.isImplicitLockOnEDT() try { - ThreadingAssertions.setImplicitLockOnEDT(false) return computation.compute() } finally { - ThreadingAssertions.setImplicitLockOnEDT(prevImplicitLock) if (release) { ts.release() } @@ -109,21 +106,10 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { override fun isWriteIntentLocked(): Boolean { val ts = myState.get() - // check for implicit - if (ts.hasWriteIntent && ThreadingAssertions.isImplicitLockOnEDT()) { - ThreadingAssertions.reportImplicitWriteIntent() - } return ts.hasWrite || ts.hasWriteIntent } - override fun isReadAccessAllowed(): Boolean { - val ts = myState.get() - // check for implicit - if (ts.hasWriteIntent && ThreadingAssertions.isImplicitLockOnEDT()) { - ThreadingAssertions.reportImplicitRead() - } - return ts.hasPermit - } + override fun isReadAccessAllowed(): Boolean = myState.get().hasPermit override fun executeOnPooledThread(action: Runnable, expired: BooleanSupplier): Future<*> { val actionDecorated = decorateRunnable(action) @@ -235,17 +221,10 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { fireBeforeReadActionStart(clazz) val ts = myState.get() if (ts.hasPermit) { - val prevImplicitLock = ThreadingAssertions.isImplicitLockOnEDT() - ThreadingAssertions.setImplicitLockOnEDT(false) - try { - fireReadActionStarted(clazz) - val rv = block.compute() - fireReadActionFinished(clazz) - return rv - } - finally { - ThreadingAssertions.setImplicitLockOnEDT(prevImplicitLock) - } + fireReadActionStarted(clazz) + val rv = block.compute() + fireReadActionFinished(clazz) + return rv } else { ts.permit = tryGetReadPermit() @@ -277,8 +256,6 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { } while (ts.permit == null) } } - val prevImplicitLock = ThreadingAssertions.isImplicitLockOnEDT() - ThreadingAssertions.setImplicitLockOnEDT(false) try { fireReadActionStarted(clazz) val rv = block.compute() @@ -286,7 +263,6 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { return rv } finally { - ThreadingAssertions.setImplicitLockOnEDT(prevImplicitLock) ts.release() fireAfterReadActionFinished(clazz) } @@ -296,16 +272,9 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { override fun tryRunReadAction(action: Runnable): Boolean { val ts = myState.get() if (ts.hasPermit) { - val prevImplicitLock = ThreadingAssertions.isImplicitLockOnEDT() - ThreadingAssertions.setImplicitLockOnEDT(false) - try { - fireReadActionStarted(action.javaClass) - action.run() - fireReadActionFinished(action.javaClass) - } - finally { - ThreadingAssertions.setImplicitLockOnEDT(prevImplicitLock) - } + fireReadActionStarted(action.javaClass) + action.run() + fireReadActionFinished(action.javaClass) return true } else { @@ -314,8 +283,6 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { if (!ts.hasPermit) { return false } - val prevImplicitLock = ThreadingAssertions.isImplicitLockOnEDT() - ThreadingAssertions.setImplicitLockOnEDT(false) try { fireReadActionStarted(action.javaClass) action.run() @@ -323,7 +290,6 @@ internal object AnyThreadWriteThreadingSupport: ThreadingSupport { return true } finally { - ThreadingAssertions.setImplicitLockOnEDT(prevImplicitLock) ts.release() fireAfterReadActionFinished(action.javaClass) } diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java index 4f48d3bdc4d6..e093d6b79efe 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java @@ -3733,7 +3733,7 @@ public final class EditorImpl extends UserDataHolderBase implements EditorEx, Hi mouseSelectionStateAlarm.cancel(); if (myMouseSelectionState != MOUSE_SELECTION_STATE_NONE) { if (mouseSelectionStateResetRunnable == null) { - mouseSelectionStateResetRunnable = () -> resetMouseSelectionState(null, null); + mouseSelectionStateResetRunnable = () -> WriteIntentReadAction.run((Runnable)() -> resetMouseSelectionState(null, null)); } mouseSelectionStateAlarm.request(Registry.intValue("editor.mouseSelectionStateResetTimeout"), ModalityState.stateForComponent(myEditorComponent), diff --git a/platform/testFramework/junit5/test/showcase/JUnit5EditorFixtureTest.kt b/platform/testFramework/junit5/test/showcase/JUnit5EditorFixtureTest.kt index 6dbd1934e5b2..5977d2033dd7 100644 --- a/platform/testFramework/junit5/test/showcase/JUnit5EditorFixtureTest.kt +++ b/platform/testFramework/junit5/test/showcase/JUnit5EditorFixtureTest.kt @@ -2,6 +2,7 @@ package com.intellij.testFramework.junit5.showcase import com.intellij.openapi.application.EDT +import com.intellij.openapi.application.writeIntentReadAction import com.intellij.testFramework.junit5.TestApplication import com.intellij.testFramework.junit5.fixture.* import kotlinx.coroutines.Dispatchers @@ -32,8 +33,10 @@ class JUnit5EditorFixtureTest { fun `caret position in editors`() { runBlocking { withContext(Dispatchers.EDT) { - localEditor.get().caretModel.moveToOffset(2) - Assertions.assertEquals(2, localEditor.get().caretModel.offset) + writeIntentReadAction { + localEditor.get().caretModel.moveToOffset(2) + Assertions.assertEquals(2, localEditor.get().caretModel.offset) + } } } } @@ -42,8 +45,10 @@ class JUnit5EditorFixtureTest { fun `selection in editors`() { runBlocking { withContext(Dispatchers.EDT) { - localEditor.get().selectionModel.setSelection(1, 3) - Assertions.assertEquals("bc", localEditor.get().selectionModel.selectedText) + writeIntentReadAction { + localEditor.get().selectionModel.setSelection(1, 3) + Assertions.assertEquals("bc", localEditor.get().selectionModel.selectedText) + } } } } diff --git a/platform/testFramework/src/com/intellij/codeInsight/inline/completion/InlineCompletionLifecycleTestDSL.kt b/platform/testFramework/src/com/intellij/codeInsight/inline/completion/InlineCompletionLifecycleTestDSL.kt index e0240deb8b0f..9fdcf26da1eb 100644 --- a/platform/testFramework/src/com/intellij/codeInsight/inline/completion/InlineCompletionLifecycleTestDSL.kt +++ b/platform/testFramework/src/com/intellij/codeInsight/inline/completion/InlineCompletionLifecycleTestDSL.kt @@ -85,7 +85,9 @@ class InlineCompletionLifecycleTestDSL(val fixture: CodeInsightTestFixture) { withContext(Dispatchers.EDT) { val lookup = fixture.lookup as? LookupImpl assertThat(lookup).isNotNull() - lookup!!.hideLookup(false) + writeIntentReadAction { + lookup!!.hideLookup(false) + } } } diff --git a/plugins/coverage/testSrc/com/intellij/coverage/view/CoverageGutterTest.kt b/plugins/coverage/testSrc/com/intellij/coverage/view/CoverageGutterTest.kt index a7dba8908ee3..30a1041332c8 100644 --- a/plugins/coverage/testSrc/com/intellij/coverage/view/CoverageGutterTest.kt +++ b/plugins/coverage/testSrc/com/intellij/coverage/view/CoverageGutterTest.kt @@ -6,6 +6,7 @@ import com.intellij.coverage.CoverageIntegrationBaseTest import com.intellij.openapi.application.EDT import com.intellij.openapi.application.readAction import com.intellij.openapi.application.writeAction +import com.intellij.openapi.application.writeIntentReadAction import com.intellij.openapi.editor.colors.CodeInsightColors import com.intellij.openapi.editor.impl.EditorImpl import com.intellij.openapi.editor.markup.FillingLineMarkerRenderer @@ -187,7 +188,9 @@ internal suspend fun findEditor(project: Project, className: String): EditorImpl internal suspend fun closeEditor(project: Project, className: String) { val psiClass = getPsiClass(project, className) withContext(Dispatchers.EDT) { - FileEditorManager.getInstance(project).closeFile(psiClass.containingFile.virtualFile) + writeIntentReadAction { + FileEditorManager.getInstance(project).closeFile(psiClass.containingFile.virtualFile) + } } } diff --git a/plugins/kotlin/gradle/gradle-java/tests.shared/test/org/jetbrains/kotlin/idea/codeInsight/gradle/AbstractGradleMultiFileQuickFixTest.kt b/plugins/kotlin/gradle/gradle-java/tests.shared/test/org/jetbrains/kotlin/idea/codeInsight/gradle/AbstractGradleMultiFileQuickFixTest.kt index 77f15bc1056c..7b79763544b0 100644 --- a/plugins/kotlin/gradle/gradle-java/tests.shared/test/org/jetbrains/kotlin/idea/codeInsight/gradle/AbstractGradleMultiFileQuickFixTest.kt +++ b/plugins/kotlin/gradle/gradle-java/tests.shared/test/org/jetbrains/kotlin/idea/codeInsight/gradle/AbstractGradleMultiFileQuickFixTest.kt @@ -90,21 +90,23 @@ abstract class AbstractGradleMultiFileQuickFixTest : MultiplePluginVersionGradle refreshRecursively(projectVFile) withContext(Dispatchers.EDT) { - PlatformTestUtil.assertDirectoriesEqual( - expected, - projectVFile, - fun(vFile: VirtualFile): Boolean { - if (vFile.parent == projectVFile) { - when (vFile.name) { - ".gradle", "gradle", "build", "gradle.properties", "gradlew", "gradlew.bat", ".kotlin" -> return false + writeIntentReadAction { + PlatformTestUtil.assertDirectoriesEqual( + expected, + projectVFile, + fun(vFile: VirtualFile): Boolean { + if (vFile.parent == projectVFile) { + when (vFile.name) { + ".gradle", "gradle", "build", "gradle.properties", "gradlew", "gradlew.bat", ".kotlin" -> return false + } } - } - if (ignoreChangesInBuildScriptFiles && ".gradle" in vFile.name) return false + if (ignoreChangesInBuildScriptFiles && ".gradle" in vFile.name) return false - return additionalResultFileFilter(vFile) - }, - ) + return additionalResultFileFilter(vFile) + }, + ) + } } } diff --git a/python/ide/impl/src/com/intellij/pycharm/community/ide/impl/newProject/impl/emptyProject/PyV3EmptyProjectSettings.kt b/python/ide/impl/src/com/intellij/pycharm/community/ide/impl/newProject/impl/emptyProject/PyV3EmptyProjectSettings.kt index 84230a2036cf..656b43ee7997 100644 --- a/python/ide/impl/src/com/intellij/pycharm/community/ide/impl/newProject/impl/emptyProject/PyV3EmptyProjectSettings.kt +++ b/python/ide/impl/src/com/intellij/pycharm/community/ide/impl/newProject/impl/emptyProject/PyV3EmptyProjectSettings.kt @@ -3,6 +3,7 @@ package com.intellij.pycharm.community.ide.impl.newProject.impl.emptyProject import com.intellij.openapi.application.EDT import com.intellij.openapi.application.writeAction +import com.intellij.openapi.application.writeIntentReadAction import com.intellij.openapi.module.Module import com.intellij.openapi.projectRoots.Sdk import com.intellij.openapi.vfs.VirtualFile @@ -19,7 +20,9 @@ class PyV3EmptyProjectSettings(var generateWelcomeScript: Boolean = false) : PyV PyWelcome.prepareFile(module.project, baseDir) } withContext(Dispatchers.EDT) { - file.navigate(true) + writeIntentReadAction { + file.navigate(true) + } } }