Shutdown CleanerThread once the last cleanable is removed

[matthiasblaesing]

No description provided.

[dbwiddis]

I'm not an expert in concurrency, so the code may be fine; just asking questions mostly for my own education.

[dbwiddis]

Does this need to be volatile to ensure it works correctly? Or is the presence of start() sufficient to ensure this is not a problem?

See any discussion of double checked locking for the relevant technical concern.

[matthiasblaesing]

I don't think cleanerThread needs to be volatile. I tried to get through the JLS regarding synchronization, but some google foo led me to this summary:

https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html#MemoryVisibility

[...] Chapter 17 of the Java Language Specification defines the happens-before relation on memory operations such as reads and writes of shared variables. The results of a write by one thread are guaranteed to be visible to a read by another thread only if the write operation happens-before the read operation. The synchronized and volatile constructs, as well as the Thread.start() and Thread.join() methods, can form happens-before relationships. In particular:

• Each action in a thread happens-before every action in that thread that comes later in the program's order.
• An unlock (synchronized block or method exit) of a monitor happens-before every subsequent lock (synchronized block or method entry) of that same monitor. And because the happens-before relation is transitive, all actions of a thread prior to unlocking happen-before all actions subsequent to any thread locking that monitor.
  [...]

In this case the critical monitor is the one associated with referenceQueue. Both places where the cleanerThread is modified are protected by a synchronization on referenceQueue and so to my reading of the happens-before guarantee for synchronized blocks, there can be no phantom-reads from cleanerThread. The same is true for firstCleanable, which needs to be considered also.

Usage of the referenceQueue monitor is sane from my POV, because there are two different variables affected (cleanerThread and firstCleanable) and multiple statement that must be executed atomically, which excludes the usage of a volatile variable.

[dbwiddis]

Yeah, I did enough rubber-ducking to convince myself the use of referenceQueue was rather effective.

My main concern was that the initial assignment of cleanerThread to an object happens-before, but there is no guarantee that the remainder of object initialization happens.... except in this case it must complete (within the block) in order for the start() method to be valid.

[dbwiddis]

I'm wondering if using an ExecutorService might make any of this easier. See https://stackoverflow.com/questions/13883293/turning-an-executorservice-to-daemon-in-java

[matthiasblaesing]

So I managed to get a basic example working. This is the called class:

public class DemoThread implements Runnable {
    public void run() {
        System.out.println("===================");
        byte[] data = new byte[1024];
        int length = LibC.INSTANCE.gethostname(data, data.length - 1);
        System.out.println(new String(data, 0, length, StandardCharsets.UTF_8));
        System.out.println("===================");
        Library.Handler lh = (Library.Handler) Proxy.getInvocationHandler(LibC.INSTANCE);
        lh.getNativeLibrary().close();
    }
}

And this is the caller:

public class Test {

    private static final Logger CLEANER_LOGGER = Logger.getLogger("com.sun.jna.internal.Cleaner");
    private static volatile byte[] data = null;

    public static void main(String[] args) throws Exception {
        CLEANER_LOGGER.setLevel(Level.FINEST);
        for(Handler h: Logger.getLogger("").getHandlers()) {
            h.setLevel(Level.ALL);
        }
        URLClassLoader classLoader = new URLClassLoader(new URL[] {
                new File("/home/matthias/src/Test/target/classes").toURI().toURL(),
                new File("/home/matthias/src/jnalib/build/jna.jar").toURI().toURL(),
                new File("/home/matthias/src/jnalib/contrib/platform/dist/jna-platform.jar").toURI().toURL()
        }, null);
        ((Runnable) classLoader.loadClass("eu.doppelhelix.liferay.testagent.test.X").newInstance()).run();
        while(true) {
            data = new byte[102400];
            Thread.sleep(100);
        }
    }
}

This shows the problem: normal objects are collected by the GC, but the LibC.INSTANCE will not be collected. Once LibC is initialized, its member INSTANCE` is holding a hard reference, that blocks a NativeLibrary cleaner from running.

The lines

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

manually close the native library and unblock the cleaner from shutting down.