IdentityFilePointer should be removed from myUrlToIdentity cache on dispose

diff --git a/java/idea-ui/src/com/intellij/openapi/roots/ui/OrderEntryAppearanceServiceImpl.java b/java/idea-ui/src/com/intellij/openapi/roots/ui/OrderEntryAppearanceServiceImpl.java
index ddabaf3f419ae34fa052377668e2c4e19ac81116..f6dbec5fc36873c02e2194f03360d296cbae5dd6 100644 (file)
--- a/java/idea-ui/src/com/intellij/openapi/roots/ui/OrderEntryAppearanceServiceImpl.java
+++ b/java/idea-ui/src/com/intellij/openapi/roots/ui/OrderEntryAppearanceServiceImpl.java
@@ -36,6 +36,7 @@ import com.intellij.openapi.vfs.JarFileSystem;
 import com.intellij.openapi.vfs.VfsUtilCore;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileManager;
+import com.intellij.openapi.vfs.impl.LightFilePointer;
 import com.intellij.ui.JBColor;
 import com.intellij.ui.SimpleTextAttributes;
 import com.intellij.util.PathUtil;

diff --git a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/libraryEditor/NewLibraryEditor.java b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/libraryEditor/NewLibraryEditor.java
index 4825ad71c111cd4fc1a11bcf2b970f8739f27334..36a26ed809a2cf0f032f2ef00e61e692be4bb7c7 100644 (file)
--- a/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/libraryEditor/NewLibraryEditor.java
+++ b/java/idea-ui/src/com/intellij/openapi/roots/ui/configuration/libraryEditor/NewLibraryEditor.java
@@ -21,10 +21,9 @@ import com.intellij.openapi.roots.impl.libraries.LibraryEx;
 import com.intellij.openapi.roots.impl.libraries.LibraryImpl;
 import com.intellij.openapi.roots.libraries.LibraryProperties;
 import com.intellij.openapi.roots.libraries.LibraryType;
-import com.intellij.openapi.roots.ui.LightFilePointer;
-import com.intellij.openapi.vfs.VfsUtil;
 import com.intellij.openapi.vfs.VfsUtilCore;
 import com.intellij.openapi.vfs.VirtualFile;
+import com.intellij.openapi.vfs.impl.LightFilePointer;
 import com.intellij.util.ArrayUtil;
 import com.intellij.util.containers.MultiMap;
 import org.jetbrains.annotations.NotNull;
@@ -116,7 +115,7 @@ public class NewLibraryEditor extends LibraryEditorBase {
       }
       result.add(file);
     }
-    return VfsUtil.toVirtualFileArray(result);
+    return VfsUtilCore.toVirtualFileArray(result);
   }
 
   @Override
@@ -149,6 +148,7 @@ public class NewLibraryEditor extends LibraryEditorBase {
     myExcludedRoots.add(new LightFilePointer(url));
   }
 
+  @Override
   public void removeExcludedRoot(@NotNull String url) {
     myExcludedRoots.remove(new LightFilePointer(url));
   }

diff --git a/platform/core-impl/src/com/intellij/openapi/vfs/impl/CoreVirtualFilePointerManager.java b/platform/core-impl/src/com/intellij/openapi/vfs/impl/CoreVirtualFilePointerManager.java
index da6c265a49379fecb3d34f4ebd5c2414703687f2..df8a19f730388b307c466d1416bc934f6dfddb64 100644 (file)
--- a/platform/core-impl/src/com/intellij/openapi/vfs/impl/CoreVirtualFilePointerManager.java
+++ b/platform/core-impl/src/com/intellij/openapi/vfs/impl/CoreVirtualFilePointerManager.java
@@ -17,7 +17,6 @@ package com.intellij.openapi.vfs.impl;
 
 import com.intellij.openapi.Disposable;
 import com.intellij.openapi.vfs.VirtualFile;
-import com.intellij.openapi.vfs.VirtualFileManager;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointer;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointerContainer;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointerListener;
@@ -32,14 +31,13 @@ public class CoreVirtualFilePointerManager extends VirtualFilePointerManager {
   @NotNull
   @Override
   public VirtualFilePointer create(@NotNull String url, @NotNull Disposable parent, @Nullable VirtualFilePointerListener listener) {
-    VirtualFile vFile = VirtualFileManager.getInstance().findFileByUrl(url);
-    return new IdentityVirtualFilePointer(vFile, url);
+    return new LightFilePointer(url);
   }
 
   @NotNull
   @Override
   public VirtualFilePointer create(@NotNull VirtualFile file, @NotNull Disposable parent, @Nullable VirtualFilePointerListener listener) {
-    return new IdentityVirtualFilePointer(file, file.getUrl());
+    return new LightFilePointer(file);
   }
 
   @NotNull
@@ -47,7 +45,7 @@ public class CoreVirtualFilePointerManager extends VirtualFilePointerManager {
   public VirtualFilePointer duplicate(@NotNull VirtualFilePointer pointer,
                                       @NotNull Disposable parent,
                                       @Nullable VirtualFilePointerListener listener) {
-    return new IdentityVirtualFilePointer(pointer.getFile(), pointer.getUrl());
+    return new LightFilePointer(pointer.getUrl());
   }
 
   @NotNull

diff --git a/platform/platform-api/src/com/intellij/openapi/roots/ui/LightFilePointer.java b/platform/core-impl/src/com/intellij/openapi/vfs/impl/LightFilePointer.java
similarity index 80%
rename from platform/platform-api/src/com/intellij/openapi/roots/ui/LightFilePointer.java
rename to platform/core-impl/src/com/intellij/openapi/vfs/impl/LightFilePointer.java
index ab1b543a829b574a55dbe76988c61d8c24669d37..e4a874ba151b7cb7abd09a4fa0dcd6bf00a3b627 100644 (file)
--- a/platform/platform-api/src/com/intellij/openapi/roots/ui/LightFilePointer.java
+++ b/platform/core-impl/src/com/intellij/openapi/vfs/impl/LightFilePointer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2015 JetBrains s.r.o.
+ * Copyright 2000-2016 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -13,18 +13,17 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package com.intellij.openapi.roots.ui;
+package com.intellij.openapi.vfs.impl;
 
-import com.intellij.openapi.util.text.StringUtil;
-import com.intellij.openapi.vfs.JarFileSystem;
+import com.intellij.openapi.vfs.StandardFileSystems;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileManager;
+import com.intellij.openapi.vfs.VirtualFileSystem;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointer;
+import com.intellij.util.ObjectUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.io.File;
-
 public class LightFilePointer implements VirtualFilePointer {
   private final String myUrl;
   private VirtualFile myFile;
@@ -69,10 +68,12 @@ public class LightFilePointer implements VirtualFilePointer {
     return toPresentableUrl(myUrl);
   }
 
-  public static String toPresentableUrl(String url) {
+  @NotNull
+  private static String toPresentableUrl(@NotNull String url) {
     String path = VirtualFileManager.extractPath(url);
-    path = StringUtil.trimEnd(path, JarFileSystem.JAR_SEPARATOR);
-    return path.replace('/', File.separatorChar);
+    String protocol = VirtualFileManager.extractProtocol(url);
+    VirtualFileSystem fileSystem = VirtualFileManager.getInstance().getFileSystem(protocol);
+    return ObjectUtils.notNull(fileSystem, StandardFileSystems.local()).extractPresentableUrl(path);
   }
 
   @Override

diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/HistoryEntry.java b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/HistoryEntry.java
index d9253e8d97bce4b38db0dbbce1e7bd6bcc932a61..be09a49388f572a5ca569722c3267be3c8bbed0b 100644 (file)
--- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/HistoryEntry.java
+++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/HistoryEntry.java
@@ -20,7 +20,7 @@ import com.intellij.openapi.fileEditor.FileEditorProvider;
 import com.intellij.openapi.fileEditor.FileEditorState;
 import com.intellij.openapi.fileEditor.ex.FileEditorProviderManager;
 import com.intellij.openapi.project.Project;
-import com.intellij.openapi.roots.ui.LightFilePointer;
+import com.intellij.openapi.vfs.impl.LightFilePointer;
 import com.intellij.openapi.util.Disposer;
 import com.intellij.openapi.util.InvalidDataException;
 import com.intellij.openapi.util.Pair;

diff --git a/platform/core-impl/src/com/intellij/openapi/vfs/impl/IdentityVirtualFilePointer.java b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/IdentityVirtualFilePointer.java
similarity index 72%
rename from platform/core-impl/src/com/intellij/openapi/vfs/impl/IdentityVirtualFilePointer.java
rename to platform/platform-impl/src/com/intellij/openapi/vfs/impl/IdentityVirtualFilePointer.java
index b66b00c75532aa4efb3c1e7e4240f8d27cb71c82..ae7fe408344cffe8a2e4ad22e206236c51b32172 100644 (file)
--- a/platform/core-impl/src/com/intellij/openapi/vfs/impl/IdentityVirtualFilePointer.java
+++ b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/IdentityVirtualFilePointer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2012 JetBrains s.r.o.
+ * Copyright 2000-2016 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,18 +15,22 @@
  */
 package com.intellij.openapi.vfs.impl;
 
+import com.intellij.openapi.Disposable;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointer;
+import com.intellij.openapi.vfs.pointers.VirtualFilePointerListener;
 import org.jetbrains.annotations.NotNull;
 
 /**
  * @author cdr
  */
-class IdentityVirtualFilePointer implements VirtualFilePointer {
+class IdentityVirtualFilePointer extends VirtualFilePointerImpl implements VirtualFilePointer, Disposable {
   private final VirtualFile myFile;
   private final String myUrl;
+  private volatile int useCount;
 
-  IdentityVirtualFilePointer(VirtualFile file, @NotNull String url) {
+  IdentityVirtualFilePointer(VirtualFile file, @NotNull String url, VirtualFilePointerListener listener) {
+    super(listener);
     myFile = file;
     myUrl = url;
   }
@@ -58,4 +62,14 @@ class IdentityVirtualFilePointer implements VirtualFilePointer {
   public boolean isValid() {
     return myFile == null || myFile.isValid();
   }
+
+  @Override
+  int incrementUsageCount(int delta) {
+    return useCount += delta;
+  }
+
+  @Override
+  public void dispose() {
+    incrementUsageCount(-1);
+  }
 }
\ No newline at end of file

diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerImpl.java
index 53812bd2f9b5cb6fb0292ccf02ce641bedd713df..dea9432d00646ae9eaf781cefb42db0c4f16ccd6 100644 (file)
--- a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerImpl.java
+++ b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerImpl.java
@@ -15,7 +15,6 @@
  */
 package com.intellij.openapi.vfs.impl;
 
-import com.intellij.openapi.Disposable;
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.progress.ProgressManager;
@@ -27,6 +26,7 @@ import com.intellij.openapi.vfs.pointers.VirtualFilePointerListener;
 import com.intellij.openapi.vfs.pointers.VirtualFilePointerManager;
 import com.intellij.util.PathUtil;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 class VirtualFilePointerImpl extends TraceableDisposable implements VirtualFilePointer {
   private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.vfs.impl.VirtualFilePointerImpl");
@@ -36,7 +36,7 @@ class VirtualFilePointerImpl extends TraceableDisposable implements VirtualFileP
 
   volatile FilePointerPartNode myNode; // null means disposed
 
-  VirtualFilePointerImpl(VirtualFilePointerListener listener, @NotNull Disposable parentDisposable, Pair<VirtualFile, String> fileAndUrl) {
+  VirtualFilePointerImpl(@Nullable VirtualFilePointerListener listener) {
     super(TRACE_CREATION);
     myListener = listener;
   }
@@ -71,7 +71,7 @@ class VirtualFilePointerImpl extends TraceableDisposable implements VirtualFileP
   }
 
   @NotNull
-  String getUrlNoUpdate() {
+  private String getUrlNoUpdate() {
     return isDisposed() ? "" : myNode.myFileAndUrl.second;
   }
 
@@ -122,4 +122,8 @@ class VirtualFilePointerImpl extends TraceableDisposable implements VirtualFileP
   VirtualFilePointerListener getListener() {
     return myListener;
   }
+
+  int incrementUsageCount(int delta) {
+    return myNode.incrementUsageCount(delta);
+  }
 }

diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java
index 87f784a97498f26186419fb51360ecf427b10ac4..8a4a386bcae61a96857f7cbde9b3b3172e148d3e 100644 (file)
--- a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java
+++ b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/VirtualFilePointerManagerImpl.java
@@ -180,7 +180,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
     if (fileSystem == TEMP_FILE_SYSTEM) {
       // for tests, recreate always
       VirtualFile found = file == null ? VirtualFileManager.getInstance().findFileByUrl(url) : file;
-      return new IdentityVirtualFilePointer(found, url);
+      return found == null ? new LightFilePointer(url) : new LightFilePointer(found);
     }
 
     boolean isJar = fileSystem == JAR_FILE_SYSTEM;
@@ -188,7 +188,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
       // we are unable to track alien file systems for now
       VirtualFile found = fileSystem == null ? null : file != null ? file : VirtualFileManager.getInstance().findFileByUrl(url);
       // if file is null, this pointer will never be alive
-      return getOrCreateIdentity(url, found);
+      return getOrCreateIdentity(url, found, parentDisposable, listener);
     }
 
     if (file == null) {
@@ -209,16 +209,34 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
       }
     }
     // else url has come from VirtualFile.getPath() and is good enough
-
-    VirtualFilePointerImpl pointer = getOrCreate(parentDisposable, listener, path, Pair.create(file, url));
+    VirtualFilePointerImpl pointer = getOrCreate(listener, path, Pair.create(file, url));
     DelegatingDisposable.registerDisposable(parentDisposable, pointer);
     return pointer;
   }
 
   private final Map<String, IdentityVirtualFilePointer> myUrlToIdentity = new THashMap<>();
   @NotNull
-  private IdentityVirtualFilePointer getOrCreateIdentity(@NotNull String url, @Nullable VirtualFile found) {
-    return myUrlToIdentity.computeIfAbsent(url, __ -> new IdentityVirtualFilePointer(found, url));
+  private IdentityVirtualFilePointer getOrCreateIdentity(@NotNull String url,
+                                                         @Nullable VirtualFile found,
+                                                         @NotNull Disposable parentDisposable,
+                                                         VirtualFilePointerListener listener) {
+    IdentityVirtualFilePointer pointer = myUrlToIdentity.get(url);
+    if (pointer == null) {
+      pointer = new IdentityVirtualFilePointer(found, url,listener){
+        @Override
+        public void dispose() {
+          synchronized (VirtualFilePointerManagerImpl.this) {
+            super.dispose();
+            myUrlToIdentity.remove(url);
+          }
+        }
+      };
+      myUrlToIdentity.put(url, pointer);
+
+      DelegatingDisposable.registerDisposable(parentDisposable, pointer);
+    }
+    pointer.incrementUsageCount(1);
+    return pointer;
   }
 
   @NotNull
@@ -236,8 +254,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
   }
 
   @NotNull
-  private VirtualFilePointerImpl getOrCreate(@NotNull Disposable parentDisposable,
-                                             @Nullable VirtualFilePointerListener listener,
+  private VirtualFilePointerImpl getOrCreate(@Nullable VirtualFilePointerListener listener,
                                              @NotNull String path,
                                              @NotNull Pair<VirtualFile, String> fileAndUrl) {
     FilePointerPartNode root = myPointers.get(listener);
@@ -254,10 +271,10 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
 
     VirtualFilePointerImpl pointer = node.getAnyPointer();
     if (pointer == null) {
-      pointer = new VirtualFilePointerImpl(listener, parentDisposable, fileAndUrl);
+      pointer = new VirtualFilePointerImpl(listener);
       node.associate(pointer, fileAndUrl);
     }
-    pointer.myNode.incrementUsageCount(1);
+    pointer.incrementUsageCount(1);
 
     root.checkConsistency();
     return pointer;
@@ -546,7 +563,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
 
   private static class DelegatingDisposable implements Disposable {
     private static final ConcurrentMap<Disposable, DelegatingDisposable> ourInstances = ContainerUtil.newConcurrentMap(ContainerUtil.<Disposable>identityStrategy());
-    private final TObjectIntHashMap<VirtualFilePointerImpl> myCounts = new TObjectIntHashMap<>();
+    private final TObjectIntHashMap<VirtualFilePointerImpl> myCounts = new TObjectIntHashMap<>(); // guarded by this
     private final Disposable myParent;
 
     private DelegatingDisposable(@NotNull Disposable parent) {
@@ -573,7 +590,7 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
       ourInstances.remove(myParent);
       synchronized (this) {
         myCounts.forEachEntry((pointer, disposeCount) -> {
-          int after = pointer.myNode.incrementUsageCount(-disposeCount + 1);
+          int after = pointer.incrementUsageCount(-disposeCount + 1);
           LOG.assertTrue(after > 0, after);
           pointer.dispose();
           return true;
@@ -590,8 +607,14 @@ public class VirtualFilePointerManagerImpl extends VirtualFilePointerManager imp
     }
     return number;
   }
+
   @TestOnly
   int numberOfListeners() {
     return myPointers.keySet().size();
   }
+
+  @TestOnly
+  int numberOfCachedUrlToIdentity() {
+    return myUrlToIdentity.size();
+  }
 }

diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java
index d273aefaec08dccd7bc37b1423441e20b990db22..afd2a6bb7d3aad6215a2fa1e76aa2b70b4c38237 100644 (file)
--- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java
+++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/VirtualFilePointerTest.java
@@ -17,6 +17,7 @@ package com.intellij.openapi.vfs.impl;
 
 import com.intellij.concurrency.Job;
 import com.intellij.concurrency.JobLauncher;
+import com.intellij.mock.MockVirtualFile;
 import com.intellij.openapi.Disposable;
 import com.intellij.openapi.application.ApplicationManager;
 import com.intellij.openapi.application.ex.PathManagerEx;
@@ -851,4 +852,24 @@ public class VirtualFilePointerTest extends PlatformTestCase {
     VirtualFilePointer pointer = myVirtualFilePointerManager.create(dir2.getUrl() + "/../" + dir1.getName() + "/" + file.getName(), disposable, null);
     assertEquals(file, pointer.getFile());
   }
+
+  public void testAlienVirtualFileSystemPointerRemovedFromUrlToIdentityCacheOnDispose() throws IOException {
+    VirtualFile mockVirtualFile = new MockVirtualFile("test_name", "test_text");
+    Disposable disposable1 = Disposer.newDisposable();
+    final VirtualFilePointer pointer = VirtualFilePointerManager.getInstance().create(mockVirtualFile, disposable1, null);
+
+    assertInstanceOf(pointer, IdentityVirtualFilePointer.class);
+    assertTrue(pointer.isValid());
+
+    VirtualFile virtualFileWithSameUrl = new MockVirtualFile("test_name", "test_text");
+    VirtualFilePointer updatedPointer = VirtualFilePointerManager.getInstance().create(virtualFileWithSameUrl, disposable1, null);
+
+    assertInstanceOf(updatedPointer, IdentityVirtualFilePointer.class);
+
+    assertEquals(1, ((VirtualFilePointerManagerImpl)VirtualFilePointerManager.getInstance()).numberOfCachedUrlToIdentity());
+
+    Disposer.dispose(disposable1);
+
+    assertEquals(0, ((VirtualFilePointerManagerImpl)VirtualFilePointerManager.getInstance()).numberOfCachedUrlToIdentity());
+  }
 }