Platform: race condition in global inspections fixed:

RefManager was passed to RefElement constructor, which in turn could call RefManager.getReference(psiElement), and cause unsynchronized access to RefElementImpl.add().
 + bug fixed in previous logic

Exception:
at java.util.ArrayList.add(ArrayList.java:352)
    at com.intellij.codeInspection.reference.RefEntityImpl.add(RefEntityImpl.java:85)
    at com.intellij.codeInspection.reference.RefFileImpl.<init>(RefFileImpl.java:40)
    at com.intellij.codeInspection.reference.RefManagerImpl$4.compute(RefManagerImpl.java:480)
    at com.intellij.codeInspection.reference.RefManagerImpl$4.compute(RefManagerImpl.java:470)
    at com.intellij.openapi.application.impl.ApplicationImpl.runReadAction(ApplicationImpl.java:893)
    at com.intellij.codeInspection.reference.RefManagerImpl.getReference(RefManagerImpl.java:470)
    at com.intellij.codeInspection.reference.RefManagerImpl.getReference(RefManagerImpl.java:448)
    at com.intellij.codeInspection.ex.GlobalInspectionContextUtil.retrieveRefElement(GlobalInspectionContextUtil.java:35)
    at com.intellij.codeInspection.GlobalInspectionUtil.createProblem(GlobalInspectionUtil.java:67)
    at com.intellij.codeInspection.DefaultHighlightVisitorBasedInspection.checkFile(DefaultHighlightVisitorBasedInspection.java:115)
    at com.intellij.codeInspection.ex.GlobalInspectionContextImpl$6.process(GlobalInspectionContextImpl.java:390)
    at com.intellij.codeInspection.ex.GlobalInspectionContextImpl$6.process(GlobalInspectionContextImpl.java:383)
    at com.intellij.concurrency.ApplierCompleter.execAndForkSubTasks(ApplierCompleter.java:122)
    at com.intellij.concurrency.ApplierCompleter.access$000(ApplierCompleter.java:44)
    at com.intellij.concurrency.ApplierCompleter$1.run(ApplierCompleter.java:85)
    at com.intellij.openapi.application.impl.ApplicationImpl.tryRunReadAction(ApplicationImpl.java:1107)
    at com.intellij.concurrency.ApplierCompleter$2.run(ApplierCompleter.java:94)
    at com.intellij.openapi.progress.impl.ProgressManagerImpl.registerIndicatorAndRun(ProgressManagerImpl.java:282)
    at com.intellij.openapi.progress.impl.ProgressManagerImpl.registerIndicatorAndRun(ProgressManagerImpl.java:279)
    at com.intellij.openapi.progress.impl.ProgressManagerImpl.executeProcessUnderProgress(ProgressManagerImpl.java:232)
    at com.intellij.concurrency.ApplierCompleter.wrapInReadActionAndIndicator(ApplierCompleter.java:106)
This commit is contained in:
Anton Makeev
2015-01-13 13:42:56 +01:00
parent d3f652d978
commit 5c21da5fbc
2 changed files with 100 additions and 79 deletions
@@ -23,10 +23,7 @@ import com.intellij.codeInspection.deadCode.UnusedDeclarationInspectionBase;
import com.intellij.codeInspection.ex.*;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Comparing;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.UserDataCache;
import com.intellij.openapi.util.*;
import com.intellij.psi.*;
import com.intellij.psi.javadoc.PsiDocComment;
import com.intellij.psi.javadoc.PsiDocTag;
@@ -164,21 +161,20 @@ public class RefJavaManagerImpl extends RefJavaManager {
}
@Override
public RefParameter getParameterReference(PsiParameter param, int index) {
public RefParameter getParameterReference(final PsiParameter param, final int index) {
LOG.assertTrue(myRefManager.isValidPointForReference(), "References may become invalid after process is finished");
RefElement ref = myRefManager.getFromRefTable(param);
if (ref == null) {
ref = new RefParameterImpl(param, index, myRefManager);
((RefParameterImpl)ref).initialize();
myRefManager.putToRefTable(param, ref);
}
return (RefParameter)ref;
return myRefManager.getFromRefTableOrCache(param, new NullableFactory<RefParameter>() {
@Nullable
@Override
public RefParameter create() {
RefParameter ref = new RefParameterImpl(param, index, myRefManager);
((RefParameterImpl)ref).initialize();
return ref;
}
});
}
@Override
public void iterate(@NotNull final RefVisitor visitor) {
if (myPackages != null) {
@@ -44,12 +44,14 @@ import com.intellij.openapi.project.Project;
import com.intellij.openapi.project.ProjectUtilCore;
import com.intellij.openapi.util.Computable;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.NullableFactory;
import com.intellij.openapi.util.Segment;
import com.intellij.openapi.vfs.VfsUtilCore;
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.Consumer;
import com.intellij.util.containers.ContainerUtil;
import gnu.trove.THashMap;
import org.jdom.Element;
@@ -340,11 +342,6 @@ public class RefManagerImpl extends RefManager {
return myRefProject;
}
@NotNull
public Map<PsiAnchor, RefElement> getRefTable() {
return myRefTable;
}
@NotNull
public List<RefElement> getSortedElements() {
LOG.assertTrue(myRefTable != null);
@@ -371,7 +368,7 @@ public class RefManagerImpl extends RefManager {
public void removeReference(@NotNull RefElement refElem) {
myLock.writeLock().lock();
try {
final Map<PsiAnchor, RefElement> refTable = getRefTable();
final Map<PsiAnchor, RefElement> refTable = myRefTable;
final PsiElement element = refElem.getElement();
final RefManagerExtension extension = element != null ? getExtension(element.getLanguage()) : null;
if (extension != null) {
@@ -460,47 +457,41 @@ public class RefManagerImpl extends RefManager {
return null;
}
RefElement ref = getFromRefTable(elem);
if (ref != null) return ref;
if (!isValidPointForReference()) {
//LOG.assertTrue(true, "References may become invalid after process is finished");
return null;
}
final RefElementImpl refElement = ApplicationManager.getApplication().runReadAction(new Computable<RefElementImpl>() {
@Override
@Nullable
public RefElementImpl compute() {
final RefManagerExtension extension = getExtension(elem.getLanguage());
if (extension != null) {
final RefElement refElement = extension.createRefElement(elem);
if (refElement != null) return (RefElementImpl)refElement;
return getFromRefTableOrCache(
elem,
new NullableFactory<RefElementImpl>() {
@Override
public RefElementImpl create() {
return ApplicationManager.getApplication().runReadAction(new Computable<RefElementImpl>() {
@Override
@Nullable
public RefElementImpl compute() {
final RefManagerExtension extension = getExtension(elem.getLanguage());
if (extension != null) {
final RefElement refElement = extension.createRefElement(elem);
if (refElement != null) return (RefElementImpl)refElement;
}
if (elem instanceof PsiFile) {
return new RefFileImpl((PsiFile)elem, RefManagerImpl.this);
}
if (elem instanceof PsiDirectory) {
return new RefDirectoryImpl((PsiDirectory)elem, RefManagerImpl.this);
}
return null;
}
});
}
if (elem instanceof PsiFile) {
return new RefFileImpl((PsiFile)elem, RefManagerImpl.this);
},
new Consumer<RefElementImpl>() {
@Override
public void consume(RefElementImpl element) {
element.initialize();
for (RefManagerExtension each : myExtensions.values()) {
each.onEntityInitialized(element, elem);
}
fireNodeInitialized(element);
}
if (elem instanceof PsiDirectory) {
return new RefDirectoryImpl((PsiDirectory)elem, RefManagerImpl.this);
}
return null;
}
});
if (refElement == null) return null;
putToRefTable(elem, refElement);
ApplicationManager.getApplication().runReadAction(new Runnable() {
@Override
public void run() {
refElement.initialize();
for (RefManagerExtension extension : myExtensions.values()) {
extension.onEntityInitialized(refElement, elem);
}
fireNodeInitialized(refElement);
}
});
return refElement;
});
}
private RefManagerExtension getExtension(final Language language) {
@@ -509,8 +500,7 @@ public class RefManagerImpl extends RefManager {
@Nullable
@Override
public
RefEntity getReference(final String type, final String fqName) {
public RefEntity getReference(final String type, final String fqName) {
for (RefManagerExtension extension : myExtensions.values()) {
final RefEntity refEntity = extension.getReference(type, fqName);
if (refEntity != null) return refEntity;
@@ -535,38 +525,73 @@ public class RefManagerImpl extends RefManager {
return null;
}
protected RefElement getFromRefTable(final PsiElement element) {
@Nullable
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) {
T result;
myLock.readLock().lock();
try {
return getRefTable().get(ApplicationManager.getApplication().runReadAction(
new Computable<PsiAnchor>() {
@Override
public PsiAnchor compute() {
return PsiAnchor.create(element);
}
//noinspection unchecked
result = (T)myRefTable.get(ApplicationManager.getApplication().runReadAction(
new Computable<PsiAnchor>() {
@Override
public PsiAnchor compute() {
return PsiAnchor.create(element);
}
}
));
}
finally {
myLock.readLock().unlock();
}
}
protected void putToRefTable(final PsiElement element, final RefElement ref) {
if (result != null) return result;
if (!isValidPointForReference()) {
//LOG.assertTrue(true, "References may become invalid after process is finished");
return null;
}
myLock.writeLock().lock();
try {
getRefTable().put(ApplicationManager.getApplication().runReadAction(
new Computable<PsiAnchor>() {
@Override
public PsiAnchor compute() {
return PsiAnchor.create(element);
}
//noinspection unchecked
result = (T)myRefTable.get(ApplicationManager.getApplication().runReadAction(
new Computable<PsiAnchor>() {
@Override
public PsiAnchor compute() {
return PsiAnchor.create(element);
}
), ref);
}
));
if (result != null) return result;
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);
}
finally {
myLock.writeLock().unlock();
}
if (whenCached != null) whenCached.consume(result);
return result;
}
@Override