Lombok: Feature request: apply @Cleanup to fields

Created on 23 Jun 2018  路  7Comments  路  Source: projectlombok/lombok

@Cleanup should be applicable to fields, not just local variables. Annotating an instance field with @Cleanup in a class implementing AutoCloseable would add the cleanup to the close() method, after any explicit or inherited code of that method, and would generate that method if missing. Any such class without a finalizer would also get one generated:

@Generated("lombok") @Override protected void finalize() {
  super.finalize();
  close();
}

For an instance field of any other class, it'd happen in finalize(), again after either the explicit code or super.finalize();. For a static field, it'd happen in the finalizer of a purpose-built singleton stored in a generated private static field, and whose finalizer would be added as a shutdown hook.

Most helpful comment

Imho lombok should not provide incentives to use things generally considered as bad practice. So let the class implement Closeable, or implement things manually.

All 7 comments

I think it is better just to implement Closable interface in this case:

// lombok

public class A {
  @Cleanup
  private InputStream is = ...;
}

// vanilla java

public class A implements Closable {
  private InputStream is = ...;

  public void close() {
    if (is != null) {
    try {
       is.close();
      ....
     }
    }
  }
}

I agree with @SergiusSidorov the finalize method is not a good idea. But if we make a class implements Closeable automatically we hide from a class' contract the way how users can use the class and now a user have to check all properties for the @Cleanup annotation.
I think we have to require from a class implements AutoCloseable or just Closeable and if there is no such interface we have to fail a build.

So my consider is for the code:

public class A {
  @Cleanup
  private InputStream is = ...;
}

it's an exception on compile time

for the code:

public class A implements Closeable {
  @Cleanup
  private InputStream is = ...;
}

// vanilla java

public class A implements Closeable {
  private InputStream is = ...;

  public void close() {
    if (is != null) {
    try {
       is.close();
      ....
     }
    }
  }
}

@SergiusSidorov That might work, but close() should still be added to the finalizer just in case. And it may sometimes need to be AutoCloseable instead of Closeable. (The only difference between the two interfaces is that Closeable.close() can't throw checked exceptions other than IOException.)

Having a finalizer does have severe impact on the garbage collector.

@Pr0methean a finalizer method can bring a big problem for performance.
The general advice is do not use a finalizer if it's possible. There is the good article about it.

Then how about a parameter, so that there's only a finalizer if it's @Cleanup(finalize = true)?

Imho lombok should not provide incentives to use things generally considered as bad practice. So let the class implement Closeable, or implement things manually.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

gardenias picture gardenias  路  3Comments

Bryksin picture Bryksin  路  3Comments

iskigow picture iskigow  路  3Comments

zenglian picture zenglian  路  3Comments

delverdev picture delverdev  路  3Comments