Skip to content

Commit

Permalink
add support for custom close method (@Owning-annotated method receiver)
Browse files Browse the repository at this point in the history
  • Loading branch information
stephan-herrmann committed Jan 11, 2024
1 parent 8d231f1 commit 384b83c
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
* enclosing object. In order to allow for precise analysis in this situation, it is recommended that the enclosing
* class of such a field implements {@link AutoCloseable}. In that case, it will be the responsibility of the class's
* {@code close()} implementation to also close all resources stored in {@code @Owning} fields.
* <dt>Method receiver (via {@link ElementType#TYPE_USE})</dt>
* <dd>Annotating a method receiver (explicit {@code this} parameter) as owning signals that the method is responsible
* for closing all contained resources. The method will be treated as a "custom close method".<br/>
* The annotation is not evaluated in any other TYPE_USE locations.
* </dl>
* <p><strong>Responsibility</strong> to close a resource may <strong>initially arise</strong> from these situations:</p>
* <ul>
Expand All @@ -52,26 +56,27 @@
* <li>Receiving a result from a call to method that is marked as {@code @Owning} and returns {@link AutoCloseable} (or a subtype).
* <ul><li>If the {@code @Owning} annotation is absent in this situation, the receiving method is <em>potentially responsible</em>.</li></ul>
* </li>
* <li>Within the {@code close()} method of a class implementing {@link AutoCloseable}, each resource field annotations as {@code @Owning}
* must be closed.</li>
* <li>Within the {@code close()} method of a class implementing {@link AutoCloseable}, and within each "custom close method",
* each resource field annotated as {@code @Owning} must be closed.</li>
* </ul>
* <p><strong>Responsibility</strong> to close a resource may be <strong>fulfilled</strong> by ensuring any of these measures on every possible path:</p>
* <ul>
* <li>Invoking {@link AutoCloseable#close()}.
* <li>Invoking {@link AutoCloseable#close()} or a "custom close method".</li>
* <li>Passing the resource into a method where the receiving parameter has type {@link AutoCloseable} (or subtype)
* and is marked as {@code @Owning}.</li>
* <li>Returning the resource to the caller, provided that the current method is marked as {@code @Owning} and has a declared return type
* of {@link AutoCloseable} (or a subtype).</li>
* <li>Assigning the resource to a field annotated as {@code @Owning}.
* <li>Within the {@code close()} method of a class implementing {@link AutoCloseable} (see above) closing the resource held by a field
* tagged as {@code @Owning} can happen either directly, or for inherited fields by invoking {@code super.close()}.
* <li>Within the {@code close()} method of a class implementing {@link AutoCloseable} and within a "custom close method"
* closing the resource held by a field tagged as {@code @Owning} can happen either directly, or for inherited fields
* by invoking {@code super.close()}, or the super version of a "custom close method".
* </ul>
*
* @since 2.3
*/
@Retention(RetentionPolicy.CLASS)
@Documented
@Target({ ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD })
@Target({ ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD, ElementType.TYPE_USE })
public @interface Owning {
// no details
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2023 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -670,6 +670,15 @@ public void resolveReceiver() {
if (this.receiver.type.hasNullTypeAnnotation(AnnotationPosition.ANY)) {
this.scope.problemReporter().nullAnnotationUnsupportedLocation(this.receiver.type);
}
if (this.scope.compilerOptions().isAnnotationBasedResourceAnalysisEnabled && this.receiver.type.resolvedType != null) {
for (AnnotationBinding annotationBinding : this.receiver.type.resolvedType.getTypeAnnotations()) {
ReferenceBinding annotationType = annotationBinding.getAnnotationType();
if (annotationType != null && annotationType.hasTypeBit(TypeIds.BitOwningAnnotation)) {
this.binding.extendedTagBits = ExtendedTagBits.IsClosingMethod;
break;
}
}
}
}
public void resolveJavadoc() {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2023 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -160,7 +160,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
if (analyseResources) {
if (nonStatic) {
// closeable.close()
if (CharOperation.equals(TypeConstants.CLOSE, this.selector)) {
if (this.binding.isClosingMethod()) {
recordCallingClose(currentScope, flowContext, flowInfo, this.receiver);
}
} else if (this.arguments != null && this.arguments.length > 0 && FakedTrackingVariable.isAnyCloseable(this.arguments[0].resolvedType)) {
Expand Down Expand Up @@ -340,20 +340,13 @@ private void yieldQualifiedCheck(BlockScope currentScope) {
currentScope.problemReporter().switchExpressionsYieldUnqualifiedMethodError(this);
}
private void recordCallingClose(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo, Expression closeTarget) {
FakedTrackingVariable trackingVariable = FakedTrackingVariable.getCloseTrackingVariable(closeTarget, flowInfo, flowContext,
currentScope.compilerOptions().isAnnotationBasedResourceAnalysisEnabled);
if (trackingVariable != null) { // null happens if target is not a variable or not an AutoCloseable
if (trackingVariable.methodScope == null || trackingVariable.methodScope == currentScope.methodScope()) {
trackingVariable.markClose(flowInfo, flowContext);
} else {
trackingVariable.markClosedInNestedMethod();
}
} else if (closeTarget instanceof SuperReference) {
ReferenceBinding currentClass = this.binding.declaringClass;
if (closeTarget.isThis() || closeTarget.isSuper()) {
// this / super calls of closing method take care of all owning fields
ReferenceBinding currentClass = this.binding.declaringClass; // responsibility starts at the class and upwards
while (currentClass != null) {
for (FieldBinding fieldBinding : currentClass.fields()) {
if (fieldBinding.closeTracker != null) {
trackingVariable = fieldBinding.closeTracker;
FakedTrackingVariable trackingVariable = fieldBinding.closeTracker;
if (trackingVariable.methodScope == null || trackingVariable.methodScope == currentScope.methodScope()) {
trackingVariable.markClose(flowInfo, flowContext);
} else {
Expand All @@ -363,6 +356,16 @@ private void recordCallingClose(BlockScope currentScope, FlowContext flowContext
}
currentClass = currentClass.superclass();
}
} else {
FakedTrackingVariable trackingVariable = FakedTrackingVariable.getCloseTrackingVariable(closeTarget, flowInfo, flowContext,
currentScope.compilerOptions().isAnnotationBasedResourceAnalysisEnabled);
if (trackingVariable != null) { // null happens if target is not a variable or not an AutoCloseable
if (trackingVariable.methodScope == null || trackingVariable.methodScope == currentScope.methodScope()) {
trackingVariable.markClose(flowInfo, flowContext);
} else {
trackingVariable.markClosedInNestedMethod();
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2022 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -148,9 +148,7 @@ public void analyseCode(ClassScope classScope, FlowContext flowContext, FlowInfo
}
CompilerOptions compilerOptions = this.scope.compilerOptions();
if (compilerOptions.isAnnotationBasedResourceAnalysisEnabled
&& this.binding.declaringClass.hasTypeBit(TypeIds.BitAutoCloseable|TypeIds.BitCloseable)
&& CharOperation.equals(this.selector, TypeConstants.CLOSE)
&& this.arguments == null)
&& this.binding.isClosingMethod())
{
// implementation of AutoCloseable.close() should close all @Owning fields, create the obligation now:
ReferenceBinding currentClass = this.binding.declaringClass;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2344,6 +2344,20 @@ private boolean scanMethodForOwningAnnotations(IBinaryMethod method, MethodBindi
// return:
methodBinding.tagBits |= scanForOwningAnnotation(method.getAnnotations());

// check for "@Owning MyType this":
IBinaryTypeAnnotation[] methodTypeAnnotations = method.getTypeAnnotations();
if (methodTypeAnnotations != null) {
ITypeAnnotationWalker walker = new TypeAnnotationWalker(methodTypeAnnotations).toReceiver();
IBinaryAnnotation[] receiverTypeAnnotations = walker.getAnnotationsAtCursor(0, false);
if (receiverTypeAnnotations != null) {
for (IBinaryAnnotation binaryAnnotation : receiverTypeAnnotations) {
int typeBit = this.environment.getAnalysisAnnotationBit(signature2qualifiedTypeName(binaryAnnotation.getTypeName()));
if ((typeBit & TypeIds.BitOwningAnnotation) != 0)
methodBinding.extendedTagBits |= ExtendedTagBits.IsClosingMethod;
}
}
}

// parameters:
TypeBinding[] parameters = methodBinding.parameters;
int numVisibleParams = parameters.length;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2020, 2021 IBM Corporation and others.
* Copyright (c) 2020, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -17,7 +17,7 @@

public interface ExtendedTagBits {

int AreRecordComponentsComplete = ASTNode.Bit1;
int AreRecordComponentsComplete = ASTNode.Bit1; // type
int HasUnresolvedPermittedSubtypes = ASTNode.Bit2;

/** From Java 16
Expand All @@ -29,4 +29,7 @@ public interface ExtendedTagBits {
int IsCanonicalConstructor = ASTNode.Bit4; // constructor
int isImplicit = ASTNode.Bit5; // constructor and method

// @Owning / closing
int IsClosingMethod = ASTNode.Bit1; // method

}
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,12 @@ public boolean hasPolymorphicSignature(Scope scope) {

return false;
}
public boolean isClosingMethod() {
if (!this.declaringClass.hasTypeBit(TypeIds.BitAutoCloseable))
return false;
return (this.extendedTagBits & ExtendedTagBits.IsClosingMethod) != 0 // custom closing method with "@Owning MyType this"
|| (CharOperation.equals(this.selector, TypeConstants.CLOSE) && this.parameters == NO_PARAMETERS); // AutoCloseable.close()
}
public boolean ownsParameter(int i) {
if (this.parameterFlowBits != null)
return (this.parameterFlowBits[i] & PARAM_OWNING) != 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ protected Map<String, String> getCompilerOptions() {
"""
package org.eclipse.jdt.annotation;
import java.lang.annotation.*;
@Target({ElementType.TYPE, ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD})
@Target({ElementType.TYPE, ElementType.PARAMETER, ElementType.METHOD, ElementType.FIELD, ElementType.TYPE_USE})
public @interface Owning {}
""";
protected static final String NOTOWNING_JAVA = "org/eclipse/jdt/annotation/NotOwning.java";
Expand Down Expand Up @@ -1361,4 +1361,102 @@ public void close() throws IOException {
"",
null);
}
public void testConsumingMethod_nok() {
runLeakTestWithAnnotations(
new String[] {
"F.java",
"""
import org.eclipse.jdt.annotation.*;
public class F implements AutoCloseable {
@Owning AutoCloseable rc1;
@Owning AutoCloseable rc2;
public void close() throws Exception { consume(); }
public void consume(@Owning F this) throws Exception {
rc1.close();
}
}
"""
},
"""
----------
1. INFO in F.java (at line 6)
public void consume(@Owning F this) throws Exception {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Resource 'rc1' should be managed by try-with-resource
----------
2. ERROR in F.java (at line 6)
public void consume(@Owning F this) throws Exception {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Resource leak: 'this.rc2' is never closed
----------
""",
null);
}
public void testConsumingMethodUse() {
runLeakTestWithAnnotations(
new String[] {
"F.java",
"""
import org.eclipse.jdt.annotation.*;
public class F implements AutoCloseable {
public void close() {}
public void consume(@Owning F this) {
}
static void test() {
F f = new F();
f.consume();
}
}
"""
},
"""
----------
1. INFO in F.java (at line 7)
F f = new F();
^
Resource 'f' should be managed by try-with-resource
----------
""",
null);
}
public void testConsumingMethodUse_binary() {
runLeakTestWithAnnotations(
new String[] {
"p1/F.java",
"""
package p1;
import org.eclipse.jdt.annotation.*;
public class F implements AutoCloseable {
public void close() {}
public void consume(@Owning F this) {
}
}
"""
},
"",
null);
runLeakTestWithAnnotations(
new String[] {
"Test.java",
"""
import p1.F;
public class Test {
static void test() {
F f = new F();
f.consume();
}
}
"""
},
"""
----------
1. INFO in Test.java (at line 4)
F f = new F();
^
Resource 'f' should be managed by try-with-resource
----------
""",
null,
false);
}
}

0 comments on commit 384b83c

Please sign in to comment.