more thread safety

platform/analysis-impl/src/com/intellij/codeInspection/reference/RefEntityImpl.java

@@ -25,7 +25,6 @@
 package com.intellij.codeInspection.reference;
 
 import com.intellij.openapi.application.ApplicationManager;
-import com.intellij.openapi.util.Computable;
 import com.intellij.openapi.util.Key;
 import com.intellij.util.BitUtil;
 import gnu.trove.THashMap;
@@ -37,18 +36,16 @@ import java.util.List;
 import java.util.Map;
 
 abstract class RefEntityImpl implements RefEntity {
-  private RefEntityImpl myOwner;         // guarded by myManager.myLock
-  protected List<RefEntity> myChildren;  // guarded by myManager.myLock
+  private volatile RefEntityImpl myOwner;
+  protected List<RefEntity> myChildren;  // guarded by this
   private final String myName;
-  private Map<Key, Object> myUserMap;
+  private Map<Key, Object> myUserMap;    // guarded by this
   protected long myFlags;
   protected final RefManagerImpl myManager;
 
   RefEntityImpl(@NotNull String name, @NotNull RefManager manager) {
     myManager = (RefManagerImpl)manager;
     myName = name;
-    myOwner = null;
-    myChildren = null;
   }
 
   @NotNull
@@ -64,58 +61,33 @@ abstract class RefEntityImpl implements RefEntity {
   }
 
   @Override
-  public List<RefEntity> getChildren() {
-    return myManager.doRead(new Computable<List<RefEntity>>() {
-      @Override
-      public List<RefEntity> compute() {
-        return myChildren;
-      }
-    });
+  public synchronized List<RefEntity> getChildren() {
+    return myChildren;
   }
 
   @Override
   public RefEntity getOwner() {
-    return myManager.doRead(new Computable<RefEntity>() {
-      @Override
-      public RefEntity compute() {
-        return myOwner;
-      }
-    });
+    return myOwner;
   }
 
-  protected void setOwner(final RefEntityImpl owner) {
-    myManager.doWrite(new Runnable() {
-      @Override
-      public void run() {
-        myOwner = owner;
-      }
-    });
+  protected void setOwner(@Nullable final RefEntityImpl owner) {
+    myOwner = owner;
   }
 
-  public void add(@NotNull final RefEntity child) {
-    myManager.doWrite(new Runnable() {
-      @Override
-      public void run() {
-        if (myChildren == null) {
-          myChildren = new ArrayList<RefEntity>(1);
-        }
+  public synchronized void add(@NotNull final RefEntity child) {
+    if (myChildren == null) {
+      myChildren = new ArrayList<RefEntity>(1);
+    }
 
-        myChildren.add(child);
-        ((RefEntityImpl)child).setOwner(RefEntityImpl.this);
-      }
-    });
+    myChildren.add(child);
+    ((RefEntityImpl)child).setOwner(this);
   }
 
-  protected void removeChild(@NotNull final RefEntity child) {
-    myManager.doWrite(new Runnable() {
-      @Override
-      public void run() {
-        if (myChildren != null) {
-          myChildren.remove(child);
-          ((RefEntityImpl)child).setOwner(null);
-        }
-      }
-    });
+  protected synchronized void removeChild(@NotNull final RefEntity child) {
+    if (myChildren != null) {
+      myChildren.remove(child);
+      ((RefEntityImpl)child).setOwner(null);
+    }
   }
 
   public String toString() {

platform/analysis-impl/src/com/intellij/codeInspection/reference/RefManagerImpl.java

@@ -51,6 +51,7 @@ import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileManager;
 import com.intellij.psi.*;
 import com.intellij.psi.impl.light.LightElement;
+import com.intellij.util.ConcurrencyUtil;
 import com.intellij.util.Consumer;
 import com.intellij.util.containers.ContainerUtil;
 import gnu.trove.THashMap;
@@ -59,8 +60,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.*;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.concurrent.ConcurrentMap;
 
 public class RefManagerImpl extends RefManager {
   private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.reference.RefManager");
@@ -71,9 +71,9 @@ public class RefManagerImpl extends RefManager {
   private final Project myProject;
   private AnalysisScope myScope;
   private RefProject myRefProject;
-  private Map<PsiAnchor, RefElement> myRefTable = new THashMap<PsiAnchor, RefElement>();
+  private final ConcurrentMap<PsiAnchor, RefElement> myRefTable = ContainerUtil.newConcurrentMap();
 
-  private Map<Module, RefModule> myModules;
+  private final ConcurrentMap<Module, RefModule> myModules = ContainerUtil.newConcurrentMap();
   private final ProjectIterator myProjectIterator = new ProjectIterator();
   private volatile boolean myDeclarationsFound;
   private final PsiManager myPsiManager;
@@ -83,11 +83,9 @@ public class RefManagerImpl extends RefManager {
   private final List<RefGraphAnnotator> myGraphAnnotators = new ArrayList<RefGraphAnnotator>();
   private GlobalInspectionContext myContext;
 
-  private final Map<Key, RefManagerExtension> myExtensions = new HashMap<Key, RefManagerExtension>();
+  private final Map<Key, RefManagerExtension> myExtensions = new THashMap<Key, RefManagerExtension>();
   private final Map<Language, RefManagerExtension> myLanguageExtensions = new HashMap<Language, RefManagerExtension>();
 
-  private final ReadWriteLock myLock = new ReentrantReadWriteLock();
-
   public RefManagerImpl(@NotNull Project project, @Nullable AnalysisScope scope, @NotNull GlobalInspectionContext context) {
     myProject = project;
     myScope = scope;
@@ -115,30 +113,24 @@ public class RefManagerImpl extends RefManager {
 
   @Override
   public void iterate(@NotNull RefVisitor visitor) {
-    myLock.readLock().lock();
-    try {
-      for (RefElement refElement : getSortedElements()) {
-        refElement.accept(visitor);
-      }
-      if (myModules != null) {
-        for (RefModule refModule : myModules.values()) {
-          refModule.accept(visitor);
-        }
-      }
-      for (RefManagerExtension extension : myExtensions.values()) {
-        extension.iterate(visitor);
+    for (RefElement refElement : getSortedElements()) {
+      refElement.accept(visitor);
+    }
+    if (myModules != null) {
+      for (RefModule refModule : myModules.values()) {
+        refModule.accept(visitor);
       }
     }
-    finally {
-      myLock.readLock().unlock();
+    for (RefManagerExtension extension : myExtensions.values()) {
+      extension.iterate(visitor);
     }
   }
 
   public void cleanup() {
     myScope = null;
     myRefProject = null;
-    myRefTable = null;
-    myModules = null;
+    myRefTable.clear();
+    myModules.clear();
     myContext = null;
 
     myGraphAnnotators.clear();
@@ -184,7 +176,7 @@ public class RefManagerImpl extends RefManager {
     }
   }
 
-  public void registerGraphAnnotator(RefGraphAnnotator annotator) {
+  public void registerGraphAnnotator(@NotNull RefGraphAnnotator annotator) {
     myGraphAnnotators.add(annotator);
   }
 
@@ -343,7 +335,6 @@ public class RefManagerImpl extends RefManager {
 
   @NotNull
   public List<RefElement> getSortedElements() {
-    LOG.assertTrue(myRefTable != null);
     List<RefElement> answer = new ArrayList<RefElement>(myRefTable.values());
     ContainerUtil.quickSort(answer, new Comparator<RefElement>() {
       @Override
@@ -364,36 +355,36 @@ public class RefManagerImpl extends RefManager {
     return myPsiManager;
   }
 
-  public void removeReference(@NotNull RefElement refElem) {
-    myLock.writeLock().lock();
-    try {
-      final Map<PsiAnchor, RefElement> refTable = myRefTable;
-      final PsiElement element = refElem.getElement();
-      final RefManagerExtension extension = element != null ? getExtension(element.getLanguage()) : null;
-      if (extension != null) {
-        extension.removeReference(refElem);
+  void removeReference(@NotNull RefElement refElem) {
+    final PsiElement element = refElem.getElement();
+    final RefManagerExtension extension = element != null ? getExtension(element.getLanguage()) : null;
+    if (extension != null) {
+      extension.removeReference(refElem);
+    }
+
+    if (element != null && myRefTable.remove(createAnchor(element)) != null) return;
+
+    //PsiElement may have been invalidated and new one returned by getElement() is different so we need to do this stuff.
+    for (Map.Entry<PsiAnchor, RefElement> entry : myRefTable.entrySet()) {
+      RefElement value = entry.getValue();
+      if (value == refElem) {
+        PsiAnchor anchor = entry.getKey();
+        myRefTable.remove(anchor, refElem);
+        return;
       }
+    }
+  }
 
-      if (element != null && refTable.remove(ApplicationManager.getApplication().runReadAction(
-          new Computable<PsiAnchor>() {
-            @Override
-            public PsiAnchor compute() {
-              return PsiAnchor.create(element);
-            }
-          }
-      )) != null) return;
-
-      //PsiElement may have been invalidated and new one returned by getElement() is different so we need to do this stuff.
-      for (PsiAnchor psiElement : refTable.keySet()) {
-        if (refTable.get(psiElement) == refElem) {
-          refTable.remove(psiElement);
-          return;
+  @NotNull
+  private static PsiAnchor createAnchor(@NotNull final PsiElement element) {
+    return ApplicationManager.getApplication().runReadAction(
+      new Computable<PsiAnchor>() {
+        @Override
+        public PsiAnchor compute() {
+          return PsiAnchor.create(element);
         }
       }
-    }
-    finally {
-      myLock.writeLock().unlock();
-    }
+    );
   }
 
   public void initializeAnnotators() {
@@ -525,32 +516,18 @@ public class RefManagerImpl extends RefManager {
   }
 
   @Nullable
-  protected <T extends RefElement> T getFromRefTableOrCache(final PsiElement element, 
-                                                            @NotNull NullableFactory<T> factory) {
+  protected <T extends RefElement> T getFromRefTableOrCache(final PsiElement element, @NotNull NullableFactory<T> factory) {
     return getFromRefTableOrCache(element, factory, null); 
   }
   
   @Nullable
-  protected <T extends RefElement> T getFromRefTableOrCache(final PsiElement element, 
-                                                            @NotNull NullableFactory<T> factory,
-                                                            @Nullable Consumer<T> whenCached) {
+  private <T extends RefElement> T getFromRefTableOrCache(final PsiElement element,
+                                                          @NotNull NullableFactory<T> factory,
+                                                          @Nullable Consumer<T> whenCached) {
 
-    myLock.readLock().lock();
-    T result;
-    try {
-      //noinspection unchecked
-      result = (T)myRefTable.get(ApplicationManager.getApplication().runReadAction(
-        new Computable<PsiAnchor>() {
-          @Override
-          public PsiAnchor compute() {
-            return PsiAnchor.create(element);
-          }
-        }
-      ));
-    }
-    finally {
-      myLock.readLock().unlock();
-    }
+    PsiAnchor psiAnchor = createAnchor(element);
+    //noinspection unchecked
+    T result = (T)myRefTable.get(psiAnchor);
 
     if (result != null) return result;
 
@@ -559,36 +536,20 @@ public class RefManagerImpl extends RefManager {
       return null;
     }
 
-    myLock.writeLock().lock();
-    try {
-      //noinspection unchecked
-      result = (T)myRefTable.get(ApplicationManager.getApplication().runReadAction(
-        new Computable<PsiAnchor>() {
-          @Override
-          public PsiAnchor compute() {
-            return PsiAnchor.create(element);
-          }
-        }
-      ));
-      if (result != null) return result;
+    //noinspection unchecked
+    result = (T)myRefTable.get(psiAnchor);
+    if (result != null) return result;
 
-      result = factory.create();
-      if (result == null) return null;
+    result = factory.create();
+    if (result == null) return null;
 
-      myRefTable.put(ApplicationManager.getApplication().runReadAction(
-        new Computable<PsiAnchor>() {
-          @Override
-          public PsiAnchor compute() {
-            return PsiAnchor.create(element);
-          }
-        }
-      ), result);
+    RefElement prev = myRefTable.putIfAbsent(psiAnchor, result);
+    if (prev == null) {
+      if (whenCached != null) whenCached.consume(result);
     }
-    finally {
-      myLock.writeLock().unlock();
+    else {
+      result = (T)prev;
     }
-    
-    if (whenCached != null) whenCached.consume(result);
 
     return result;
   }
@@ -598,51 +559,11 @@ public class RefManagerImpl extends RefManager {
     if (module == null) {
       return null;
     }
-    myLock.readLock().lock();
-    try {
-      if (myModules != null) {
-        RefModule refModule = myModules.get(module);
-        if (refModule != null) {
-          return refModule;
-        }
-      }
-    }
-    finally {
-      myLock.readLock().unlock();
-    }
-
-    myLock.writeLock().lock();
-    try {
-      if (myModules == null) {
-        myModules = new THashMap<Module, RefModule>();
-      }
-      final RefModule refModule = new RefModuleImpl(module, this);
-      myModules.put(module, refModule);
-      return refModule;
-    }
-    finally {
-      myLock.writeLock().unlock();
-    }
-  }
-
-  public void doWrite(@NotNull Runnable runnable) {
-    myLock.writeLock().lock();
-    try {
-      runnable.run();
-    }
-    finally {
-      myLock.writeLock().unlock();
-    }
-  }
-
-  public <T> T doRead(@NotNull Computable<T> runnable) {
-    myLock.readLock().lock();
-    try {
-      return runnable.compute();
-    }
-    finally {
-      myLock.readLock().unlock();
+    RefModule refModule = myModules.get(module);
+    if (refModule == null) {
+      refModule = ConcurrencyUtil.cacheOrGet(myModules, module, new RefModuleImpl(module, this));
     }
+    return refModule;
   }
 
   @Override

platform/analysis-impl/src/com/intellij/codeInspection/reference/RefModuleImpl.java

@@ -39,32 +39,22 @@ class RefModuleImpl extends RefEntityImpl implements RefModule {
   }
 
   @Override
-  public void add(@NotNull final RefEntity child) {
-    myManager.doWrite(new Runnable() {
-      @Override
-      public void run() {
-        if (myChildren == null) {
-          myChildren = new ArrayList<RefEntity>();
-        }
-        myChildren.add(child);
+  public synchronized void add(@NotNull final RefEntity child) {
+    if (myChildren == null) {
+      myChildren = new ArrayList<RefEntity>();
+    }
+    myChildren.add(child);
 
-        if (child.getOwner() == null) {
-          ((RefEntityImpl)child).setOwner(RefModuleImpl.this);
-        }
-      }
-    });
+    if (child.getOwner() == null) {
+      ((RefEntityImpl)child).setOwner(this);
+    }
   }
 
   @Override
-  protected void removeChild(@NotNull final RefEntity child) {
-    myManager.doWrite(new Runnable() {
-      @Override
-      public void run() {
-        if (myChildren != null) {
-          myChildren.remove(child);
-        }
-      }
-    });
+  protected synchronized void removeChild(@NotNull final RefEntity child) {
+    if (myChildren != null) {
+      myChildren.remove(child);
+    }
   }
 
   @Override