From 1b600a5f5f289d7381b6da501324adf85f3bb6a2 Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 18 Nov 2019 19:47:51 +0100 Subject: [PATCH] clear references to dom problem holders on a PSI change (needed for IDEA-225622 to avoid leaks) instead of user data with a complicated CachedValue-based uptodateness check, keep holders in a single map GitOrigin-RevId: 381ceeb72ccdea854ae433764b97a1cefc5d1662 --- .../DomElementAnnotationsManagerImpl.java | 64 +++++++++---------- .../util/xml/impl/DefaultDomAnnotator.java | 2 +- .../util/xml/ui/DomUIFactoryImpl.java | 3 +- .../DomElementAnnotationsManager.java | 3 +- .../util/xml/DomHighlightingLiteTest.java | 7 +- .../intellij/util/xml/MockDomFileElement.java | 6 +- 6 files changed, 38 insertions(+), 47 deletions(-) diff --git a/xml/dom-impl/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManagerImpl.java b/xml/dom-impl/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManagerImpl.java index eea3949e0d26..2b27c894137e 100644 --- a/xml/dom-impl/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManagerImpl.java +++ b/xml/dom-impl/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManagerImpl.java @@ -11,19 +11,16 @@ import com.intellij.codeInspection.ex.InspectionToolWrapper; import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.openapi.Disposable; import com.intellij.openapi.project.Project; -import com.intellij.openapi.roots.ProjectRootManager; -import com.intellij.openapi.util.Key; import com.intellij.profile.ProfileChangeAdapter; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; -import com.intellij.psi.util.CachedValue; -import com.intellij.psi.util.CachedValueProvider; -import com.intellij.psi.util.CachedValuesManager; +import com.intellij.psi.impl.source.xml.XmlFileImpl; import com.intellij.psi.util.PsiModificationTracker; import com.intellij.psi.xml.XmlFile; import com.intellij.psi.xml.XmlTag; import com.intellij.util.EventDispatcher; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.messages.MessageBusConnection; import com.intellij.util.xml.DomElement; import com.intellij.util.xml.DomFileElement; import com.intellij.util.xml.DomUtil; @@ -32,12 +29,11 @@ import org.jetbrains.annotations.Nullable; import java.util.Collections; import java.util.List; +import java.util.Map; public class DomElementAnnotationsManagerImpl extends DomElementAnnotationsManager { - public static final Object LOCK = new Object(); + static final Object LOCK = new Object(); - private static final Key DOM_PROBLEM_HOLDER_KEY = Key.create("DomProblemHolder"); - private static final Key> CACHED_VALUE_KEY = Key.create("DomProblemHolderCachedValue"); private final EventDispatcher myDispatcher = EventDispatcher.create(DomHighlightingListener.class); private static final DomElementsProblemsHolder EMPTY_PROBLEMS_HOLDER = new DomElementsProblemsHolder() { @@ -75,11 +71,11 @@ public class DomElementAnnotationsManagerImpl extends DomElementAnnotationsManag } }; - private final Project myProject; + private final Map myHolders = ContainerUtil.createWeakMap(); public DomElementAnnotationsManagerImpl(@NotNull Project project) { - myProject = project; - project.getMessageBus().connect().subscribe(ProfileChangeAdapter.TOPIC, new ProfileChangeAdapter() { + MessageBusConnection connection = project.getMessageBus().connect(); + connection.subscribe(ProfileChangeAdapter.TOPIC, new ProfileChangeAdapter() { @Override public void profileActivated(InspectionProfile oldProfile, @Nullable InspectionProfile profile) { dropAnnotationsCache(); @@ -90,11 +86,14 @@ public class DomElementAnnotationsManagerImpl extends DomElementAnnotationsManag dropAnnotationsCache(); } }); + connection.subscribe(PsiModificationTracker.TOPIC, this::dropAnnotationsCache); } @Override public void dropAnnotationsCache() { - incModificationCount(); + synchronized (LOCK) { + myHolders.clear(); + } } public final List appendProblems(@NotNull DomFileElement element, @NotNull DomElementAnnotationHolder annotationHolder, Class inspectionClass) { @@ -107,40 +106,37 @@ public class DomElementAnnotationsManagerImpl extends DomElementAnnotationsManag return Collections.unmodifiableList(holderImpl); } + @NotNull private DomElementsProblemsHolderImpl _getOrCreateProblemsHolder(final DomFileElement element) { - DomElementsProblemsHolderImpl holder; - final DomElement rootElement = element.getRootElement(); - final XmlTag rootTag = rootElement.getXmlTag(); + XmlTag rootTag = element.getRootElement().getXmlTag(); if (rootTag == null) return new DomElementsProblemsHolderImpl(element); - holder = rootTag.getUserData(DOM_PROBLEM_HOLDER_KEY); - if (isHolderOutdated(element.getFile()) || holder == null) { - holder = new DomElementsProblemsHolderImpl(element); - rootTag.putUserData(DOM_PROBLEM_HOLDER_KEY, holder); - final CachedValue cachedValue = CachedValuesManager.getManager(myProject).createCachedValue( - () -> new CachedValueProvider.Result<>(Boolean.FALSE, element, PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT, - this, ProjectRootManager.getInstance(myProject)), false); - cachedValue.getValue(); - element.getFile().putUserData(CACHED_VALUE_KEY, cachedValue); - } - return holder; + return myHolders.computeIfAbsent(rootTag, __ -> new DomElementsProblemsHolderImpl(element)); } - public static boolean isHolderUpToDate(DomElement element) { + public boolean isHolderUpToDate(DomElement element) { + return !isHolderOutdated(DomUtil.getFile(element)); + } + + public void outdateProblemHolder(DomElement element) { + XmlTag rootTag = getRootTagIfParsed(DomUtil.getFile(element)); synchronized (LOCK) { - return !isHolderOutdated(DomUtil.getFile(element)); + if (rootTag != null) { + myHolders.remove(rootTag); + } } } - public static void outdateProblemHolder(final DomElement element) { + private boolean isHolderOutdated(XmlFile file) { synchronized (LOCK) { - DomUtil.getFile(element).putUserData(CACHED_VALUE_KEY, null); + XmlTag rootTag = getRootTagIfParsed(file); + return rootTag == null || !myHolders.containsKey(rootTag); } } - private static boolean isHolderOutdated(final XmlFile file) { - final CachedValue cachedValue = file.getUserData(CACHED_VALUE_KEY); - return cachedValue == null || !cachedValue.hasUpToDateValue(); + @Nullable + private static XmlTag getRootTagIfParsed(@NotNull XmlFile file) { + return ((XmlFileImpl)file).isContentsLoaded() ? file.getRootTag() : null; } @Override @@ -152,7 +148,7 @@ public class DomElementAnnotationsManagerImpl extends DomElementAnnotationsManag synchronized (LOCK) { final XmlTag tag = fileElement.getRootElement().getXmlTag(); if (tag != null) { - final DomElementsProblemsHolder readyHolder = tag.getUserData(DOM_PROBLEM_HOLDER_KEY); + DomElementsProblemsHolder readyHolder = myHolders.get(tag); if (readyHolder != null) { return readyHolder; } diff --git a/xml/dom-impl/src/com/intellij/util/xml/impl/DefaultDomAnnotator.java b/xml/dom-impl/src/com/intellij/util/xml/impl/DefaultDomAnnotator.java index f8694718530b..421137389890 100644 --- a/xml/dom-impl/src/com/intellij/util/xml/impl/DefaultDomAnnotator.java +++ b/xml/dom-impl/src/com/intellij/util/xml/impl/DefaultDomAnnotator.java @@ -48,7 +48,7 @@ public class DefaultDomAnnotator implements Annotator { public void runInspection(@Nullable final DomElementsInspection inspection, final DomFileElement fileElement, List toFill) { if (inspection == null) return; DomElementAnnotationsManagerImpl annotationsManager = getAnnotationsManager(fileElement); - if (DomElementAnnotationsManagerImpl.isHolderUpToDate(fileElement) && annotationsManager.getProblemHolder(fileElement).isInspectionCompleted(inspection)) return; + if (annotationsManager.isHolderUpToDate(fileElement) && annotationsManager.getProblemHolder(fileElement).isInspectionCompleted(inspection)) return; DomElementAnnotationHolderImpl annotationHolder = new DomElementAnnotationHolderImpl(true, fileElement); inspection.checkFileElement(fileElement, annotationHolder); diff --git a/xml/dom-impl/src/com/intellij/util/xml/ui/DomUIFactoryImpl.java b/xml/dom-impl/src/com/intellij/util/xml/ui/DomUIFactoryImpl.java index 6142c8f5dc82..5dfacc8d7746 100644 --- a/xml/dom-impl/src/com/intellij/util/xml/ui/DomUIFactoryImpl.java +++ b/xml/dom-impl/src/com/intellij/util/xml/ui/DomUIFactoryImpl.java @@ -24,6 +24,7 @@ import com.intellij.util.containers.ClassMap; import com.intellij.util.ui.JBUI; import com.intellij.util.xml.DomElement; import com.intellij.util.xml.DomUtil; +import com.intellij.util.xml.highlighting.DomElementAnnotationsManager; import com.intellij.util.xml.highlighting.DomElementAnnotationsManagerImpl; import com.intellij.util.xml.highlighting.DomElementsErrorPanel; import org.jetbrains.annotations.NotNull; @@ -104,7 +105,7 @@ public class DomUIFactoryImpl extends DomUIFactory { isProcessingChange = true; try { for (final DomElement element : elements) { - DomElementAnnotationsManagerImpl.outdateProblemHolder(element); + ((DomElementAnnotationsManagerImpl)DomElementAnnotationsManager.getInstance(element.getManager().getProject())).outdateProblemHolder(element); } CommittableUtil.updateHighlighting(panel); } diff --git a/xml/dom-openapi/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManager.java b/xml/dom-openapi/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManager.java index 1aa27d4fd855..d16d4f14971c 100644 --- a/xml/dom-openapi/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManager.java +++ b/xml/dom-openapi/src/com/intellij/util/xml/highlighting/DomElementAnnotationsManager.java @@ -21,7 +21,6 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.openapi.Disposable; import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.SimpleModificationTracker; import com.intellij.util.xml.DomElement; import com.intellij.util.xml.DomFileElement; import org.jetbrains.annotations.NotNull; @@ -29,7 +28,7 @@ import org.jetbrains.annotations.NotNull; import java.util.EventListener; import java.util.List; -public abstract class DomElementAnnotationsManager extends SimpleModificationTracker { +public abstract class DomElementAnnotationsManager { public static DomElementAnnotationsManager getInstance(Project project) { return ServiceManager.getService(project, DomElementAnnotationsManager.class); diff --git a/xml/dom-tests/tests/com/intellij/util/xml/DomHighlightingLiteTest.java b/xml/dom-tests/tests/com/intellij/util/xml/DomHighlightingLiteTest.java index 105e3e97157d..31406658726c 100644 --- a/xml/dom-tests/tests/com/intellij/util/xml/DomHighlightingLiteTest.java +++ b/xml/dom-tests/tests/com/intellij/util/xml/DomHighlightingLiteTest.java @@ -157,12 +157,11 @@ public class DomHighlightingLiteTest extends DomTestCase { public void testHolderRecreationAfterChange() { myAnnotationsManager.appendProblems(myElement, createHolder(), MyDomElementsInspection.class); - assertTrue(DomElementAnnotationsManagerImpl.isHolderUpToDate(myElement)); + assertTrue(myAnnotationsManager.isHolderUpToDate(myElement)); final DomElementsProblemsHolder holder = myAnnotationsManager.getProblemHolder(myElement); - myElement.incModificationCount(); - assertFalse(DomElementAnnotationsManagerImpl.isHolderUpToDate(myElement)); - assertSame(holder, myAnnotationsManager.getProblemHolder(myElement)); + getPsiManager().dropPsiCaches(); + assertFalse(myAnnotationsManager.isHolderUpToDate(myElement)); myAnnotationsManager.appendProblems(myElement, createHolder(), MyDomElementsInspection.class); assertNotSame(holder, assertNotEmptyHolder(myAnnotationsManager.getProblemHolder(myElement))); diff --git a/xml/dom-tests/tests/com/intellij/util/xml/MockDomFileElement.java b/xml/dom-tests/tests/com/intellij/util/xml/MockDomFileElement.java index ec16169e73ab..e470aa937e93 100644 --- a/xml/dom-tests/tests/com/intellij/util/xml/MockDomFileElement.java +++ b/xml/dom-tests/tests/com/intellij/util/xml/MockDomFileElement.java @@ -34,7 +34,6 @@ import java.lang.reflect.Type; * @author peter */ public class MockDomFileElement extends UserDataHolderBase implements DomFileElement { - private long myModCount = 0; private DomFileDescription myFileDescription; public void setFileDescription(final DomFileDescription fileDescription) { @@ -230,10 +229,7 @@ public class MockDomFileElement extends UserDataHolderBase implements DomFileEle @Override public long getModificationCount() { - return myModCount; + return 0; } - public void incModificationCount() { - myModCount++; - } }