Added test for per-thread cleaner cleanup (failing)

Author: pmconrad

src/com/sun/jna/internal/Cleaner.java

@@ -121,8 +121,8 @@ protected synchronized boolean remove(long n) {
         }
     }
 
-    private static class MasterCleaner extends Cleaner {
-        private static MasterCleaner INSTANCE;
+    static class MasterCleaner extends Cleaner {
+        static MasterCleaner INSTANCE;
 
         public static synchronized void add(Cleaner cleaner) {
             if (INSTANCE == null) {
@@ -146,7 +146,7 @@ private static synchronized boolean deleteIfEmpty() {
             return false;
         }
 
-        private final Map<CleanerImpl,Boolean> cleanerImpls = new ConcurrentHashMap<CleanerImpl,Boolean>();
+        final Map<CleanerImpl,Boolean> cleanerImpls = new ConcurrentHashMap<CleanerImpl,Boolean>();
         private long lastNonEmpty = System.currentTimeMillis();
 
         private MasterCleaner() {

test/com/sun/jna/CallbacksTest.java

@@ -88,7 +88,7 @@ public static abstract class Condition<T> {
 
         abstract boolean evaluate(T t);
     }
-    protected static void waitForGc(Condition<?> condition) throws Exception {
+    public static void waitForGc(Condition<?> condition) throws Exception {
         for (int i = 0; i < 2 * Cleaner.MASTER_CLEANUP_INTERVAL_MS / 10 + 5 && condition.evaluate(); ++i) {
             synchronized (CallbacksTest.class) { // cleanups happen in a different thread, make sure we see it here
                 System.gc();

test/com/sun/jna/MasterCleanerTest.java

@@ -0,0 +1,92 @@
+package com.sun.jna;
+
+import com.sun.jna.internal.MasterAccessor;
+import junit.framework.TestCase;
+
+import java.util.function.Supplier;
+
+public class MasterCleanerTest extends TestCase {
+    private CallbacksTest.TestLibrary lib;
+
+    public static boolean waitFor(Supplier<Boolean> cond, long maxWaitMs) {
+        long start = System.currentTimeMillis();
+        while (System.currentTimeMillis() <= start + maxWaitMs && !cond.get()) {
+            try {
+                Thread.sleep(10);
+            } catch (InterruptedException ignore) {}
+        }
+        return cond.get();
+    }
+
+    @Override
+    protected void setUp() {
+        lib = Native.load("testlib", CallbacksTest.TestLibrary.class);
+    }
+
+    @Override
+    protected void tearDown() {
+        lib = null;
+    }
+
+    public void testGCCallbackOnFinalize() throws Exception {
+        final boolean[] latch = { false };
+        Thread thread = new Thread(() -> {
+            CallbacksTest.TestLibrary.VoidCallback cb = new CallbacksTest.TestLibrary.VoidCallback() {
+                @Override
+                public void callback() {
+                    latch[0] = true;
+                }
+            };
+            synchronized (latch) {
+                lib.callVoidCallback(cb);
+                latch.notifyAll();
+                try {
+                    latch.wait();
+                } catch (InterruptedException ignore) {}
+            }
+        });
+        thread.start();
+
+        CallbackReference ref;
+        synchronized (latch) {
+            if (!latch[0]) {
+                latch.wait();
+            }
+            assertTrue(latch[0]);
+            // Assert thread is running and has registered a callback and master is running too
+            assertTrue(thread.isAlive());
+            assertTrue(MasterAccessor.masterIsRunning());
+            assertFalse(MasterAccessor.getCleanerImpls().isEmpty());
+            assertEquals(1, CallbackReference.callbackMap.size());
+            ref = CallbackReference.callbackMap.values().iterator().next();
+            CallbacksTest.waitForGc(new CallbacksTest.Condition<CallbackReference>(ref) {
+                public boolean evaluate(CallbackReference _ref) {
+                    return _ref.get() != null || CallbackReference.callbackMap.containsValue(_ref);
+                }
+            });
+            assertNotNull("Callback GC'd prematurely", ref.get());
+            assertTrue("Callback no longer in map", CallbackReference.callbackMap.containsValue(ref));
+            latch.notifyAll();
+        }
+        thread.join();
+
+        // thread is no longer running, dummy is collectable, master is still running
+        final Pointer cbstruct = ref.cbstruct;
+        assertTrue(MasterAccessor.masterIsRunning());
+        CallbacksTest.waitForGc(new CallbacksTest.Condition<CallbackReference>(ref) {
+            public boolean evaluate(CallbackReference _ref) {
+                return _ref.get() != null || CallbackReference.callbackMap.containsValue(_ref);
+            }
+        });
+        assertNull("Callback not GC'd", ref.get());
+        assertFalse("Callback still in map", CallbackReference.callbackMap.containsValue(ref));
+
+        assertTrue("CleanerImpl still exists",
+                   waitFor(() -> MasterAccessor.getCleanerImpls().isEmpty(), 60_000)); // 1 minute
+
+        thread = null;
+        // thread is collectable -> wait until master terminates
+        assertTrue("Master still running",
+                   waitFor(() -> !MasterAccessor.masterIsRunning(), 60_000)); // 1 minute
+    }
+}

test/com/sun/jna/internal/MasterAccessor.java

@@ -0,0 +1,14 @@
+package com.sun.jna.internal;
+
+import java.util.Map;
+
+public class MasterAccessor {
+    public static synchronized boolean masterIsRunning() { // synchronized is for memory synch not mutex
+        return Cleaner.MasterCleaner.INSTANCE != null;
+    }
+
+    public static synchronized Map<?,Boolean> getCleanerImpls() { // synchronized is for memory synch not mutex
+        Cleaner.MasterCleaner mc = Cleaner.MasterCleaner.INSTANCE;
+        return mc == null ? null : mc.cleanerImpls;
+    }
+}