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

False positive in the resource leak checker when the finalizer method has multiple @EnsuresCalledMethods annotation #5911

Closed
Nargeshdb opened this issue May 22, 2023 · 2 comments · Fixed by #6180

Comments

@Nargeshdb
Copy link
Contributor

Consider the following example, in which there's a class that includes two fields with non-empty must-call obligations. The resource leak checker is unable to verify that both fields are properly released in the finalizer method.

import org.checkerframework.checker.mustcall.qual.*;
import org.checkerframework.checker.calledmethods.qual.*;

class MultipleECMAnnosOnMethods {

  @InheritableMustCall("a")
  static class Foo {
    void a() {}
  }

  @InheritableMustCall("b")
  static class Bar {
    void b() {}
  }

  @InheritableMustCall("finalizer")
  static class OwningField {

    // :: warning: (required.method.not.called)
    private final @Owning Foo owningFoo;

    // :: warning: (required.method.not.called)
    private final @Owning Bar owningBar;

    public OwningField() {
      this.owningFoo = new Foo();
      this.owningBar = new Bar();
    }

    @EnsuresCalledMethods(value = {"this.owningBar"}, methods = {"b"})
    @EnsuresCalledMethods(value = {"this.owningFoo"}, methods = {"a"})
    void finalizer() {
      this.owningFoo.a();
      this.owningBar.b();
    }

  }
}
@jacek-lewandowski
Copy link

Shouldn't it be written like:

@EnsureCalledMethods.List({
    @EnsuresCalledMethods(value = {"this.owningBar"}, methods = {"b"})
    @EnsuresCalledMethods(value = {"this.owningFoo"}, methods = {"a"})
}

?

@msridhar
Copy link
Contributor

Shouldn't it be written like:

@EnsureCalledMethods.List({
    @EnsuresCalledMethods(value = {"this.owningBar"}, methods = {"b"})
    @EnsuresCalledMethods(value = {"this.owningFoo"}, methods = {"a"})
}

?

@jacek-lewandowski since the @EnsuresCalledMethod annotation has a @Repeatable meta-annotation I think either variant is equivalent.

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.

3 participants