Cleaner (thread) related memory leak

[cardamon]

JNA version: 5.13.0

$java --version
openjdk 17.0.7 2023-04-18
OpenJDK Runtime Environment (build 17.0.7+7-Ubuntu-0ubuntu122.04.2)
OpenJDK 64-Bit Server VM (build 17.0.7+7-Ubuntu-0ubuntu122.04.2, mixed mode, sharing)

Issue

We're running into a memory leak that is related to com.sun.jna.internal.Cleaner, when using JNA in a web app servlet that is served by Tomcat.

Cleaner starts a daemon thread called JNA Cleaner, which is not shut down upon undeploying the servlet. The thread will get Tomcat's custom/dedicated class loader for the servlet as its contextClassLoader (an instance of Tomcat's ParallelWebappClassLoader).
This classloader maintains references to all the classes it loaded (i.e. the entire webapp), and so all these are prevented from being GC'ed.

Workaround

I've managed to construct a hacky workaround that solves the problem, but it's not pretty...
It uses a sneaky throw of an InterruptedException in a cleanup task which is registered to the cleaner instance in the webapp's contextDestroyed method.

private static class JnaCleanerDestroyer implements Runnable {
    @lombok.SneakyThrows
    @Override
    public void run() {
        throw new InterruptedException();
    }
}

In the webapp's contextDestroyed:

var ref = new Object();
var cleaner = com.sun.jna.internal.Cleaner.getCleaner();
cleaner.register(ref, new JnaCleanerDestroyer());
ref = null;

This works as long as the Cleaner impl. does not change the way it handles InterruptedException, but it's obviously messy.

Feature request

Could a more permanent solution be realized? E.g. a method dispose() on Cleaner, that causes the thread to be shut down.

[matthiasblaesing]

What could be done is check if there are remaining cleanup tasks registered when a reference is cleared and if not shutdown the thread draining the queue. On addition there would be the need to check whether or not the thread is running and if not start it up. The thread shutdown should not happen immediatly, else it would be possible that the Thread is created and destroyed in rapid succession.

An alternative might be to give environments an option to use an external cleanup queue and reduce Cleaner to a shim that calls into that.

Your work around will leak native memory as you are preventing cleanup of native resources when you kill the cleaner thread an explicit dispose method will not change that. The dispose method might even be problematic as it would have to call into the clean methods and can't determine whether or not it is save to do so, in the worst case freeing memory while it is in use by a native function.

[cardamon]

Ah it's more complicated, good to know - so now basically one memory leak is swapped for another...

An alternative might be to give environments an option to use an external cleanup queue

That sounds nice I think, and possibly the easiest solution?
It would allow for users to opt in for something that permanently shuts down, e.g. by causing future cleanup tasks after 'shutdown' to be executed immediately, and then throw an exception.

[matthiasblaesing]

An alternative might be to give environments an option to use an external cleanup queue

That sounds nice I think, and possibly the easiest solution? It would allow for users to opt in for something that permanently shuts down, e.g. by causing future cleanup tasks after 'shutdown' to be executed immediately, and then throw an exception.

No - you still must not externally trigger cleanup. Consider Memory. If you externally invoke clean on the cleaner you are freeing the memory. If you are lucky you get an immediate segfault because you pass a null pointer to native. If you are unlucky you pass a valid pointer to native, free the memory while it is in use by native and get random memory corruption.

Raising exceptions from the cleaner will leak memory as the native memory allocation has be done and you just prevent cleanup.

[cardamon]

OK - I don't think I can be of much help so I'll wait & hope for a solution.

[matthiasblaesing]

This will be harder, than I first thought. The cleaner thread itself is trivial, but there are more long lived referenced, that are held in static fields, so are essentially not collected and thus keep the cleaner alive.
You could try loading JNA from the tomcat classpath, so that it only loaded once.

[matthiasblaesing]

@cardamon could you please check if the build from #1555 works for you? For the trivial case the cleaner terminates, it would be good to know if it helps in reality. Please note, that the cleaner will terminate 30s after the last tracked reference is cleared.

[cardamon]

We recently switched to loading JNA by configuring a shared.loader in Tomcat as suggested so it's actually not something I can test anymore.

I can close this feature request if you'd like, but perhaps its still useful to someone else?

[matthiasblaesing]

It is great that you solved the issue for your use-case. Please keep this open, as I indeed think a cleaner that shuts down if it is not necessary anymore is a good tool. I'll just have to see how to test myself 😃

[matthiasblaesing]

Shutdown is now enabled via: #1555

[kennymacleod]

This change didn't make it into the 5.14.0 release, for some reason. Can we get a new release containing this fix?

[matthiasblaesing]

It sure is in 5.14.0, but I suspect, that you think it works "just so", which it does not. Have a look at this comment:

#1555 (comment)

You need to ensure, that all references are collected, then the cleaner can be shut down. In the comment the reference this code:

    Library.Handler lh = (Library.Handler) Proxy.getInvocationHandler(LibC.INSTANCE);
    lh.getNativeLibrary().close();

ensures, that the Cleaner for LibC is ran, else the static field keeps the cleaner running.