checker-framework icon indicating copy to clipboard operation
checker-framework copied to clipboard

See which other JDK subtypes of `AutoCloseable` should be `@MustCall("close")`

Open msridhar opened this issue 2 years ago • 4 comments

Right now, our JDK model does not annotate AutoCloseable as @MustCall("close"):

https://github.com/typetools/jdk/blob/88af971354b7e482ade8b250fde7699ed2e20217/src/java.base/share/classes/java/lang/AutoCloseable.java#L56-L59

It'd be nice if we could annotate it as @MustCall("close") and then annotate those AutoCloseable types that do not manage a resource as @MustCall(""), but it seems this causes issues with excessive warnings. Perhaps we could fix this? If not, we should at some point look through the list of JDK types implementing AutoCloseable and see which other ones should be @MustCall("close"):

https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/AutoCloseable.html

msridhar avatar Dec 10 '23 19:12 msridhar

hey @msridhar ,I guess you are correct like annotating @MustCall("close") would lead to excessive warnings especially for classes that don't manage any resources and one cannot determine the appropriate warnings in the stack.

Tejasker avatar Dec 13 '23 08:12 Tejasker

Can someone help verify if I understand this problem correctly: We want to look through all JDK types implementing AutoCloseable and check if they manage resources or not, and tag them all with @MustCall("") for non resource managing types and @MustCall("close") for resource managing types.

I looked through the application and only see the following classes defined within the application implementing AutoCloseable:

  • CloseableAndMore on CloseableAndMore.java:11
  • StaticNested on ScopingConstruct.java:8
  • NestedNested on ScopingConstruct.java:12
  • NestedInner on ScopingConstruct.java:17
  • Inner on ScopingConstruct.java:23
  • InnerInner on ScopingConstruct.java:30
  • InitalizationAfterSuperTest on InitalizationAfterSuperTest.java:12

But all of these look like test related objects. These are not the lines that we want to annotate, right?

I began looking further to see if any imported packages implement AutoCloseable, and found the following two objects within the app src code that do implement AutoCloseable:

  • Context
  • FileLock

But the object classes are not defined within this project, so they cannot be annotated. Is the ask here to simply annotate these test objects? All of which appear to be non-resource managing. Or is it to wrap the Context and FileLock objects in the source code with 'try with resources' blocks to ensure the .close() is called for these resource manging types?

jakelurie avatar May 14 '24 19:05 jakelurie

@msridhar @Tejasker

jakelurie avatar May 14 '24 19:05 jakelurie

Thanks for looking into this, @jakelurie! I don't think you've quite understood the issue correctly. Take a look at this page:

https://docs.oracle.com/en/java/javase/17/docs//api/java.base/java/lang/AutoCloseable.html

At the top of the page, you can see a list of all types within the JDK that implement AutoCloseable. The idea would be to go through that list, figure out which of the types actually manage a resource, and add an @InheritableMustCall("close") annotation to those types in the typetools/jdk repository, which will cause those types to be modeled as resources by the Resource Leak Checker. Does that make sense?

msridhar avatar May 16 '24 00:05 msridhar