From ee7ece6b71b6452f2e1a6c3b57762b3e117bfc65 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Mon, 27 Apr 2015 15:06:38 +0300 Subject: [PATCH] more thread safety --- .../reference/RefEntityImpl.java | 66 ++---- .../reference/RefManagerImpl.java | 205 ++++++------------ .../reference/RefModuleImpl.java | 34 +-- 3 files changed, 94 insertions(+), 211 deletions(-) diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefEntityImpl.java b/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefEntityImpl.java index fc631f74c246..493298dc4164 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefEntityImpl.java +++ b/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 myChildren; // guarded by myManager.myLock + private volatile RefEntityImpl myOwner; + protected List myChildren; // guarded by this private final String myName; - private Map myUserMap; + private Map 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 getChildren() { - return myManager.doRead(new Computable>() { - @Override - public List compute() { - return myChildren; - } - }); + public synchronized List getChildren() { + return myChildren; } @Override public RefEntity getOwner() { - return myManager.doRead(new Computable() { - @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(1); - } + public synchronized void add(@NotNull final RefEntity child) { + if (myChildren == null) { + myChildren = new ArrayList(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() { diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefManagerImpl.java b/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefManagerImpl.java index d11b39cd7504..fec2b7c08d79 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefManagerImpl.java +++ b/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 myRefTable = new THashMap(); + private final ConcurrentMap myRefTable = ContainerUtil.newConcurrentMap(); - private Map myModules; + private final ConcurrentMap 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 myGraphAnnotators = new ArrayList(); private GlobalInspectionContext myContext; - private final Map myExtensions = new HashMap(); + private final Map myExtensions = new THashMap(); private final Map myLanguageExtensions = new HashMap(); - 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 getSortedElements() { - LOG.assertTrue(myRefTable != null); List answer = new ArrayList(myRefTable.values()); ContainerUtil.quickSort(answer, new Comparator() { @Override @@ -364,36 +355,36 @@ public class RefManagerImpl extends RefManager { return myPsiManager; } - public void removeReference(@NotNull RefElement refElem) { - myLock.writeLock().lock(); - try { - final Map 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 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() { - @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() { + @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 getFromRefTableOrCache(final PsiElement element, - @NotNull NullableFactory factory) { + protected T getFromRefTableOrCache(final PsiElement element, @NotNull NullableFactory factory) { return getFromRefTableOrCache(element, factory, null); } @Nullable - protected T getFromRefTableOrCache(final PsiElement element, - @NotNull NullableFactory factory, - @Nullable Consumer whenCached) { + private T getFromRefTableOrCache(final PsiElement element, + @NotNull NullableFactory factory, + @Nullable Consumer whenCached) { - myLock.readLock().lock(); - T result; - try { - //noinspection unchecked - result = (T)myRefTable.get(ApplicationManager.getApplication().runReadAction( - new Computable() { - @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() { - @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() { - @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(); - } - 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 doRead(@NotNull Computable 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 diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefModuleImpl.java b/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefModuleImpl.java index 10f0a501dfb3..39f3db32d548 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/reference/RefModuleImpl.java +++ b/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(); - } - myChildren.add(child); + public synchronized void add(@NotNull final RefEntity child) { + if (myChildren == null) { + myChildren = new ArrayList(); + } + 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