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

Resource Leak Checker: false negative related to owning fields #6276

Closed
kelloggm opened this issue Oct 30, 2023 · 0 comments · Fixed by #6280
Closed

Resource Leak Checker: false negative related to owning fields #6276

kelloggm opened this issue Oct 30, 2023 · 0 comments · Fixed by #6280

Comments

@kelloggm
Copy link
Contributor

kelloggm commented Oct 30, 2023

Based on my review of #6271, I wrote the following test case, which demonstrates an unsoundness in the current implementation of the Resource Leak Checker. The same problem exists in the code in #6271, but @Calvin-L has helpfully added a "TODO: this is very wrong" comment to the code that's responsible (thanks to @Calvin-L for that - I would not have noticed the issue otherwise):

// Test case for https://github.com/typetools/checker-framework/issues/6276

import org.checkerframework.checker.mustcall.qual.*;
import org.checkerframework.checker.calledmethods.qual.*;
import java.net.Socket;

@InheritableMustCall("a")
public class OwningFieldStringComparison {

  // :: error: required.method.not.called
  @Owning Socket s;

  // important to the bug: the name of this field must contain
  // the name of the owning socket
  /* @NotOwning */ Socket s2;

  // Note this "destructor" closes the wrong socket
  @EnsuresCalledMethods(value="this.s2", methods="close")
  public void a() {
    try {
      this.s2.close();
    } catch(Exception e) {

    } finally {
      try {
        this.s2.close();
      } catch (Exception e) {

      }
    }
  }
}

This problem is that String#contains is used by the RLC internally to check whether the value field of a destructor's @EnsuresCalledMethods annotation contains the simple name of an owning field. Instead, something like program element equality should be checked, instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant