Skip to content

fixes #1616 tighten up synchronization and conditional logic #1688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+55 −58
Open

fixes #1616 tighten up synchronization and conditional logic #1688

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 55 additions & 58 deletions src/com/sun/jna/internal/Cleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.lang.ref.PhantomReference;
import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand All @@ -35,84 +36,80 @@
* objects. It replaces the {@code Object#finalize} based resource deallocation
* that is deprecated for removal from the JDK.
*
* <p><strong>This class is intented to be used only be JNA itself.</strong></p>
* <p><strong>This class is intended to be used only be JNA itself.</strong></p>
*/
public class Cleaner {
private static final Cleaner INSTANCE = new Cleaner();
private static final Logger logger = Logger.getLogger(Cleaner.class.getName());
private static final long CLEANER_LINGER_TIME = TimeUnit.SECONDS.toMillis(30);

public static Cleaner getCleaner() {
return INSTANCE;
}

private final ReferenceQueue<Object> referenceQueue;
private Thread cleanerThread;
private boolean cleanerRunning;
private CleanerRef firstCleanable;

private Cleaner() {
referenceQueue = new ReferenceQueue<>();
}

public synchronized Cleanable register(Object obj, Runnable cleanupTask) {
// The important side effect is the PhantomReference, that is yielded
// after the referent is GCed
return add(new CleanerRef(this, obj, referenceQueue, cleanupTask));
}
public synchronized Cleanable register(final Object obj, final Runnable cleanupTask) {
// The important side effect is the PhantomReference, that is yielded after the referent is GCed
final CleanerRef ref = new CleanerRef(obj, referenceQueue, cleanupTask);

private synchronized CleanerRef add(CleanerRef 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;
if (firstCleanable != null) {
ref.setNext(firstCleanable);
firstCleanable.setPrevious(ref);
}
firstCleanable = ref;

if (!cleanerRunning) {
logger.log(Level.FINE, "Starting CleanerThread");
Thread cleanerThread = new CleanerThread();
cleanerThread.start();
cleanerRunning = true;
}

return ref;
}

private synchronized boolean remove(CleanerRef ref) {
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;
private synchronized boolean remove(final CleanerRef ref) {
final CleanerRef prev = ref.getPrevious();
final CleanerRef next = ref.getNext();
boolean inChain = false;

if (ref == firstCleanable) {
firstCleanable = next;
inChain = true;
}
if (prev != null) {
prev.setNext(next);
inChain = true;
}
if (next != null) {
next.setPrevious(prev);
inChain = true;
}
return inChain;
}

private static class CleanerRef extends PhantomReference<Object> implements Cleanable {
private final Cleaner cleaner;
private final Runnable cleanupTask;
private CleanerRef previous;
private CleanerRef next;

public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue<? super Object> q, Runnable cleanupTask) {
super(referent, q);
this.cleaner = cleaner;
public CleanerRef(final Object referent, final ReferenceQueue<? super Object> queue, final Runnable cleanupTask) {
super(referent, queue);
this.cleanupTask = cleanupTask;
}

@Override
public void clean() {
if(cleaner.remove(this)) {
if (INSTANCE.remove(this)) {
previous = null;
next = null;
cleanupTask.run();
}
}
Expand All @@ -121,27 +118,25 @@ CleanerRef getPrevious() {
return previous;
}

void setPrevious(CleanerRef previous) {
void setPrevious(final CleanerRef previous) {
this.previous = previous;
}

CleanerRef getNext() {
return next;
}

void setNext(CleanerRef next) {
void setNext(final CleanerRef next) {
this.next = next;
}
}

public static interface Cleanable {
public void clean();
public interface Cleanable {
void clean();
}

private class CleanerThread extends Thread {

private static final long CLEANER_LINGER_TIME = 30000;

public CleanerThread() {
super("JNA Cleaner");
setDaemon(true);
Expand All @@ -151,20 +146,19 @@ public CleanerThread() {
public void run() {
while (true) {
try {
Reference<? extends Object> ref = referenceQueue.remove(CLEANER_LINGER_TIME);
Reference<?> 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());
synchronized (INSTANCE) {
if (firstCleanable == null) {
cleanerThread = null;
logger.log(Level.FINE, "Shutting down CleanerThread");
cleanerRunning = false;
break;
} else if (logger.isLoggable(Level.FINER)) {
StringBuilder registeredCleaners = new StringBuilder();
for(CleanerRef cleanerRef = firstCleanable; cleanerRef != null; cleanerRef = cleanerRef.next) {
if(registeredCleaners.length() != 0) {
for (CleanerRef cleanerRef = firstCleanable; cleanerRef != null; cleanerRef = cleanerRef.next) {
if (registeredCleaners.length() != 0) {
registeredCleaners.append(", ");
}
registeredCleaners.append(cleanerRef.cleanupTask.toString());
Expand All @@ -178,6 +172,9 @@ public void run() {
// our reference queue, well, there is no way to separate
// the two cases.
// https://groups.google.com/g/jna-users/c/j0fw96PlOpM/m/vbwNIb2pBQAJ
synchronized (INSTANCE) {
cleanerRunning = false;
}
break;
} catch (Exception ex) {
Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex);
Expand Down
Loading