In Java, how to check that AutoCloseable.close() h

Question

I am authoring a java library. Some of the classes that are meant to be used by library users, hold native system resources (over JNI). I'd like to ensure that the user "disposes" these objects, as they are heavy, and in a testsuite they may cause leakage between testcases (for example, I need to ensure TearDown will dispose). For this purpose I made the Java classes implement AutoCloseable, but this doesn't seem to suffice, or I'm not using it correctly:

1. I don't see how to use try-with-resources statement in the context of tests (I'm using JUnit5 with Mockito), in that the "resource" is not short-lived - it is part of the test fixture.

2. Being diligent as always, I tried implementing finalize() and testing for closure there, but it turns out finalize() is not even called (Java10). This is also marked as deprecated and I'm sure this idea will be frowned upon.

How is this done? To be clear, I want the application's tests (that use my library) to fail if they don't call close() on my objects.

---

Edit: adding some code if it helps. It's not much, but it's what I'm trying to do.

@SuppressWarnings("deprecation") // finalize() provided just to assert closure (deprecated starting Java 9)
@Override
protected final void finalize() throws Throwable {
    if (nativeHandle_ != 0) {
         // TODO finalizer is never called, how to assert that close() gets called?
        throw new AssertionError("close() was not called; native object leaking");
    }
}

---

Edit2, outcome of bounty Thanks all for replying, half the bounty was automatically awarded. I concluded that for my case it would be best to try the solution involving Cleaner. However it seems, cleaning actions although registered, are not invoked. I asked a follow-up question here.

Answer 1

If you are interested in consistency in tests, just add method destroy() marked by @AfterClass annotation into test class and close all previously allocated resources in it.

If you are interested in an approach that allow you to protect the resource from being not closed, you could provide a way that doesn't expose resource to user explicitly. For example, you code could control resource life cycle and accept only Consumer<T> from user.

If you can't do that, but still want to be sure that resource will be closed even if user doesn't use it correctly you'll have to do several tricky things. You could split your resource on sharedPtr and resource itself. Then expose sharedPtr to user and put it into some internal storage wrapped into WeakReference. As result of that you'll be able to catch the moment when GC removes sharedPtr and call close() on the resource. Be aware that resource must not be exposed to user. I prepared an example, it's not very accurate, but hope it shows the idea:

public interface Resource extends AutoCloseable {

    public int jniCall();
}

class InternalResource implements Resource {

    public InternalResource() {
        // Allocate resources here.
        System.out.println("Resources were allocated");
    }

    @Override public int jniCall() {
        return 42;
    }

    @Override public void close() {
        // Dispose resources here.
        System.out.println("Resources were disposed");
    }
}

class SharedPtr implements Resource {

    private final Resource delegate;

    public SharedPtr(Resource delegate) {
        this.delegate = delegate;
    }

    @Override public int jniCall() {
        return delegate.jniCall();
    }

    @Override public void close() throws Exception {
        delegate.close();
    }
}

public class ResourceFactory {

    public static Resource getResource() {
        InternalResource resource = new InternalResource();
        SharedPtr sharedPtr = new SharedPtr(resource);

        Thread watcher = getWatcherThread(new WeakReference<>(sharedPtr), resource);
        watcher.setDaemon(true);
        watcher.start();

        Runtime.getRuntime().addShutdownHook(new Thread(resource::close));

        return sharedPtr;
    }

    private static Thread getWatcherThread(WeakReference<SharedPtr> ref, InternalResource resource) {
        return new Thread(() -> {
            while (!Thread.currentThread().isInterrupted() && ref.get() != null)
                LockSupport.parkNanos(1_000_000);

            resource.close();
        });
    }
}

Answer 2

If I were you, I'd do the following:

• Write a static wrapper around your calls that returns "heavy" objects
• Create a collection of PhantomReferences to hold all your heavy objects, for cleanup purposes
• Create a collection of WeakReferences to hold all your heavy objects, to check whether they are GC'd or not (have any reference from the caller or not)
• At teardown I would check the wrapper to see what resources have been GC'd (have reference in the Phantom, but not in the Weak), and I'd check whether they have been closed or nor properly.
• If you add some debug/caller/stacktrace information while serving the resource, it will be easier to trace back the leaking test case.

It also depends whether you want to use this mechanism in production or not - maybe it is worth to add this feature to your lib, because resource management will be a problem in production environment, too. In this case you don't need a wrapper, but you can extend your current classes with this feature. Instead of a teardown, you can use a background thread for regular checks.

Regarding reference types, I recommend this link. PhantomReferences are recommended to use for resource cleanups.

Answer 3

This post does not directly answer your question but provides a different point of view.

One approach to make your clients consistently call close is to free them from this responsibility.

How can you do it?

Use template pattern.

Sketch implementation

You mentioned that you're working with TCP, so let's assume that you have a TcpConnection class that has a close() method.

Let's define TcpConnectionOpertaions interface:

public interface TcpConnectionOperations {
  <T> T doWithConnection(TcpConnectionAction<T> action);
}

and implement it:

public class TcpConnectionTemplate implements TcpConnectionOperations {
  @Override
  public <T> T doWithConnection(TcpConnectionAction<T> action) {
    try (TcpConnection tcpConnection = getConnection()) {
      return action.doWithConnection(tcpConnection);
    }
  }
}

TcpConnectionAction is just a callback, nothing fancy.

public interface TcpConnectionAction<T> {
  T doWithConnection(TcpConnection tcpConnection);
}

How the library should be consumed now?

• It must be consumed only through TcpConnectionOperations interface.
• Consumers supply actions

For example:

String s = tcpConnectionOperations.doWithConnection(connection -> {
  // do what we with with the connection
  // returning to string for example
  return connection.toString();
});

Pros

• Clients don't have to worry about:
• • getting a TcpConnection
• • closing the connection
• You are in control of creating connections:
• • you can cache them
• • log them
• • collect statistics
• • many other use cases...
• In tests you can provide mock TcpConnectionOperations and mock TcpConnections and make assertions against them

Cons

This approach may not work if the lifecycle of a resource is longer than action. E.g. it is necessary for the client to keep the resource for a longer time.

Then you might want to dive deep in ReferenceQueue/Cleaner (since Java 9) and related API.

Inspired by Spring framework

This pattern is widely used in Spring framework.

See for example:

• JdbcTemplate
• TransactionTemplate
• JmsTemplate.
• (there are many others)

Update 2/7/19

How can I cache/reuse the resource?

This is some kind of pooling:

a pool is a collection of resources that are kept ready to use, rather than acquired on use and released

Some pools in Java:

• HikariCP is a JDBC connection pool library
• OkHttps ConnectionPool
• Tomcat JDBC Connection pool
• ThreadPoolExecutor

When implementing a pool several questions are raised:

• When the resource actually should be closed?
• How the resource should be shared between multiple threads?

When the resource should be closed?

Usually pools provide an explicit close method (it may have a different name but the purpose is the same) which closes all the resources held.

• HikariDataSource#close
• ConnectionPool#evictAll "Close and remove all idle connections in the pool."
• ConnectionPool#close
• ThreadPoolExecutor#shutdown

How it can be shared across multiple threads?

It depends on a kind of the resource itself.

Usually you want to ensure that only one thread accesses one resource.

This can be done using some kind of locking

Demo

Note that code provided here is only for demonstration purposes It has awful performance and violates some OOP principles.

IpAndPort.java

@Value
public class IpAndPort {
  InetAddress address;
  int port;
}

TcpConnection.java

@Data
public class TcpConnection {
  private static final AtomicLong counter = new AtomicLong();

  private final IpAndPort ipAndPort;
  private final long instance = counter.incrementAndGet();

  public void close() {
    System.out.println("Closed " + this);
  }
}

CachingTcpConnectionTemplate.java

public class CachingTcpConnectionTemplate implements TcpConnectionOperations {
  private final Map<IpAndPort, TcpConnection> cache
      = new HashMap<>();
  private boolean closed; 
  public CachingTcpConnectionTemplate() {
    System.out.println("Created new template");
  }

  @Override
  public synchronized <T> T doWithConnectionTo(IpAndPort ipAndPort, TcpConnectionAction<T> action) {
    if (closed) {
      throw new IllegalStateException("Closed");
    }
    TcpConnection tcpConnection = cache.computeIfAbsent(ipAndPort, this::getConnection);
    try {
      System.out.println("Executing action with connection " + tcpConnection);
      return action.doWithConnection(tcpConnection);
    } finally {
      System.out.println("Returned connection " + tcpConnection);
    }
  }

  private TcpConnection getConnection(IpAndPort ipAndPort) {
    return new TcpConnection(ipAndPort);
  }


  @Override
  public synchronized void close() {
    if (closed) {
      throw new IllegalStateException("closed");
    }
    closed = true;
    for (Map.Entry<IpAndPort, TcpConnection> entry : cache.entrySet()) {
      entry.getValue().close();
    }
    System.out.println("Template closed");
  }
}

Tests infrastructure

TcpConnectionOperationsParameterResolver.java

public class TcpConnectionOperationsParameterResolver implements ParameterResolver, AfterAllCallback {
  private final CachingTcpConnectionTemplate tcpConnectionTemplate = new CachingTcpConnectionTemplate();

  @Override
  public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
    return parameterContext.getParameter().getType().isAssignableFrom(CachingTcpConnectionTemplate.class)
        && parameterContext.isAnnotated(ReuseTemplate.class);
  }

  @Override
  public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException {
    return tcpConnectionTemplate;
  }

  @Override
  public void afterAll(ExtensionContext context) throws Exception {
    tcpConnectionTemplate.close();
  }
}

The ParameterResolver and AfterAllCallback are from JUnit.

@ReuseTemplate is a custom annotation

ReuseTemplate.java:

@Retention(RetentionPolicy.RUNTIME)
public @interface ReuseTemplate {
}

Finally test:

@ExtendWith(TcpConnectionOperationsParameterResolver.class)
public class Tests2 {
  private final TcpConnectionOperations tcpConnectionOperations;

  public Tests2(@ReuseTemplate TcpConnectionOperations tcpConnectionOperations) {
    this.tcpConnectionOperations = tcpConnectionOperations;
  }

  @Test
  void google80() throws UnknownHostException {
    tcpConnectionOperations.doWithConnectionTo(new IpAndPort(InetAddress.getByName("google.com"), 80), tcpConnection -> {
      System.out.println("Using " + tcpConnection);
      return tcpConnection.toString();
    });
  }

  @Test
  void google80_2() throws Exception {
    tcpConnectionOperations.doWithConnectionTo(new IpAndPort(InetAddress.getByName("google.com"), 80), tcpConnection -> {
      System.out.println("Using " + tcpConnection);
      return tcpConnection.toString();
    });
  }

  @Test
  void google443() throws Exception {
    tcpConnectionOperations.doWithConnectionTo(new IpAndPort(InetAddress.getByName("google.com"), 443), tcpConnection -> {
      System.out.println("Using " + tcpConnection);
      return tcpConnection.toString();
    });
  }
}

Running:

$ mvn test

Output:

Created new template
[INFO] Running Tests2
Executing action with connection TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Using TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Returned connection TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Executing action with connection TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=443), instance=2)
Using TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=443), instance=2)
Returned connection TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=443), instance=2)
Executing action with connection TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Using TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Returned connection TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Closed TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=80), instance=1)
Closed TcpConnection(ipAndPort=IpAndPort(address=google.com/74.125.131.102, port=443), instance=2)
Template closed

The key observation here is that connections are reused (see "instance=")

This is oversimplified example of what can be done. Sure, in the real world pooling connections is not so simple. The pool should not grow indefinitely, connections can be kept only for specific period of time and so on. Usually some problems are solved by having something in the background.

Returning to the question

I don't see how to use try-with-resources statement in the context of tests (I'm using JUnit5 with Mockito), in that the "resource" is not short-lived - it is part of the test fixture.

See Junit 5 User Guide. Extension model

Being diligent as always, I tried implementing finalize() and testing for closure there, but it turns out finalize() is not even called (Java10). This is also marked as deprecated and I'm sure this idea will be frowned upon.

You overrode finalize so that it throws an exception but they are ignored.

See Object#finalize

If an uncaught exception is thrown by the finalize method, the exception is ignored and finalization of that object terminates.

The best you can do here is to log the resource leakage and close the resource

To be clear, I want the application's tests (that use my library) to fail if they don't call close() on my objects.

How do application tests use your resource? Do they instantiate it using new operator? If yes then I think PowerMock can help you (but I'm not sure)

If you have hidden instantiation of the resource behind some kind of a factory then you can give the application tests some mock factory

---

If you're interested you can watch this talk. It's in Russian, but still may be helpful (part of my answer is based on this talk).

Answer 4

In general, if you could reliably test whether a resource has been closed, you could just close it yourself.

The first thing to do is to make handling resource easy for the client. Use the Execute Around idiom.

As far as I know, the only use of execute around for resource handling in the Java library is java.security.AccessController.doPrivileged and that's special (the resource is a magic stack frame, that you really don't want to leave open). I believe Spring has long had a much-needed JDBC library for this. I was certainly using execute-around (didn't know it was called that at the time) for JDBC shortly after Java 1.1 made it vaguely practical.

The library code should look something like:

@FunctionalInterface
public interface WithMyResource<R> {
    R use(MyResource resource) throws MyException;
}
public class MyContext {
// ...
    public <R> R doAction(Arg arg, WithMyResource<R> with) throws MyException {
        try (MyResource resource = acquire(arg)) {
            return with.use(resource);
        }
    }

(Do get the type parameter declarations in the right place.)

Client side usage looks something like:

MyType myResult = yourContext.doContext(resource -> {
    ...blah...;
    return ...thing...;
});

Back to testing. How do we make it easy to test even if the testee exfiltrates the resource from the execute around or some other mechanism is available?

The obvious answer is that you provide the execute around solution to the test. You will need to provide some execute around-using API to verify all of your resources that have been acquired in the scope have also been closed. This should be paired with the context the resource is acquired from rather than using global state.

Depending upon which testing framework your clients are using, you may be able to offer something better. JUnit5, for instance, has an annotation-based extension facility which allows you to supply the context as an argument and also apply checks after each test has executed. (But I haven't used it much, so I'm not going to say anything more.)

Answer 5

i would provide instances to those objects through Factory methods and with that i have control over the creation of them, and i will feed the consumers with Proxies that does the logic of closing the object

interface Service<T> {
 T execute();
 void close();
}

class HeavyObject implements Service<SomeObject> {
  SomeObject execute() {
  // .. some logic here
  }
  private HeavyObject() {}

  public static HeavyObject create() {
   return new HeavyObjectProxy(new HeavyObject());
  }

  public void close() {
   // .. the closing logic here
  }
}

class HeavyObjectProxy extends HeavyObject {

  public SomeObject execute() {
    SomeObject value = super.execute();
    super.close();
    return value;
  }
}