diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/CapturingCleanerInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/CapturingCleanerInspection.java index d94128b311e5..87510e9f178e 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/CapturingCleanerInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/CapturingCleanerInspection.java @@ -92,17 +92,27 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT if (!lambda.getParameterList().isEmpty()) return null; PsiElement lambdaBody = lambda.getBody(); if (lambdaBody == null) return null; - return getLambdaElementCapturingThis(lambdaBody, trackedClass).orElse(null); + return getLambdaOrInnerClassElementCapturingThis(lambdaBody, trackedClass); } if (runnableExpr instanceof PsiNewExpression) { PsiNewExpression newExpression = (PsiNewExpression)runnableExpr; - if (newExpression.getAnonymousClass() != null) return newExpression; + if (newExpression.getAnonymousClass() != null) { + if (PsiUtil.isLanguageLevel18OrHigher(trackedClass)) { + PsiElement elementCapturingThis = getLambdaOrInnerClassElementCapturingThis(newExpression, trackedClass); + if (elementCapturingThis != null) { + return elementCapturingThis; + } + } else { + return newExpression; + } + } PsiJavaCodeReferenceElement classReference = newExpression.getClassReference(); if (classReference == null) return null; PsiClass aClass = tryCast(classReference.resolve(), PsiClass.class); if (aClass == null) return null; if (aClass.getContainingClass() != trackedClass) return null; if (aClass.hasModifierProperty(PsiModifier.STATIC)) return null; + if (PsiUtil.isLanguageLevel18OrHigher(trackedClass) && getLambdaOrInnerClassElementCapturingThis(newExpression, trackedClass) == null) return null; return classReference; } return null; @@ -110,16 +120,19 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT }; } - private static Optional getLambdaElementCapturingThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) { + @Contract(pure = true) + private static @Nullable PsiElement getLambdaOrInnerClassElementCapturingThis(@NotNull PsiElement lambdaBody, @NotNull PsiClass containingClass) { return StreamEx.ofTree(lambdaBody, el -> StreamEx.of(el.getChildren())) - .findAny(element -> isThisCapturingElement(containingClass, element)); + .findAny(element -> isThisCapturingElement(containingClass, element)) + .orElse(null); } + @Contract(pure = true) private static boolean isThisCapturingElement(@NotNull PsiClass containingClass, PsiElement element) { if (element instanceof PsiThisExpression) { return PsiUtil.resolveClassInType(((PsiThisExpression)element).getType()) == containingClass; } - else if (element instanceof PsiReferenceExpression) { + if (element instanceof PsiReferenceExpression) { PsiReferenceExpression qualifierReference = tryCast(((PsiReferenceExpression)element).getQualifierExpression(), PsiReferenceExpression.class); if (qualifierReference != null) return false; @@ -129,7 +142,7 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT return false; } - @Contract("_, null -> false") + @Contract(value = "_, null -> false", pure = true) private static boolean memberBringsThisRef(@NotNull PsiClass containingClass, PsiMember member) { if (member == null) return false; PsiClass memberContainingClass = member.getContainingClass(); @@ -141,7 +154,7 @@ public class CapturingCleanerInspection extends AbstractBaseJavaLocalInspectionT return !member.hasModifierProperty(PsiModifier.STATIC); } - @Contract("_, null -> false") + @Contract(value = "_, null -> false", pure = true) private static boolean isInnerClassOf(@Nullable PsiClass inner, @Nullable PsiClass outer) { if (inner == null || inner.hasModifierProperty(PsiModifier.STATIC)) return false; return PsiTreeUtil.isAncestor(outer, inner, false); diff --git a/java/java-impl/src/inspectionDescriptions/CapturingCleaner.html b/java/java-impl/src/inspectionDescriptions/CapturingCleaner.html index 3035135d402d..b28013becba7 100644 --- a/java/java-impl/src/inspectionDescriptions/CapturingCleaner.html +++ b/java/java-impl/src/inspectionDescriptions/CapturingCleaner.html @@ -1,10 +1,11 @@ -Reports Runnable passed to a Cleaner.register() capturing reference that leads to a memory leak. +Reports Runnable passed to a Cleaner.register() capturing reference being registered. +If the reference is captured, it will never be phantom reachable and the cleaning action will never be invoked.

Possible sources of this problem:

diff --git a/java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java b/java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java index a9a7d67d73ac..2885aab0d5e5 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/PsiUtil.java @@ -1052,6 +1052,10 @@ public final class PsiUtil extends PsiUtilCore { return getLanguageLevel(element).isAtLeast(LanguageLevel.JDK_17); } + public static boolean isLanguageLevel18OrHigher(@NotNull PsiElement element) { + return getLanguageLevel(element).isAtLeast(LanguageLevel.JDK_18); + } + @NotNull public static LanguageLevel getLanguageLevel(@NotNull PsiElement element) { if (element instanceof PsiDirectory) { diff --git a/java/java-tests/testData/inspection/cleanerCapturingThis/18/CapturingCleaner.java b/java/java-tests/testData/inspection/cleanerCapturingThis/18/CapturingCleaner.java new file mode 100644 index 000000000000..57cc1617ef75 --- /dev/null +++ b/java/java-tests/testData/inspection/cleanerCapturingThis/18/CapturingCleaner.java @@ -0,0 +1,187 @@ +import java.lang.ref.Cleaner; + +class Anonymous { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, new Runnable() { + @Override + public void run() { + System.out.println("adsad"); + } + }); +} + +class Inner { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, new MyRunnable()); + + private class MyRunnable implements Runnable { + @Override + public void run() { + System.out.println("adsad"); + } + } +} + +class InstanceMethodReference { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, this::run); + + private void run() { + System.out.println("adsad"); + free(fileDescriptor); + } +} + +class LambdaExprBodyInstanceMethod { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> run()); + + private void run() { + System.out.println("adsad"); + free(fileDescriptor); + } +} + +class LambdaInstanceField { + int fileDescriptor; + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + System.out.println("adsad"); + fileDescriptor = 0; + }); +} + +class LambdaInstanceMethod { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + System.out.println("adsad"); + free(fileDescriptor); + }); +} + +class Base { + int fileDescriptor; +} + +class LambdaInstanceSuperField extends Base { + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + System.out.println("adsad"); + fileDescriptor = 0; + }); +} + +class LambdaThis { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, () -> { + LambdaThis o = this; + }); +} + +class StaticMethodReference { + int fileDescriptor; + + static void free(int descriptor) {} + + Cleaner.Cleanable cleanable = Cleaner.create().register(this, StaticMethodReference::run); + + private static void run() { + System.out.println("adsad"); + } +} + +class ResourceHolder { + int resource; + public void free(){} + public void free(int resource){} +} + +class SafeInstanceMethodReference { + Cleaner cleaner = Cleaner.create(); + + SafeInstanceMethodReference() { + ResourceHolder resource = new ResourceHolder(); + cleaner.register(this, resource::free); + } +} + + +class FieldMethodReference { + Cleaner cleaner = Cleaner.create(); + ResourceHolder resource = new ResourceHolder(); + + FieldMethodReference() { + cleaner.register(this, resource::free); + } +} + +// Reference as tracking target + +class StaticMethodFactory { + static Cleaner cleaner = Cleaner.create(); + + static ResourceHolder create() { + ResourceHolder holder = new ResourceHolder(); + cleaner.register(holder, holder::free); + return holder; + } +} + +class ConstructorDelegatesToStaticMethod { + int resource; + static Cleaner cleaner = Cleaner.create(); + + ConstructorDelegatesToStaticMethod(int resource) { + this.resource = resource; + register(this); + } + + static void register(ConstructorDelegatesToStaticMethod holder) { + cleaner.register(holder, () -> free(holder.resource)); + } + + static void free(int resource){} +} + + +class InnerAccesInstanceOuterMembers { + int resource; + + class Inner { + Cleaner cleaner = Cleaner.create(); + + public Inner() { + cleaner.register(this, () -> resource = -1); + } + } +} + +class LambdaUsingAnotherInstanceMember { + int fileDescriptor; + + static Cleaner cleaner = Cleaner.create(); + + void register() { + LambdaUsingAnotherInstanceMember another = new LambdaUsingAnotherInstanceMember(); + cleaner.register(this, () -> { + another.fileDescriptor = 12; + }); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleaner.java b/java/java-tests/testData/inspection/cleanerCapturingThis/before18/CapturingCleaner.java similarity index 100% rename from java/java-tests/testData/inspection/cleanerCapturingThis/CapturingCleaner.java rename to java/java-tests/testData/inspection/cleanerCapturingThis/before18/CapturingCleaner.java diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleaner18InspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleaner18InspectionTest.java new file mode 100644 index 000000000000..5cf7629c34d3 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleaner18InspectionTest.java @@ -0,0 +1,37 @@ +// Copyright 2000-2022 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. + +package com.intellij.java.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInspection.CapturingCleanerInspection; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; + +public class CapturingCleaner18InspectionTest extends LightJavaCodeInsightFixtureTestCase { + + public void testCapturingCleaner() {doTest();} + + @Override + protected void setUp() throws Exception { + super.setUp(); + + myFixture.enableInspections(new CapturingCleanerInspection()); + } + + private void doTest() { + myFixture.testHighlighting(true, false, false, getTestName(false) + ".java"); + } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_18; + } + + @NotNull + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection/cleanerCapturingThis/18"; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java index c907f95a729a..5703433ebbf8 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/CapturingCleanerInspectionTest.java @@ -32,6 +32,6 @@ public class CapturingCleanerInspectionTest extends LightJavaCodeInsightFixtureT @NotNull @Override protected String getTestDataPath() { - return JavaTestUtil.getJavaTestDataPath() + "/inspection/cleanerCapturingThis"; + return JavaTestUtil.getJavaTestDataPath() + "/inspection/cleanerCapturingThis/before18"; } } \ No newline at end of file