diff --git a/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorImpl.kt b/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorImpl.kt index 0a70c6bfc008..ef5aa22bd65f 100644 --- a/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorImpl.kt +++ b/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorImpl.kt @@ -18,6 +18,7 @@ import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.project.Project import com.intellij.openapi.vfs.VirtualFile +import kotlinx.coroutines.Deferred import java.awt.event.ComponentAdapter import java.awt.event.ComponentEvent import java.util.concurrent.CancellationException @@ -39,8 +40,9 @@ open class PsiAwareTextEditorImpl : TextEditorImpl { internal constructor(project: Project, file: VirtualFile, - asyncLoader: AsyncEditorLoader, - editor: EditorImpl) : super(project = project, file = file, editor = editor, asyncLoader = asyncLoader) + provider: TextEditorProvider, + editor: EditorImpl, + task: Deferred) : super(project = project, file = file, editor = editor, provider = provider, asyncLoader = createAsyncEditorLoader(provider, project, editor, file, task)) override fun createEditorComponent(project: Project, file: VirtualFile, editor: EditorImpl): TextEditorComponent { val component = PsiAwareTextEditorComponent(project = project, file = file, textEditor = this, editor = editor) diff --git a/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorProvider.kt b/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorProvider.kt index 5ca01bb37d14..6a9a43631cd7 100644 --- a/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorProvider.kt +++ b/platform/lang-impl/src/com/intellij/openapi/fileEditor/impl/text/PsiAwareTextEditorProvider.kt @@ -23,7 +23,6 @@ import com.intellij.openapi.editor.impl.EditorGutterLayout import com.intellij.openapi.extensions.ExtensionPointName import com.intellij.openapi.fileEditor.* import com.intellij.openapi.fileEditor.impl.text.AsyncEditorLoader.Companion.isEditorLoaded -import com.intellij.openapi.fileEditor.impl.text.TextEditorImpl.Companion.createAsyncEditorLoader import com.intellij.openapi.project.Project import com.intellij.openapi.util.WriteExternalException import com.intellij.openapi.vfs.VirtualFile @@ -45,8 +44,6 @@ open class PsiAwareTextEditorProvider : TextEditorProvider(), AsyncFileEditorPro } override suspend fun createEditorBuilder(project: Project, file: VirtualFile, document: Document?): AsyncFileEditorProvider.Builder { - val asyncLoader = createAsyncEditorLoader(provider = this, project = project) - val effectiveDocument = if (document == null) { val fileDocumentManager = serviceAsync() fileDocumentManager.getCachedDocument(file) ?: readActionBlocking { @@ -76,7 +73,7 @@ open class PsiAwareTextEditorProvider : TextEditorProvider(), AsyncFileEditorPro val editorDeferred = CompletableDeferred() - val task = asyncLoader.coroutineScope.async(CoroutineName("TextEditorInitializer")) { + val task = TextEditorImpl.editorLoaderScope(project).async(CoroutineName("call TextEditorInitializers")) { val editorSupplier = suspend { editorDeferred.await() } val highlighterReady = suspend { highlighterDeferred.join() } @@ -122,9 +119,8 @@ open class PsiAwareTextEditorProvider : TextEditorProvider(), AsyncFileEditorPro override fun build(): FileEditor { val editor = factory.createMainEditor(effectiveDocument, project, file, highlighter) editor.gutterComponentEx.setInitialIconAreaWidth(EditorGutterLayout.getInitialGutterWidth()) - val textEditor = PsiAwareTextEditorImpl(project = project, file = file, editor = editor, asyncLoader = asyncLoader) - editorDeferred.complete(textEditor.editor) - asyncLoader.start(textEditor = textEditor, tasks = listOf(task)) + editorDeferred.complete(editor) + val textEditor = PsiAwareTextEditorImpl(project = project, file = file, provider = this@PsiAwareTextEditorProvider, editor = editor, task = task) return textEditor } } diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/AsyncEditorLoader.kt b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/AsyncEditorLoader.kt index a4f69a8f39b2..c5aea0fddc3d 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/AsyncEditorLoader.kt +++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/AsyncEditorLoader.kt @@ -8,7 +8,10 @@ import com.intellij.openapi.application.ModalityState import com.intellij.openapi.application.asContextElement import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.ex.EditorEx +import com.intellij.openapi.editor.impl.EditorImpl +import com.intellij.openapi.fileEditor.FileEditorManager import com.intellij.openapi.fileEditor.FileEditorStateLevel +import com.intellij.openapi.fileEditor.TextEditor import com.intellij.openapi.project.Project import com.intellij.openapi.util.Key import com.intellij.openapi.vfs.VirtualFile @@ -20,7 +23,6 @@ import com.intellij.util.concurrency.annotations.RequiresReadLock import com.intellij.util.ui.AsyncProcessIcon import kotlinx.coroutines.* import org.jetbrains.annotations.ApiStatus.Internal -import java.util.* import java.util.concurrent.atomic.AtomicReference import javax.swing.JComponent import javax.swing.event.ChangeEvent @@ -28,29 +30,56 @@ import kotlin.coroutines.resume import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds -private val ASYNC_LOADER = Key.create("ASYNC_LOADER") - class AsyncEditorLoader internal constructor(private val project: Project, private val provider: TextEditorProvider, - @JvmField val coroutineScope: CoroutineScope) { - private val delayedActions = ArrayDeque() + @JvmField val coroutineScope: CoroutineScope, + editor: EditorImpl, + virtualFile: VirtualFile, task: Deferred?) { + private val tasks: List> + init { + val textEditorInit = coroutineScope.async(CoroutineName("HighlighterTextEditorInitializer")) { + TextEditorImpl.setHighlighterToEditor(project, virtualFile, editor.document, editor) + } + tasks = if (task == null) listOf(textEditorInit) else listOf(textEditorInit, task) + } + /** + * [delayedActions] contains either: + * - empty list: the editor was not loaded + * - list of runnables: the editor was not loaded and these runnables need to be run on load + * - [LOADED]: the editor is loaded + * empty list was chosen to mark editor "not loaded" as early as possible, to avoid a narrow data race between TextEditorImpl instantiation and [AsyncEditorLoader.start] call + */ + private val LOADED: List = listOf(Runnable {}) + private val delayedActions: AtomicReference> = AtomicReference(listOf()) private val delayedScrollState = AtomicReference() companion object { - internal val OPENED_IN_BULK = Key.create("EditorSplitters.opened.in.bulk") + internal val OPENED_IN_BULK: Key = Key.create("EditorSplitters.opened.in.bulk") @Internal fun isOpenedInBulk(file: VirtualFile): Boolean = file.getUserData(OPENED_IN_BULK) != null + private fun findTextEditor(editor: Editor): TextEditor? { + val project = editor.project + val virtualFile = editor.virtualFile + if (project == null || virtualFile == null) return null + return FileEditorManager.getInstance(project).getAllEditors(virtualFile).find { f -> f is TextEditor } as TextEditor? + } + @JvmStatic @RequiresEdt fun performWhenLoaded(editor: Editor, runnable: Runnable) { - val loader = editor.getUserData(ASYNC_LOADER) - loader?.delayedActions?.add(captureThreadContext(runnable)) ?: runnable.run() + val asyncLoader = (findTextEditor(editor) as? TextEditorImpl)?.asyncLoader + if (asyncLoader == null) { + runnable.run() + } + else { + asyncLoader.performWhenLoaded(runnable) + } } internal suspend fun waitForLoaded(editor: Editor) { - if (editor.getUserData(ASYNC_LOADER) != null) { + if (!isEditorLoaded(editor)) { withContext(Dispatchers.EDT + ModalityState.any().asContextElement()) { suspendCancellableCoroutine { performWhenLoaded(editor) { it.resume(Unit) } @@ -61,19 +90,32 @@ class AsyncEditorLoader internal constructor(private val project: Project, @JvmStatic fun isEditorLoaded(editor: Editor): Boolean { - return editor.getUserData(ASYNC_LOADER) == null + val textEditor = findTextEditor(editor) + if (textEditor !is TextEditorImpl) return true + return textEditor.isLoaded() + } + } + + @RequiresEdt + private fun performWhenLoaded(runnable: Runnable) { + val toRunLater = captureThreadContext(runnable) + val newActions = delayedActions.updateAndGet { oldActions: List -> + if (oldActions == LOADED || oldActions.contains(toRunLater)) oldActions + else oldActions + toRunLater + } + if (!newActions.contains(toRunLater)) { + runnable.run() } } // executed in the same EDT task where TextEditorImpl is created @Internal @RequiresEdt - fun start(textEditor: TextEditorImpl, tasks: List>) { + fun start(textEditor: TextEditorImpl) { val editor = textEditor.editor - editor.putUserData(ASYNC_LOADER, this) if (ApplicationManager.getApplication().isUnitTestMode) { - startInTests(tasks = tasks, editor = editor) + startInTests(tasks = tasks) return } @@ -89,12 +131,9 @@ class AsyncEditorLoader internal constructor(private val project: Project, indicatorJob.cancel() withContext(Dispatchers.EDT + CoroutineName("execute delayed actions")) { - editor.putUserData(ASYNC_LOADER, null) editor.scrollingModel.disableAnimation() try { - while (true) { - (delayedActions.pollFirst() ?: break).run() - } + markLoadedAndExecuteDelayedActions() } finally { editor.scrollingModel.enableAnimation() @@ -104,15 +143,18 @@ class AsyncEditorLoader internal constructor(private val project: Project, } } - private fun startInTests(tasks: List>, editor: EditorEx) { + private fun markLoadedAndExecuteDelayedActions() { + val delayedActions = delayedActions.getAndSet(LOADED) + for (action in delayedActions) { + action.run() + } + } + + private fun startInTests(tasks: List>) { runWithModalProgressBlocking(project, "") { tasks.awaitAll() } - editor.putUserData(ASYNC_LOADER, null) - - while (true) { - (delayedActions.pollFirst() ?: break).run() - } + markLoadedAndExecuteDelayedActions() } @RequiresReadLock @@ -136,6 +178,10 @@ class AsyncEditorLoader internal constructor(private val project: Project, internal fun dispose() { coroutineScope.cancel() } + + internal fun isLoaded(): Boolean { + return delayedActions.get() == LOADED + } } private class DelayedScrollState(@JvmField val relativeCaretPosition: Int, @JvmField val exactState: Boolean) diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/TextEditorImpl.kt b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/TextEditorImpl.kt index 3b113f410044..87032896acfe 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/TextEditorImpl.kt +++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/text/TextEditorImpl.kt @@ -33,10 +33,9 @@ import com.intellij.platform.diagnostic.telemetry.impl.span import com.intellij.platform.util.coroutines.childScope import com.intellij.pom.Navigatable import com.intellij.psi.PsiDocumentManager -import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.async import org.jetbrains.annotations.ApiStatus.Internal import org.jetbrains.annotations.NonNls import java.beans.PropertyChangeListener @@ -46,33 +45,20 @@ import kotlin.coroutines.EmptyCoroutineContext private val TRANSIENT_EDITOR_STATE_KEY = Key.create("transientState") -open class TextEditorImpl @Internal constructor(@JvmField protected val project: Project, - @JvmField protected val file: VirtualFile, - editor: EditorImpl, - private val asyncLoader: AsyncEditorLoader) : UserDataHolderBase(), TextEditor { +open class TextEditorImpl +@Internal @JvmOverloads constructor(@JvmField protected val project: Project, + @JvmField protected val file: VirtualFile, + provider: TextEditorProvider, + editor: EditorImpl, + internal val asyncLoader: AsyncEditorLoader = createAsyncEditorLoader(provider, project, editor, file)) : UserDataHolderBase(), TextEditor { @Suppress("LeakingThis") private val changeSupport = PropertyChangeSupport(this) private val component: TextEditorComponent - constructor(project: Project, - file: VirtualFile, - provider: TextEditorProvider, - editor: EditorImpl) : this(project = project, - file = file, - editor = editor, - asyncLoader = createAsyncEditorLoader(provider, project)) { - val editorSupplier = suspend { editor } - @Suppress("LeakingThis") - asyncLoader.start(textEditor = this, tasks = listOf( - asyncLoader.coroutineScope.async(CoroutineName("HighlighterTextEditorInitializer")) { - setHighlighterToEditor(project, file, editor.document, editorSupplier) - }, - )) - } - init { @Suppress("LeakingThis") component = createEditorComponent(project = project, file = file, editor = editor) + asyncLoader.start(this) for (customizer in TextEditorCustomizer.EP.extensionList) { @Suppress("LeakingThis") customizer.customize(this) @@ -89,18 +75,25 @@ open class TextEditorImpl @Internal constructor(@JvmField protected val project: // don't pollute global scope companion object { - fun createAsyncEditorLoader(provider: TextEditorProvider, project: Project): AsyncEditorLoader { + fun createAsyncEditorLoader(provider: TextEditorProvider, project: Project, editor: EditorImpl, virtualFile: VirtualFile, task: Deferred? = null): AsyncEditorLoader { + return AsyncEditorLoader( + project = project, + provider = provider, + coroutineScope = editorLoaderScope(project), + editor, virtualFile, task + ) + } + + @Internal + fun editorLoaderScope(project: Project): CoroutineScope { // `openEditorImpl` uses runWithModalProgressBlocking, // but an async editor load is performed in the background, out of the `openEditorImpl` call val modality = ModalityState.any().asContextElement() val context = if (StartUpMeasurer.isEnabled()) rootTask() else EmptyCoroutineContext - return AsyncEditorLoader( - project = project, - provider = provider, - coroutineScope = project.service().coroutineScope.childScope(supervisor = false, - context = context + modality), - ) + val coroutineScope = project.service().coroutineScope.childScope(supervisor = false, + context = context + modality) + return coroutineScope } @Internal @@ -122,10 +115,10 @@ open class TextEditorImpl @Internal constructor(@JvmField protected val project: return factory.createMainEditor(document!!, project, file, null) } - private suspend fun setHighlighterToEditor(project: Project, - file: VirtualFile, - document: Document, - editorSupplier: suspend () -> EditorEx) { + internal suspend fun setHighlighterToEditor(project: Project, + file: VirtualFile, + document: Document, + editor: EditorImpl) { val scheme = serviceAsync().globalScheme val editorHighlighterFactory = serviceAsync() val highlighter = readAction { @@ -136,7 +129,6 @@ open class TextEditorImpl @Internal constructor(@JvmField protected val project: highlighter } - val editor = editorSupplier() span("editor highlighter set", Dispatchers.EDT) { editor.settings.setLanguageSupplier { getDocumentLanguage(editor) } editor.highlighter = highlighter @@ -215,6 +207,10 @@ open class TextEditorImpl @Internal constructor(@JvmField protected val project: } override fun toString(): @NonNls String = "Editor: ${component.file}" + + internal fun isLoaded(): Boolean { + return asyncLoader.isLoaded() + } } private class TransientEditorState { diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorImplTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorImplTest.java index c26113c891ca..9016ccdac1bf 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorImplTest.java @@ -17,6 +17,7 @@ import com.intellij.openapi.editor.impl.view.FontLayoutService; import com.intellij.openapi.editor.markup.HighlighterTargetArea; import com.intellij.openapi.editor.markup.RangeHighlighter; import com.intellij.openapi.editor.markup.TextAttributes; +import com.intellij.openapi.fileEditor.impl.text.AsyncEditorLoader; import com.intellij.openapi.ide.CopyPasteManager; import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.SystemInfo; @@ -431,6 +432,7 @@ public class EditorImplTest extends AbstractEditorTest { public void testDefaultHorizontalScrolling() { initText("" + StringUtil.repeat("abc", 100)); + assertTrue(AsyncEditorLoader.Companion.isEditorLoaded(getEditor())); int spaceWidth = EditorUtil.getSpaceWidth(Font.PLAIN, getEditor()); EditorTestUtil.setEditorVisibleSize(getEditor(), 15, 2); diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLoadTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLoadTest.java new file mode 100644 index 000000000000..f5a70e4d0d48 --- /dev/null +++ b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/EditorLoadTest.java @@ -0,0 +1,61 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.openapi.editor.impl; + +import com.intellij.diagnostic.ThreadDumper; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.editor.EditorFactory; +import com.intellij.openapi.editor.event.EditorFactoryEvent; +import com.intellij.openapi.editor.event.EditorFactoryListener; +import com.intellij.openapi.fileEditor.FileEditor; +import com.intellij.openapi.fileEditor.OpenFileDescriptor; +import com.intellij.openapi.fileEditor.impl.text.AsyncEditorLoader; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.testFramework.FileEditorManagerTestCase; +import org.jetbrains.annotations.NotNull; + +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +public class EditorLoadTest extends FileEditorManagerTestCase { + public void testEditorLoadedStateMustTransitionInOneDirection_Stress() throws Exception { + VirtualFile virtualFile = createTempFile(getTestName(false) + ".txt", "text".getBytes(StandardCharsets.UTF_8)); + AtomicReference> testStateInvariant = new AtomicReference<>(); + AtomicBoolean run = new AtomicBoolean(); + AtomicBoolean loaded = new AtomicBoolean(); + EditorFactory.getInstance().addEditorFactoryListener(new EditorFactoryListener() { + @Override + public void editorCreated(@NotNull EditorFactoryEvent event) { + Editor editor = event.getEditor(); + testStateInvariant.set(ApplicationManager.getApplication().executeOnPooledThread(() -> { + while (run.get()) { + boolean markedLoaded = AsyncEditorLoader.Companion.isEditorLoaded(editor); + boolean oldLoaded = loaded.getAndSet(markedLoaded); + if (oldLoaded && !markedLoaded) { + throw new AssertionError("incorrect state transition: loaded->!loaded\nthreaddump:\n" + ThreadDumper.dumpThreadsToString()); + } + } + })); + } + }, getTestRootDisposable()); + for (int i=0; i<100; i++) { + testStateInvariant.set(null); + run.set(true); + loaded.set(false); + List editors = manager.openEditor(new OpenFileDescriptor(getProject(), virtualFile, 0), false); + LOG.debug(i+": "+editors); + System.out.println(i+": "+editors); + try { + assertTrue(loaded.get()); + run.set(false); + testStateInvariant.get().get(); + } + finally { + manager.closeFile(virtualFile); + } + } + } +}