How to close a static variable of AutoClosable type?

Question

I’m the author of a Java library that provides Java access to a C++ library our company sells. The instances of one of the classes “own” C++ objects in the following sense: The class has a few private long fields that are set by certain native methods and have the role of (unique) pointers into the C++ heap. Because the C++ heap is not garbage collected, the memory must be freed manually when the owning Java object doesn’t need them anymore, in particular when the owning Java object itself isn’t needed anymore, so the class implements AutoClosable and close() frees all the C++ memory owned by the instance. Ideally, users would use try-with-resources on the instances of that class.

One customer complained that SonarQube warns them that close() isn’t called to free resources and suggests to use try-with-resources on the object, but the object is held by a static variable or something equivalent and lives until the application closes (as far as I understood). I want to help them, but removing AutoClosable (as they suggested) simply isn’t correct. The issue isn’t so much about the memory: When the application ends and the C++ library is unloaded, the memory resources are freed (by the OS) anyway.

So, how does one close AutoClosable objects held by static variables, ideally in a way that SonarQube detects? Java does not seem to have the opposite of a class initializer that could be used.

I don’t have access to SonarQube to play around and see what could work, i.e. when SonarQube recognizes that close() will be called. I’m about to tell them that they should appropriately reconfigure SonarQube or suppress the warning. Asking this question, I want to make sure it’s essentially the best course of action. “Yes, it is” would be an appropriate answer if it is true. Of course, not using global state, i.e. not having a static AutoClosable is a no-brainer, but I guess they already know that.

It seems like no-one had the problem of static AutoClosable objects, since there are no questions about it on Stack Overflow. I figured that AutoClosable is similar to IDisposable in .NET (C#), and I found this question very much asking this, however the answers are specific to the use-case and .NET, and also don’t provide a general solution.

Answer 1

One customer complained that SonarQube warns them that close() isn’t called to free resources and suggests to use try-with-resources on the object, but the object is held by a static variable or something equivalent and lives until the application closes (as far as I understood).

"Doctor, it hurts when I smash a hammer into my face!"

Stop doing that then.

SonarQube is a tool. It can be configured to just say silly things, similar to that hammer. The fix isn't to try to grow a hammerproof face. The fix is to simply stop doing the thing whose only purpose is to hurt.

Unfortunately, your customer might not be willing to hear that.

But if they are, they can tell sonarqube to stop doing that. They can even add a simple comment to tell sonarqube not to complain about that specific case.

It seems like no-one had the problem of static AutoClosable objects, since there are no questions about it on Stack Overflow.

It's kind of a weird thing to want it, even by your customer. If there's a global variable that has this kind of state, that means your customer's code is untestable (they can't really replace it with a dummy implementation for example) - but, again, given that it's a customer, they might not be willing to hear that.

Normally you make the resource once soon after app start and hand it to the rest of your code in a try-with block that never exits unless the app exits:

public static void main(String[] args) {
  try (var out = new FileOutputStream(args[0])) {
    yourActualApp(out);
  }

something like the above. The same would work for your library. But, your customer must want to move away from a static global field and pass the resource along everywhere to make this work, and they might not want to do that.

A possible solution

Split whatever java class represents your resource in twain. 2 classes, both of which simply wrap around the actual implementation. One implements AutoClosable, the other doesn't. Name the one that doesn't 'GlobalFoo' where Foo is what you currently name it - and put in its javadoc that the intent for such a thing is that it exists for the lifetime of your JVM. This way your users have to explictly opt into 'nono this one lives forever, no need to close it!', and you can add some smarts, such as - if it's a resource with a unique key (say, it's representing a file. files have unique keys: The fully qualified file name!) - you can store those unique keys and throw an error if a user of your library attempts to make such a global object using the same key more than once ever.

You can have one extend the other - that extender simply adds AutoClosable and nothing more.

Answer 2

One customer complained that SonarQube warns them that close() isn’t called to free resources and suggests to use try-with-resources on the object, but the object is held by a static variable or something equivalent and lives until the application closes (as far as I understood).

If SonarQube is to be believed, then the user is creating an instance of your class that is not managed by a try-with-resources statement and that they do not close() explicitly. There are then two possibilities:

1. that's ok for instances of your particular class, or
2. that's a usage error.

Either way, it's fundamentally a "them" problem, not a "you" problem.

I want to help them, but removing AutoClosable (as they suggested) simply isn’t correct.

I expect that removing AutoClosable (and any of its subinterfaces that you've explicitly implemented) will silence SonarQube, because it will no longer recognize the object as one whose close() method is relevant. I'm inclined to believe you that that would be incorrect. Certainly that would make the class invalid for use with try-with-resources statements. That would not make the usage that SonarQube presently reports on any more or less correct than it already is.

The issue isn’t so much about the memory: When the application ends and the C++ library is unloaded, the memory resources are freed (by the OS) anyway.

That tends to support the premise that it is (1), above, that applies to your class. There are other potential issues with objects not being closed, however. For example, if instances of your class manage modifiable files then failing to close them could mean that some writes are lost if the VM terminates abnormally. But again, this seems like a usage problem, not a design or implementation problem.

So, how does one close AutoClosable objects held by static variables, ideally in a way that SonarQube detects? Java does not seem to have the opposite of a class initializer that could be used.

By explicitly invoking their close() methods. AutoCloseable is not relevant here except inasmuch as it declares that method. It does not have any special semantics apart from its relevance to try-with-finally statements. And you are correct, Java does not have any kind of hook for executing code before a class is cleaned up, nor does it guarantee that unused classes ever will be cleaned up at all, not even at program termination.

Comments on the question mention shutdown hooks, but these are bad juju. Moreover, I doubt that anything you could do with them inside the class itself would satisfy SonarQube, and I'd be a little surprised if your user could successfully use them to satisfy SonarQube, either. And even if they did, would that actually make their application work in any way better or more reliably?

Overall, I think it is usually a mistake to store an AutoCloseable object in a static field in the first place. To the extent that there is a reasonable expectation that AutoClosables need to be closed, that's hard to reconcile with the indefinite availability typical of static fields.

I’m about to tell them that they should appropriately reconfigure SonarQube or suppress the warning.

If you're satisfied that it's ok for the object not to be closed (at all), then that's exactly what I would do.

Of course, not using global state, i.e. not having a static AutoClosable is a no-brainer, but I guess they already know that.

I wouldn't assume that they do know that, but I also wouldn't assume that they would respond well to being told that they shouldn't be doing what they are doing. Nor is that a particularly good look from a user support perspective.