[multiverse] IJPL-217347 restart async commit if another FileViewProvider is created during commit run

Imagine the following situation: There is a file with 2 code insight contexts. And FileManager has a PsiFile with context A for the file. Now, the file text gets updated and is being asynchronously committed. At the same time, another activity spawns a read-action that requests a PsiFile of this file with context B. The requested PsiFile must stay in sync with the document. GitOrigin-RevId: 080dcd87964a5e3d4e3a5abdaf078fe360e96a12
2026-09-27 10:03:11 +07:00 · 2025-11-09 00:49:10 +00:00
parent 845bc9416e
commit 740edb5c7b
3 changed files with 178 additions and 32 deletions
@@ -1186,11 +1186,19 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen
    VirtualFile virtualFile = FileDocumentManager.getInstance().getFile(document);
    if (virtualFile != null) {
      FileManager fileManager = getFileManager();
-      FileViewProvider viewProvider = fileManager.findCachedViewProvider(virtualFile);
-      if (viewProvider != null) {
+      List<FileViewProvider> viewProviders = fileManager.findCachedViewProviders(virtualFile);
+      boolean isWriteAccess = ApplicationManager.getApplication().isWriteAccessAllowed();
+      if (!viewProviders.isEmpty()) {
        // we can end up outside write action here if the document has forUseInNonAWTThread=true
-        ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction(() ->
-                                                                                                           ((AbstractFileViewProvider)viewProvider).onContentReload()));
+        ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction(() -> {
+          List<FileViewProvider> effectiveViewProviders =
+            isWriteAccess ? viewProviders
+                          : fileManager.findCachedViewProviders(virtualFile); // new view providers could appear concurrently
+
+          for (FileViewProvider viewProvider : effectiveViewProviders) {
+            ((AbstractFileViewProvider)viewProvider).onContentReload();
+          }
+        }));
      }
      else if (FileIndexFacade.getInstance(myProject).isInContent(virtualFile)) {
        ApplicationManager.getApplication().runWriteAction(ExternalChangeActionUtil.externalChangeAction(() ->
@@ -10,6 +10,7 @@ import com.intellij.openapi.application.*
 import com.intellij.openapi.components.Service
 import com.intellij.openapi.components.service
 import com.intellij.openapi.diagnostic.logger
+import com.intellij.openapi.diagnostic.trace
 import com.intellij.openapi.editor.Document
 import com.intellij.openapi.editor.ex.DocumentEx
 import com.intellij.openapi.fileEditor.FileDocumentManager
@@ -22,7 +23,6 @@ import com.intellij.openapi.util.ProperTextRange
 import com.intellij.openapi.util.TextRange
 import com.intellij.openapi.util.registry.Registry
 import com.intellij.openapi.util.text.StringUtil
-import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.psi.*
 import com.intellij.psi.text.BlockSupport
 import com.intellij.util.SmartList
@@ -199,12 +199,12 @@ class DocumentCommitThread : DocumentCommitProcessor, Disposable {
    val finishProcessors = SmartList<BooleanRunnable>()
    val reparseInjectedProcessors = SmartList<BooleanRunnable>()

+    LOG.trace { "commitUnderProgress: ${task.myReason}, $document, synchronously: $synchronously " }

-    val psiManager = PsiManagerEx.getInstanceEx(project)
-    val virtualFile = FileDocumentManager.getInstance().getFile(document)
-    val viewProviders = findViewProvidersForCommit(psiManager, virtualFile)
+    val viewProviders = findViewProvidersForCommit(document, project)
    if (viewProviders.isEmpty()) {
      finishProcessors.add(handleCommitWithoutPsi(task, documentManager))
+      task.cachedViewProviders = emptyList()
    }
    else {
      // While we were messing around transferring things to background thread, the ViewProviders can become obsolete
@@ -239,6 +239,13 @@ class DocumentCommitThread : DocumentCommitProcessor, Disposable {
      // this document was not referenced by anyone, hence we don't need to perform a write action
      val document = task.myDocumentRef.get() ?: return@task

+
+      if (!synchronously && newViewProvidersWereConcurrentlyAdded(document, task.cachedViewProviders, project)) {
+        // add a document back to the queue
+        commitAsynchronously(project, documentManager, document, "Re-added back because of new view providers", task.myCreationModality)
+        return@task
+      }
+
      val success = documentManager.finishCommit(document, finishProcessors, reparseInjectedProcessors, synchronously, task.myReason)
      if (synchronously) {
        assert(success)
@@ -246,31 +253,55 @@ class DocumentCommitThread : DocumentCommitProcessor, Disposable {
      if (synchronously || success) {
        assert(!documentManager.isInUncommittedSet(document))
      }
-      if (!success && viewProviders.isEventSystemEnabled()) {
+      if (!success && task.cachedViewProviders.isEventSystemEnabled()) {
        // add a document back to the queue
        commitAsynchronously(project, documentManager, document, "Re-added back", task.myCreationModality)
      }
    }
  }

-  private fun findViewProvidersForCommit(
-    psiManager: PsiManagerEx,
-    virtualFile: VirtualFile?,
-  ): List<FileViewProvider> {
-    if (virtualFile == null) {
-      return emptyList()
-    }
+  private fun findViewProvidersForCommit(document: Document, project: Project): List<FileViewProvider> {
+    val psiManager = PsiManagerEx.getInstanceEx(project)
+    val virtualFile = FileDocumentManager.getInstance().getFile(document) ?: return emptyList()

    if (isSharedSourceSupportEnabled(psiManager.project)) {
-      val providers = psiManager.fileManagerEx.findCachedViewProviders(virtualFile)
-      if (providers.isNotEmpty()) {
-        return providers
+      val cached = psiManager.fileManagerEx.findCachedViewProviders(virtualFile)
+      if (cached.isNotEmpty()) {
+        return cached
      }
-      // no providers mean that they might be collected.
-      // so let's try and find at least one with the help of the following line.
+    }
+    return listOfNotNull(psiManager.findViewProvider(virtualFile))
+  }
+
+  private fun newViewProvidersWereConcurrentlyAdded(
+    document: Document,
+    committedViewProviders: List<FileViewProvider>,
+    project: Project,
+  ): Boolean {
+    val currentProviders = findViewProvidersForCommit(document, project)
+
+    if (committedViewProviders.size != currentProviders.size) {
+      LOG.trace { "Concurrent view provider modification detected. Was: ${committedViewProviders.size}, Now: ${currentProviders.size}. Adding document back to the queue. $document" }
+      return true
    }

-    return listOfNotNull(psiManager.findViewProvider(virtualFile))
+    if (committedViewProviders.size == 1) {
+      if (committedViewProviders.first() == currentProviders.first()) {
+        return false
+      }
+      else {
+        LOG.trace { "Concurrent view provider modification detected: view provider was changed to another one. Adding document back to the queue. $document" }
+        return true
+      }
+    }
+
+    if (committedViewProviders.toSet().containsAll(currentProviders)) {
+      return false
+    }
+    else {
+      LOG.trace { "Concurrent view provider modification detected. Adding document back to the queue. $document" }
+      return true
+    }
  }

  override fun toString(): String = "Document commit thread; application: ${ApplicationManager.getApplication()}; isDisposed: $isDisposed"
@@ -312,7 +343,9 @@ class DocumentCommitThread : DocumentCommitProcessor, Disposable {
    val myLastCommittedText: CharSequence
    // store initial document modification sequence here to check if it changed later before commit in EDT
    private val myModificationSequence: Int
-    @Volatile var cachedViewProviders: List<FileViewProvider>? = null
+
+    /** initialized under read-action in commitUnderProgress */
+    @Volatile lateinit var cachedViewProviders: List<FileViewProvider>

    constructor(
      project: Project,
@@ -4,21 +4,24 @@ package com.intellij.psi.impl.file.impl
 import com.intellij.codeInsight.multiverse.ProjectModelContextBridge
 import com.intellij.openapi.application.readAction
 import com.intellij.openapi.application.writeAction
+import com.intellij.openapi.command.CommandProcessor
 import com.intellij.openapi.fileEditor.FileDocumentManager
 import com.intellij.platform.testFramework.junit5.projectStructure.fixture.withSharedSourceEnabled
-import com.intellij.psi.PsiDocumentManager
-import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiElementVisitor
-import com.intellij.psi.PsiFile
+import com.intellij.psi.*
 import com.intellij.psi.impl.PsiManagerEx
 import com.intellij.psi.util.PsiUtilCore
 import com.intellij.testFramework.IndexingTestUtil
 import com.intellij.testFramework.common.timeoutRunBlocking
 import com.intellij.testFramework.junit5.TestApplication
-import com.intellij.testFramework.junit5.fixture.moduleFixture
-import com.intellij.testFramework.junit5.fixture.projectFixture
-import com.intellij.testFramework.junit5.fixture.virtualFileFixture
+import com.intellij.testFramework.junit5.fixture.*
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.delay
+import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.RepeatedTest
 import org.junit.jupiter.api.Test
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.Duration.Companion.seconds

@TestApplication
 internal class CommitMultiverseFileTest {
@@ -29,14 +32,21 @@ internal class CommitMultiverseFileTest {
    private val module2 = projectFixture.moduleFixture("CommitMultiverseFileTest_src2")

    private val sourceRoot = sharedSourceRootFixture(module1, module2)
+
+    private val project by projectFixture
+
+    private val psiManager by lazy { PsiManagerEx.getInstanceEx(project) }
+
+    private val contextBridge by lazy { ProjectModelContextBridge.getInstance(project) }
+    private val context1 by lazy { contextBridge.getContext(module1.get())!! }
+    private val context2 by lazy { contextBridge.getContext(module2.get())!! }
  }

  private val virtualFile by sourceRoot.virtualFileFixture("TestCommon.java", "class A {}")
+  private val files by sourceRoot.fileFixtures(50)

  @Test
  fun `test commit document reparses both psi versions`() = timeoutRunBlocking {
-    val project = projectFixture.get()
-
    IndexingTestUtil.waitUntilIndexesAreReady(project)

    val (psiFile1, psiFile2) = readAction {
@@ -70,6 +80,73 @@ internal class CommitMultiverseFileTest {
    }
  }

+  /**
+   * This test checks a situation:
+   * a virtual file has a PSI file for context A.
+   * The file text gets updated and is being asynchronously committed.
+   * At the same time, another read-action requests a PSI file of this file with context B.
+   * The requested PSI file must stay in sync with the first PSI file.
+   *
+   * When commit infra is not ready for this, around 10% of test runs fail.
+   */
+  @RepeatedTest(100)
+  fun `test commit document and requesting psi for another context at the same time`() = timeoutRunBlocking(20.seconds) {
+    IndexingTestUtil.waitUntilIndexesAreReady(project)
+
+    // preparing psi files
+    val psiFiles1 = readAction {
+      files.map { psiManager.findFile(it, context1)!! }
+    }
+
+    readAction {
+      for (file in psiFiles1) {
+        ensureParsed(file)
+      }
+    }
+
+    val documents = readAction {
+      files.map { FileDocumentManager.getInstance().getDocument(it)!! }
+    }
+
+    // adding long suffix to all files
+    writeAction {
+      for (file in files) {
+        Assertions.assertEquals(1, psiManager.fileManagerEx.findCachedViewProviders(file).size)
+      }
+
+      documents.forEach {
+        CommandProcessor.getInstance().executeCommand(project, {
+          it.insertString(it.textLength, longText)
+        }, null, null)
+      }
+    }
+
+    // requesting another psi file version, trying to do that concurrently with async commit
+    val psiFiles2 = files.asReversed().mapIndexed { index, file ->
+      async {
+        delay(index.milliseconds) // waiting a bit to increase the chance of clashing between async commit and file request
+        readAction {
+          val psi = psiManager.findFile(file, context2)!!
+          ensureParsed(psi)
+          psi
+        }
+      }
+    }.awaitAll()
+
+    // waiting for commit to finish
+    while (PsiDocumentManager.getInstance(project).hasUncommitedDocuments()) {
+      delay(50)
+    }
+
+    // ensuring everything is in sync
+    readAction {
+      for (file in psiFiles1 + psiFiles2) {
+        PsiUtilCore.ensureValid(file)
+        file.viewProvider.contentsSynchronized()
+      }
+    }
+  }
+
  private fun ensureParsed(file: PsiFile) {
    file.accept(object : PsiElementVisitor() {
      override fun visitElement(element: PsiElement) {
@@ -77,4 +154,32 @@ internal class CommitMultiverseFileTest {
      }
    })
  }
+}
+
+/**
+ *
+ * class A1 {
+ * class A2 {
+ * class A3 {
+ * ...
+ * }
+ * }
+ * }
+ */
+private val longText = buildString {
+  val times = 20
+  appendLine()
+  repeat(times) {
+    appendLine("class A$it {")
+  }
+  repeat(times) {
+    appendLine("}")
+  }
+}
+
+private fun TestFixture<PsiDirectory>.fileFixtures(number: Int) = testFixture {
+  val files = (0..number).map {
+    virtualFileFixture("file$it.java", "class A {}").init()
+  }
+  initialized(files) {}
 }