don't invalidate all PSI on root change

It was originally done to avoid UI freeze (IDEA-172762), but has led to too many PIEAEs (like IDEA-191185, IDEA-188292, IDEA-184186, EA-114990). Ideally those clients should all be converted to smart pointers, but that proved to be quite hard to do, especially without breaking API. And they mostly worked before those batch invalidations. So now we have a smarter way of dealing with this issue. On root change, we mark PSI as "potentially invalid", and then, when someone calls "isValid" (hopefully not for all PSI and hopefully in a background thread), we check if the old PSI is equivalent to the one that would be re-created in its place. If yes, we return valid. If no, we invalidate the old PSI forever and return the new one.
2026-09-27 10:03:11 +07:00 · 2018-05-19 08:40:54 +02:00
parent b794841223
commit 372b4d43a2
5 changed files with 171 additions and 48 deletions
@@ -44,6 +44,7 @@ import com.intellij.psi.impl.file.PsiBinaryFileImpl;
 import com.intellij.psi.impl.file.PsiLargeBinaryFileImpl;
 import com.intellij.psi.impl.file.PsiLargeTextFileImpl;
 import com.intellij.psi.impl.file.impl.FileManager;
+import com.intellij.psi.impl.file.impl.FileManagerImpl;
 import com.intellij.psi.impl.source.PsiFileImpl;
 import com.intellij.psi.impl.source.PsiPlainTextFileImpl;
 import com.intellij.psi.impl.source.SourceTreeToPsiMap;
@@ -62,6 +63,7 @@ import java.lang.ref.SoftReference;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.function.Consumer;

 public abstract class AbstractFileViewProvider extends UserDataHolderBase implements FileViewProvider {
  private static final Logger LOG = Logger.getInstance("#com.intellij.psi.AbstractFileViewProvider");
@@ -73,7 +75,6 @@ public abstract class AbstractFileViewProvider extends UserDataHolderBase implem
  private final VirtualFile myVirtualFile;
  private final boolean myEventSystemEnabled;
  private final boolean myPhysical;
-  private boolean myInvalidated;
  private volatile Content myContent;
  private volatile Reference<Document> myDocument;
  @NotNull
@@ -418,11 +419,13 @@ public abstract class AbstractFileViewProvider extends UserDataHolderBase implem
  public abstract List<FileElement> getKnownTreeRoots();

  public final void markInvalidated() {
-    if (myInvalidated) return;
-
    invalidateCachedPsi();
-    myInvalidated = true;
-    invalidateCopies();
+    forKnownCopies(copy -> myManager.getFileManager().setViewProvider(copy.getVirtualFile(), null));
+  }
+
+  public final void markPossiblyInvalidated() {
+    invalidateCachedPsi();
+    forKnownCopies(FileManagerImpl::markPossiblyInvalidated);
  }

  private void invalidateCachedPsi() {
@@ -433,12 +436,12 @@ public abstract class AbstractFileViewProvider extends UserDataHolderBase implem
    }
  }

-  private void invalidateCopies() {
+  private void forKnownCopies(Consumer<AbstractFileViewProvider> action) {
    Set<AbstractFileViewProvider> knownCopies = getUserData(KNOWN_COPIES);
    if (knownCopies != null) {
      for (AbstractFileViewProvider copy : knownCopies) {
        if (copy.getCachedPsiFiles().stream().anyMatch(f -> f.getOriginalFile().getViewProvider() == this)) {
-          myManager.getFileManager().setViewProvider(copy.getVirtualFile(), null);
+          action.accept(copy);
        }
      }
    }
@@ -24,6 +24,7 @@ import com.intellij.openapi.util.TextRange;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.psi.*;
 import com.intellij.psi.impl.*;
+import com.intellij.psi.impl.file.impl.FileManagerImpl;
 import com.intellij.psi.impl.source.resolve.FileContextUtil;
 import com.intellij.psi.search.PsiElementProcessor;
 import com.intellij.util.ArrayUtil;
@@ -39,11 +40,11 @@ public class PsiBinaryFileImpl extends PsiElementBase implements PsiBinaryFile,
  private final PsiManagerImpl myManager;
  private String myName; // for myFile == null only
  private byte[] myContents; // for myFile == null only
-  private final FileViewProvider myViewProvider;
-  private boolean myInvalidated;
+  private final AbstractFileViewProvider myViewProvider;
+  private volatile boolean myPossiblyInvalidated;

  public PsiBinaryFileImpl(PsiManagerImpl manager, FileViewProvider viewProvider) {
-    myViewProvider = viewProvider;
+    myViewProvider = (AbstractFileViewProvider)viewProvider;
    myManager = manager;
  }

@@ -244,7 +245,18 @@ public class PsiBinaryFileImpl extends PsiElementBase implements PsiBinaryFile,
  @Override
  public boolean isValid() {
    if (isCopy()) return true; // "dummy" file
-    return getVirtualFile().isValid() && !myManager.getProject().isDisposed() && !myInvalidated;
+    if (!getVirtualFile().isValid() || myManager.getProject().isDisposed()) return false;
+
+
+    if (!myPossiblyInvalidated) return true;
+
+    // synchronized by read-write action
+    if (((FileManagerImpl)myManager.getFileManager()).evaluateValidity(myViewProvider)) {
+      myPossiblyInvalidated = false;
+      PsiInvalidElementAccessException.setInvalidationTrace(this, null);
+      return true;
+    }
+    return false;
  }

  @Override
@@ -317,7 +329,7 @@ public class PsiBinaryFileImpl extends PsiElementBase implements PsiBinaryFile,

  @Override
  public void markInvalidated() {
-    myInvalidated = true;
+    myPossiblyInvalidated = true;
    DebugUtil.onInvalidated(this);
  }
 }
@@ -27,18 +27,13 @@ import com.intellij.openapi.fileEditor.FileDocumentManager;
 import com.intellij.openapi.fileTypes.FileType;
 import com.intellij.openapi.project.DumbService;
 import com.intellij.openapi.roots.FileIndexFacade;
-import com.intellij.openapi.util.Disposer;
-import com.intellij.openapi.util.Key;
-import com.intellij.openapi.util.LowMemoryWatcher;
+import com.intellij.openapi.util.*;
 import com.intellij.openapi.util.registry.Registry;
 import com.intellij.openapi.vfs.VfsUtilCore;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileVisitor;
 import com.intellij.psi.*;
-import com.intellij.psi.impl.DebugUtil;
-import com.intellij.psi.impl.PsiManagerImpl;
-import com.intellij.psi.impl.PsiModificationTrackerImpl;
-import com.intellij.psi.impl.PsiTreeChangeEventImpl;
+import com.intellij.psi.impl.*;
 import com.intellij.psi.impl.file.PsiDirectoryFactory;
 import com.intellij.testFramework.LightVirtualFile;
 import com.intellij.util.ConcurrencyUtil;
@@ -54,6 +49,7 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicReference;

 public class FileManagerImpl implements FileManager {
+  private static final Key<Boolean> IN_COMA = Key.create("IN_COMA");
  private static final Logger LOG = Logger.getInstance("#com.intellij.psi.impl.file.impl.FileManagerImpl");
  private final Key<FileViewProvider> myPsiHardRefKey = Key.create("HARD_REFERENCE_TO_PSI"); //non-static!

@@ -63,6 +59,11 @@ public class FileManagerImpl implements FileManager {
  private final AtomicReference<ConcurrentMap<VirtualFile, PsiDirectory>> myVFileToPsiDirMap = new AtomicReference<>();
  private final AtomicReference<ConcurrentMap<VirtualFile, FileViewProvider>> myVFileToViewProviderMap = new AtomicReference<>();

+  /**
+   * Holds thread-local temporary providers that are sometimes needed while checking if a file is valid
+   */
+  private final ThreadLocal<Map<VirtualFile, FileViewProvider>> myTempProviders = ThreadLocal.withInitial(() -> new HashMap<>());
+
  private boolean myDisposed;

  private final FileDocumentManager myFileDocumentManager;
@@ -197,6 +198,12 @@ public class FileManagerImpl implements FileManager {
    if (file instanceof VirtualFileWindow) {
      throw new IllegalStateException("File " + file + " is invalid");
    }
+
+    Map<VirtualFile, FileViewProvider> tempMap = myTempProviders.get();
+    if (tempMap.containsKey(file)) {
+      return Objects.requireNonNull(tempMap.get(file), "Recursive file view provider creation");
+    }
+
    viewProvider = createFileViewProvider(file, true);
    if (file instanceof LightVirtualFile) {
      return file.putUserDataIfAbsent(myPsiHardRefKey, viewProvider);
@@ -206,21 +213,34 @@ public class FileManagerImpl implements FileManager {

  @Override
  public FileViewProvider findCachedViewProvider(@NotNull final VirtualFile file) {
+    FileViewProvider viewProvider = getRawCachedViewProvider(file);
+
+    if (viewProvider instanceof AbstractFileViewProvider && viewProvider.getUserData(IN_COMA) != null) {
+      Map<VirtualFile, FileViewProvider> tempMap = myTempProviders.get();
+      if (tempMap.containsKey(file)) {
+        return tempMap.get(file);
+      }
+
+      if (!evaluateValidity((AbstractFileViewProvider)viewProvider)) {
+        return null;
+      }
+    }
+    return viewProvider;
+  }
+
+  @Nullable
+  private FileViewProvider getRawCachedViewProvider(@NotNull VirtualFile file) {
    ConcurrentMap<VirtualFile, FileViewProvider> map = myVFileToViewProviderMap.get();
    FileViewProvider viewProvider = map == null ? null : map.get(file);
-    if (viewProvider == null) viewProvider = file.getUserData(myPsiHardRefKey);
-    return viewProvider;
+    return viewProvider == null ? file.getUserData(myPsiHardRefKey) : viewProvider;
  }

  @Override
  public void setViewProvider(@NotNull final VirtualFile virtualFile, @Nullable final FileViewProvider fileViewProvider) {
-    FileViewProvider prev = findCachedViewProvider(virtualFile);
+    FileViewProvider prev = getRawCachedViewProvider(virtualFile);
    if (prev == fileViewProvider) return;
    if (prev != null) {
-      DebugUtil.performPsiModification(null, () -> {
-        markInvalidated(prev);
-        DebugUtil.onInvalidated(prev);
-      });
+      DebugUtil.performPsiModification(null, () -> markInvalidated(prev));
    }

    if (fileViewProvider == null) {
@@ -272,7 +292,7 @@ public class FileManagerImpl implements FileManager {
          event.setPropertyName(PsiTreeChangeEvent.PROP_FILE_TYPES);
          myManager.beforePropertyChange(event);

-          invalidateAllPsi();
+          possiblyInvalidatePhysicalPsi();

          myManager.propertyChanged(event);
        });
@@ -283,12 +303,12 @@ public class FileManagerImpl implements FileManager {
    });
  }

-  void invalidateAllPsi() {
-    myVFileToPsiDirMap.set(null);
-    for (final FileViewProvider provider : getVFileToViewProviderMap().values()) {
-      markInvalidated(provider);
+  void possiblyInvalidatePhysicalPsi() {
+    ApplicationManager.getApplication().assertWriteAccessAllowed();
+    removeInvalidDirs(true);
+    for (FileViewProvider provider : getVFileToViewProviderMap().values()) {
+      markPossiblyInvalidated(provider);
    }
-    myVFileToViewProviderMap.set(null);
  }

  void dispatchPendingEvents() {
@@ -301,6 +321,10 @@ public class FileManagerImpl implements FileManager {

  @TestOnly
  public void checkConsistency() {
+    for (VirtualFile file : new ArrayList<>(getVFileToViewProviderMap().keySet())) {
+      findCachedViewProvider(file); // complete delayed validity checks
+    }
+
    Map<VirtualFile, FileViewProvider> fileToViewProvider = new HashMap<>(getVFileToViewProviderMap());
    myVFileToViewProviderMap.set(null);
    for (Map.Entry<VirtualFile, FileViewProvider> entry : fileToViewProvider.entrySet()) {
@@ -426,10 +450,18 @@ public class FileManagerImpl implements FileManager {
  }

  private void markInvalidated(@NotNull FileViewProvider viewProvider) {
+    viewProvider.putUserData(IN_COMA, null);
    ((AbstractFileViewProvider)viewProvider).markInvalidated();
    viewProvider.getVirtualFile().putUserData(myPsiHardRefKey, null);
  }

+  public static void markPossiblyInvalidated(@NotNull FileViewProvider viewProvider) {
+    LOG.assertTrue(!(viewProvider instanceof FreeThreadedFileViewProvider));
+    viewProvider.putUserData(IN_COMA, true);
+    ((AbstractFileViewProvider)viewProvider).markPossiblyInvalidated();
+    clearPsiCaches(viewProvider);
+  }
+
  @Nullable
  PsiFile getCachedPsiFileInner(@NotNull VirtualFile file) {
    FileViewProvider fileViewProvider = findCachedViewProvider(file);
@@ -440,8 +472,11 @@ public class FileManagerImpl implements FileManager {
  @Override
  public List<PsiFile> getAllCachedFiles() {
    List<PsiFile> files = new ArrayList<>();
-    for (FileViewProvider provider : getVFileToViewProviderMap().values()) {
-      ContainerUtil.addIfNotNull(files, ((AbstractFileViewProvider)provider).getCachedPsi(provider.getBaseLanguage()));
+    for (VirtualFile file : new ArrayList<>(getVFileToViewProviderMap().keySet())) {
+      FileViewProvider provider = findCachedViewProvider(file);
+      if (provider != null) {
+        ContainerUtil.addIfNotNull(files, ((AbstractFileViewProvider)provider).getCachedPsi(provider.getBaseLanguage()));
+      }
    }
    return files;
  }
@@ -484,8 +519,8 @@ public class FileManagerImpl implements FileManager {
        continue;
      }

+      FileViewProvider view = fileToPsiFileMap.get(vFile);
      if (useFind) {
-        FileViewProvider view = fileToPsiFileMap.get(vFile);
        if (view == null) { // soft ref. collected
          iterator.remove();
          continue;
@@ -503,6 +538,9 @@ public class FileManagerImpl implements FileManager {
          clearPsiCaches(view);
        }
      }
+      else if (!evaluateValidity((AbstractFileViewProvider)view)) {
+        iterator.remove();
+      }
    }
    myVFileToViewProviderMap.set(null);
    getVFileToViewProviderMap().putAll(fileToPsiFileMap);
@@ -519,10 +557,8 @@ public class FileManagerImpl implements FileManager {
    if (!view1.getLanguages().equals(view2.getLanguages())) return false;
    PsiFile psi1 = view1.getPsi(baseLanguage);
    PsiFile psi2 = view2.getPsi(baseLanguage);
-    if (psi1 == null) return psi2 == null;
-    if (psi1.getClass() != psi2.getClass()) return false;
-
-    return true;
+    if (psi1 == null || psi2 == null) return psi1 == psi2;
+    return psi1.getClass() == psi2.getClass();
  }

  private void markInvalidations(@NotNull Map<VirtualFile, FileViewProvider> originalFileToPsiFileMap) {
@@ -559,4 +595,67 @@ public class FileManagerImpl implements FileManager {

    ((AbstractFileViewProvider)viewProvider).onContentReload();
  }
+
+  /**
+   * Should be called only from implementations of {@link PsiFile#isValid()}, only after they've been {@link PsiFileEx#markInvalidated()},
+   * and only to check if they can be made valid again.
+   * Synchronized by read-write action. Calls from several threads in read action for the same virtual file are allowed.
+   * @return if the view provider is still valid
+   */
+  public boolean evaluateValidity(@NotNull AbstractFileViewProvider viewProvider) {
+    ApplicationManager.getApplication().assertReadAccessAllowed();
+    
+    VirtualFile file = viewProvider.getVirtualFile();
+    if (getRawCachedViewProvider(file) != viewProvider) {
+      return false;
+    }
+
+    if (viewProvider.getUserData(IN_COMA) == null) {
+      return true;
+    }
+
+    if (shouldResurrect(viewProvider, file)) {
+      viewProvider.putUserData(IN_COMA, null);
+      LOG.assertTrue(getRawCachedViewProvider(file) == viewProvider);
+
+      for (PsiFile psiFile : viewProvider.getCachedPsiFiles()) {
+        // update "myPossiblyInvalidated" fields in files
+        // that will call us recursively again, but since we're not IN_COMA now, we'll exit earlier and avoid SOE
+        LOG.assertTrue(psiFile.isValid());
+      }
+      return true;
+    }
+    
+    getVFileToViewProviderMap().remove(file, viewProvider);
+    file.replace(myPsiHardRefKey, viewProvider, null);
+    viewProvider.putUserData(IN_COMA, null);
+
+    return false;
+  }
+
+  private boolean shouldResurrect(FileViewProvider viewProvider, VirtualFile file) {
+    if (!file.isValid()) return false;
+
+    Map<VirtualFile, FileViewProvider> tempProviders = myTempProviders.get();
+    LOG.assertTrue(!tempProviders.containsKey(file), "isValid leads to endless recursion");
+    tempProviders.put(file, null);
+    try {
+      FileViewProvider recreated = createFileViewProvider(file, true);
+      tempProviders.put(file, recreated);
+      return areViewProvidersEquivalent(viewProvider, recreated) &&
+             ((AbstractFileViewProvider)viewProvider).getCachedPsiFiles().stream().noneMatch(f -> hasInvalidOriginal(f));
+    }
+    finally {
+      FileViewProvider temp = tempProviders.remove(file);
+      if (temp != null) {
+        DebugUtil.performPsiModification("invalidate temp view provider", () -> ((AbstractFileViewProvider)temp).markInvalidated());
+      }
+    }
+  }
+
+  private static boolean hasInvalidOriginal(PsiFile file) {
+    PsiFile original = file.getOriginalFile();
+    return original != file && !original.isValid();
+  }
+
 }
@@ -63,10 +63,9 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF
  private long myModificationStamp;

  protected PsiFile myOriginalFile;
-  private final FileViewProvider myViewProvider;
+  private final AbstractFileViewProvider myViewProvider;
  private volatile FileTrees myTrees = FileTrees.noStub(null, this);
-  private boolean myInvalidated;
-  @SuppressWarnings("FieldAccessedSynchronizedAndUnsynchronized")
+  private volatile boolean myPossiblyInvalidated;
  protected final PsiManagerEx myManager;
  public static final Key<Boolean> BUILDING_STUB = new Key<>("Don't use stubs mark!");
  private final PsiLock myPsiLock;
@@ -78,8 +77,8 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF

  protected PsiFileImpl(@NotNull FileViewProvider provider ) {
    myManager = (PsiManagerEx)provider.getManager();
-    myViewProvider = provider;
-    myPsiLock = ((AbstractFileViewProvider) provider).getFilePsiLock();
+    myViewProvider = (AbstractFileViewProvider)provider;
+    myPsiLock = myViewProvider.getFilePsiLock();
  }

  public void setContentElementType(final IElementType contentElementType) {
@@ -146,12 +145,22 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF
      // but some VFS listeners receive the same events before that and ask PsiFile.isValid
      return false;
    }
-    return !myInvalidated;
+
+    if (!myPossiblyInvalidated) return true;
+    
+    // synchronized by read-write action
+    if (((FileManagerImpl)myManager.getFileManager()).evaluateValidity(myViewProvider)) {
+      myPossiblyInvalidated = false;
+      PsiInvalidElementAccessException.setInvalidationTrace(this, null);
+      return true;
+    }
+    System.out.println("fail " + getName() + Thread.currentThread());
+    return false;
  }

  @Override
-  public void markInvalidated() {
-    myInvalidated = true;
+  public final void markInvalidated() {
+    myPossiblyInvalidated = true;
    DebugUtil.onInvalidated(this);
  }

@@ -585,7 +585,7 @@ public class PsiVFSListener implements VirtualFileListener, BulkFileListener {
          assert depthCounter >= 0 : depthCounter;
          if (depthCounter > 0) return;

-          DebugUtil.performPsiModification(null, () -> myFileManager.invalidateAllPsi());
+          DebugUtil.performPsiModification(null, () -> myFileManager.possiblyInvalidatePhysicalPsi());

          PsiTreeChangeEventImpl treeEvent = new PsiTreeChangeEventImpl(myManager);
          treeEvent.setPropertyName(PsiTreeChangeEvent.PROP_ROOTS);