From 45ef6b2b31861b973db38f6db54d2dac6200960f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Bl=C3=A4sing?= Date: Thu, 28 Sep 2023 22:52:01 +0200 Subject: [PATCH] Shutdown CleanerThread once the last cleanable is removed --- CHANGES.md | 1 + src/com/sun/jna/internal/Cleaner.java | 130 ++++++++++++++++---------- 2 files changed, 83 insertions(+), 48 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d347f12625..1048cf3dc5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,7 @@ Features * [#1548](https://github.com/java-native-access/jna/pull/1548): Make interface `c.s.j.p.mac.XAttr public` - [@matthiasblaesing](https://github.com/matthiasblaesing). * [#1551](https://github.com/java-native-access/jna/pull/1551): Add `c.s.j.p.bsd.ExtAttr` and `c.s.j.p.bsd.ExtAttrUtil` to wrap BSD [](https://man.freebsd.org/cgi/man.cgi?query=extattr&sektion=2) system calls. [@rednoah](https://github.com/rednoah). * [#1517](https://github.com/java-native-access/jna/pull/1517): Add missing `O_*` (e.g. `O_APPEND`, `O_SYNC`, `O_DIRECT`, ...) to `c.s.j.p.linux.Fcntl` - [@matthiasblaesing](https://github.com/matthiasblaesing). +* [#1521](https://github.com/java-native-access/jna/issues/1521): Shutdown CleanerThread once the last cleanable is removed - [@matthiasblaesing](https://github.com/matthiasblaesing). Bug Fixes --------- diff --git a/src/com/sun/jna/internal/Cleaner.java b/src/com/sun/jna/internal/Cleaner.java index 7d0ec8e28e..3a1b0eac54 100644 --- a/src/com/sun/jna/internal/Cleaner.java +++ b/src/com/sun/jna/internal/Cleaner.java @@ -45,35 +45,11 @@ public static Cleaner getCleaner() { } private final ReferenceQueue referenceQueue; - private final Thread cleanerThread; + private Thread cleanerThread; private CleanerRef firstCleanable; private Cleaner() { referenceQueue = new ReferenceQueue(); - cleanerThread = new Thread() { - @Override - public void run() { - while(true) { - try { - Reference ref = referenceQueue.remove(); - if(ref instanceof CleanerRef) { - ((CleanerRef) ref).clean(); - } - } catch (InterruptedException ex) { - // Can be raised on shutdown. If anyone else messes with - // our reference queue, well, there is no way to separate - // the two cases. - // https://groups.google.com/g/jna-users/c/j0fw96PlOpM/m/vbwNIb2pBQAJ - break; - } catch (Exception ex) { - Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex); - } - } - } - }; - cleanerThread.setName("JNA Cleaner"); - cleanerThread.setDaemon(true); - cleanerThread.start(); } public synchronized Cleanable register(Object obj, Runnable cleanupTask) { @@ -83,34 +59,43 @@ public synchronized Cleanable register(Object obj, Runnable cleanupTask) { } private synchronized CleanerRef add(CleanerRef ref) { - if(firstCleanable == null) { - firstCleanable = ref; - } else { - ref.setNext(firstCleanable); - firstCleanable.setPrevious(ref); - firstCleanable = ref; + synchronized (referenceQueue) { + if (firstCleanable == null) { + firstCleanable = ref; + } else { + ref.setNext(firstCleanable); + firstCleanable.setPrevious(ref); + firstCleanable = ref; + } + if (cleanerThread == null) { + Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Starting CleanerThread"); + cleanerThread = new CleanerThread(); + cleanerThread.start(); + } + return ref; } - return ref; } private synchronized boolean remove(CleanerRef ref) { - boolean inChain = false; - if(ref == firstCleanable) { - firstCleanable = ref.getNext(); - inChain = true; - } - if(ref.getPrevious() != null) { - ref.getPrevious().setNext(ref.getNext()); - } - if(ref.getNext() != null) { - ref.getNext().setPrevious(ref.getPrevious()); - } - if(ref.getPrevious() != null || ref.getNext() != null) { - inChain = true; + synchronized (referenceQueue) { + boolean inChain = false; + if (ref == firstCleanable) { + firstCleanable = ref.getNext(); + inChain = true; + } + if (ref.getPrevious() != null) { + ref.getPrevious().setNext(ref.getNext()); + } + if (ref.getNext() != null) { + ref.getNext().setPrevious(ref.getPrevious()); + } + if (ref.getPrevious() != null || ref.getNext() != null) { + inChain = true; + } + ref.setNext(null); + ref.setPrevious(null); + return inChain; } - ref.setNext(null); - ref.setPrevious(null); - return inChain; } private static class CleanerRef extends PhantomReference implements Cleanable { @@ -125,6 +110,7 @@ public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue ref = referenceQueue.remove(CLEANER_LINGER_TIME); + if (ref instanceof CleanerRef) { + ((CleanerRef) ref).clean(); + } else if (ref == null) { + synchronized (referenceQueue) { + Logger logger = Logger.getLogger(Cleaner.class.getName()); + if (firstCleanable == null) { + cleanerThread = null; + logger.log(Level.FINE, "Shutting down CleanerThread"); + break; + } else if (logger.isLoggable(Level.FINER)) { + StringBuilder registeredCleaners = new StringBuilder(); + for(CleanerRef cleanerRef = firstCleanable; cleanerRef != null; cleanerRef = cleanerRef.next) { + if(registeredCleaners.length() != 0) { + registeredCleaners.append(", "); + } + registeredCleaners.append(cleanerRef.cleanupTask.toString()); + } + logger.log(Level.FINER, "Registered Cleaners: {0}", registeredCleaners.toString()); + } + } + } + } catch (InterruptedException ex) { + // Can be raised on shutdown. If anyone else messes with + // our reference queue, well, there is no way to separate + // the two cases. + // https://groups.google.com/g/jna-users/c/j0fw96PlOpM/m/vbwNIb2pBQAJ + break; + } catch (Exception ex) { + Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex); + } + } + } + } }