Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
msridhar opened this issue Dec 10, 2023 · 4 comments
Open
Labels
good first issue A beginner-friendly place to start contributing to the Checker Framework ResourceLeakChecker
Milestone

Comments

@msridhar
Copy link
Contributor

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 msridhar added ResourceLeakChecker good first issue A beginner-friendly place to start contributing to the Checker Framework labels Dec 10, 2023
@Tejasker
Copy link

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.

@msridhar msridhar added this to the Medium milestone Dec 17, 2023
@jakelurie
Copy link

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
Copy link

@msridhar @Tejasker

@msridhar
Copy link
Contributor Author

msridhar commented May 16, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A beginner-friendly place to start contributing to the Checker Framework ResourceLeakChecker
Projects
None yet
Development

No branches or pull requests

3 participants