Skip to content

Commit d75749c

Browse files
pmconradpmconrad
committed
Re-implemented Cleaner
1 parent 6149093 commit d75749c

File tree

2 files changed

+177
-116
lines changed

2 files changed

+177
-116
lines changed

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

Lines changed: 173 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@
2727
import java.lang.ref.PhantomReference;
2828
import java.lang.ref.Reference;
2929
import java.lang.ref.ReferenceQueue;
30+
import java.util.Iterator;
31+
import java.util.Map;
32+
import java.util.concurrent.ConcurrentHashMap;
33+
import java.util.concurrent.atomic.AtomicBoolean;
34+
import java.util.concurrent.atomic.AtomicLong;
3035
import java.util.logging.Level;
3136
import java.util.logging.Logger;
3237

@@ -35,154 +40,209 @@
3540
* objects. It replaces the {@code Object#finalize} based resource deallocation
3641
* that is deprecated for removal from the JDK.
3742
*
38-
* <p><strong>This class is intented to be used only be JNA itself.</strong></p>
43+
* <p><strong>This class is intended to be used only be JNA itself.</strong></p>
3944
*/
4045
public class Cleaner {
41-
private static final Cleaner INSTANCE = new Cleaner();
42-
43-
public static Cleaner getCleaner() {
44-
return INSTANCE;
45-
}
46+
/* General idea:
47+
*
48+
* There's one Cleaner per thread, kept in a ThreadLocal static variable.
49+
* This instance handles all to-be-cleaned objects registered by this
50+
* thread. Whenever the thread registers another object, it first checks
51+
* if there are references in the queue and cleans them up, then continues
52+
* with the registration.
53+
*
54+
* This leaves two cases open, for which we employ a "Master Cleaner" and
55+
* a separate cleaning thread.
56+
* 1. If a long-lived thread registers some objects in the beginning, but
57+
* then stops registering more objects, the previously registered
58+
* objects will never be cleared.
59+
* 2. When a thread exists before all its registered objects have been
60+
* cleared, the ThreadLocal instance are lost, and so are the pending
61+
* objects.
62+
*
63+
* The Master Cleaner handles the first issue by regularly handling the
64+
* queues of the Cleaners registered with it.
65+
* The seconds issue is handled by registering the per-thread Cleaner
66+
* instances with the Master's reference queue.
67+
*/
68+
69+
private static class CleanerImpl {
70+
protected final ReferenceQueue<Object> referenceQueue = new ReferenceQueue<Object>();
71+
protected final Map<Long,CleanerRef> cleanables = new ConcurrentHashMap<Long,CleanerRef>();
72+
private final AtomicBoolean lock = new AtomicBoolean(false);
73+
74+
private void cleanQueue() {
75+
if (lock.compareAndSet(false, true)) {
76+
try {
77+
Reference<?> ref;
78+
while ((ref = referenceQueue.poll()) != null) {
79+
try {
80+
if (ref instanceof Cleanable) {
81+
((Cleanable) ref).clean();
82+
}
83+
} catch (RuntimeException ex) {
84+
Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex);
85+
}
86+
}
87+
} finally {
88+
lock.set(false);
89+
}
90+
}
91+
}
4692

47-
private final ReferenceQueue<Object> referenceQueue;
48-
private Thread cleanerThread;
49-
private CleanerRef firstCleanable;
93+
public Cleanable register(Object obj, Runnable cleanupTask) {
94+
cleanQueue();
95+
// The important side effect is the PhantomReference, that is yielded
96+
// after the referent is GCed
97+
return new CleanerRef(this, obj, referenceQueue, cleanupTask);
98+
}
5099

51-
private Cleaner() {
52-
referenceQueue = new ReferenceQueue<>();
53-
}
100+
protected void put(long n, CleanerRef ref) {
101+
cleanables.put(n, ref);
102+
}
54103

55-
public synchronized Cleanable register(Object obj, Runnable cleanupTask) {
56-
// The important side effect is the PhantomReference, that is yielded
57-
// after the referent is GCed
58-
return add(new CleanerRef(this, obj, referenceQueue, cleanupTask));
104+
protected boolean remove(long n) {
105+
return cleanables.remove(n) != null;
106+
}
59107
}
60108

61-
private synchronized CleanerRef add(CleanerRef ref) {
62-
synchronized (referenceQueue) {
63-
if (firstCleanable == null) {
64-
firstCleanable = ref;
65-
} else {
66-
ref.setNext(firstCleanable);
67-
firstCleanable.setPrevious(ref);
68-
firstCleanable = ref;
69-
}
70-
if (cleanerThread == null) {
71-
Logger.getLogger(Cleaner.class.getName()).log(Level.FINE, "Starting CleanerThread");
72-
cleanerThread = new CleanerThread();
73-
cleanerThread.start();
74-
}
75-
return ref;
109+
private static class MasterCleanerImpl extends CleanerImpl {
110+
@Override
111+
protected synchronized void put(long n, CleanerRef ref) {
112+
super.put(n, ref);
76113
}
77-
}
78114

79-
private synchronized boolean remove(CleanerRef ref) {
80-
synchronized (referenceQueue) {
81-
boolean inChain = false;
82-
if (ref == firstCleanable) {
83-
firstCleanable = ref.getNext();
84-
inChain = true;
85-
}
86-
if (ref.getPrevious() != null) {
87-
ref.getPrevious().setNext(ref.getNext());
88-
}
89-
if (ref.getNext() != null) {
90-
ref.getNext().setPrevious(ref.getPrevious());
91-
}
92-
if (ref.getPrevious() != null || ref.getNext() != null) {
93-
inChain = true;
94-
}
95-
ref.setNext(null);
96-
ref.setPrevious(null);
97-
return inChain;
115+
@Override
116+
protected synchronized boolean remove(long n) {
117+
return super.remove(n);
98118
}
99119
}
100120

101-
private static class CleanerRef extends PhantomReference<Object> implements Cleanable {
102-
private final Cleaner cleaner;
103-
private final Runnable cleanupTask;
104-
private CleanerRef previous;
105-
private CleanerRef next;
121+
public static class MasterCleaner extends Cleaner {
122+
public static final long CLEANUP_INTERVAL_MS = 5000;
123+
public static final long MAX_LINGER_MS = 30000;
106124

107-
public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue<? super Object> q, Runnable cleanupTask) {
108-
super(referent, q);
109-
this.cleaner = cleaner;
110-
this.cleanupTask = cleanupTask;
125+
private static MasterCleaner INSTANCE;
126+
127+
public static synchronized void add(Cleaner cleaner) {
128+
if (INSTANCE == null) {
129+
INSTANCE = new MasterCleaner();
130+
}
131+
final CleanerImpl impl = cleaner.impl;
132+
INSTANCE.cleanerImpls.put(impl, true);
133+
INSTANCE.register(cleaner, new Runnable() {
134+
@Override
135+
public void run() {
136+
INSTANCE.cleanerImpls.put(impl, false);
137+
}
138+
});
111139
}
112140

113-
@Override
114-
public void clean() {
115-
if(cleaner.remove(this)) {
116-
cleanupTask.run();
141+
private static synchronized boolean deleteIfEmpty() {
142+
if (INSTANCE != null && INSTANCE.cleanerImpls.isEmpty()) {
143+
INSTANCE = null;
144+
return true;
117145
}
146+
return false;
118147
}
119148

120-
CleanerRef getPrevious() {
121-
return previous;
149+
private final Map<CleanerImpl,Boolean> cleanerImpls = new ConcurrentHashMap<CleanerImpl,Boolean>();
150+
private long lastNonEmpty = System.currentTimeMillis();
151+
152+
private MasterCleaner() {
153+
super(true);
154+
Thread cleanerThread = new Thread() {
155+
@Override
156+
public void run() {
157+
long now;
158+
while ((now = System.currentTimeMillis()) < lastNonEmpty + MAX_LINGER_MS || !deleteIfEmpty()) {
159+
if (!cleanerImpls.isEmpty()) { lastNonEmpty = now; }
160+
try {
161+
Reference<?> ref = impl.referenceQueue.remove(CLEANUP_INTERVAL_MS);
162+
if(ref instanceof CleanerRef) {
163+
((CleanerRef) ref).clean();
164+
} else {
165+
masterCleanup();
166+
}
167+
} catch (InterruptedException ex) {
168+
// Can be raised on shutdown. If anyone else messes with
169+
// our reference queue, well, there is no way to separate
170+
// the two cases.
171+
// https://groups.google.com/g/jna-users/c/j0fw96PlOpM/m/vbwNIb2pBQAJ
172+
break;
173+
} catch (Exception ex) {
174+
Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex);
175+
}
176+
}
177+
}
178+
};
179+
cleanerThread.setName("JNA Cleaner");
180+
cleanerThread.setDaemon(true);
181+
cleanerThread.start();
122182
}
123183

124-
void setPrevious(CleanerRef previous) {
125-
this.previous = previous;
184+
private void masterCleanup() {
185+
Iterator<Map.Entry<CleanerImpl,Boolean>> it = cleanerImpls.entrySet().iterator();
186+
while (it.hasNext()) {
187+
Map.Entry<CleanerImpl,Boolean> entry = it.next();
188+
entry.getKey().cleanQueue();
189+
if (!entry.getValue() && entry.getKey().cleanables.isEmpty()) {
190+
it.remove();
191+
}
192+
}
126193
}
194+
}
127195

128-
CleanerRef getNext() {
129-
return next;
196+
private static final ThreadLocal<Cleaner> MY_INSTANCE = new ThreadLocal<Cleaner>() {
197+
@Override
198+
protected Cleaner initialValue() {
199+
return new Cleaner(false);
130200
}
201+
};
131202

132-
void setNext(CleanerRef next) {
133-
this.next = next;
203+
public static Cleaner getCleaner() {
204+
return MY_INSTANCE.get();
205+
}
206+
207+
protected final CleanerImpl impl;
208+
209+
private Cleaner(boolean master) {
210+
if (master) {
211+
impl = new MasterCleanerImpl();
212+
} else {
213+
impl = new CleanerImpl();
214+
MasterCleaner.add(this);
134215
}
135216
}
136217

137-
public static interface Cleanable {
138-
public void clean();
218+
public Cleanable register(Object obj, Runnable cleanupTask) {
219+
return impl.register(obj, cleanupTask);
139220
}
140221

141-
private class CleanerThread extends Thread {
222+
private static class CleanerRef extends PhantomReference<Object> implements Cleanable {
223+
private static final AtomicLong COUNTER = new AtomicLong(Long.MIN_VALUE);
142224

143-
private static final long CLEANER_LINGER_TIME = 30000;
225+
private final CleanerImpl cleaner;
226+
private final long number = COUNTER.incrementAndGet();
227+
private Runnable cleanupTask;
144228

145-
public CleanerThread() {
146-
super("JNA Cleaner");
147-
setDaemon(true);
229+
public CleanerRef(CleanerImpl impl, Object referent, ReferenceQueue<Object> q, Runnable cleanupTask) {
230+
super(referent, q);
231+
this.cleaner = impl;
232+
this.cleanupTask = cleanupTask;
233+
cleaner.put(number, this);
148234
}
149235

150236
@Override
151-
public void run() {
152-
while (true) {
153-
try {
154-
Reference<? extends Object> ref = referenceQueue.remove(CLEANER_LINGER_TIME);
155-
if (ref instanceof CleanerRef) {
156-
((CleanerRef) ref).clean();
157-
} else if (ref == null) {
158-
synchronized (referenceQueue) {
159-
Logger logger = Logger.getLogger(Cleaner.class.getName());
160-
if (firstCleanable == null) {
161-
cleanerThread = null;
162-
logger.log(Level.FINE, "Shutting down CleanerThread");
163-
break;
164-
} else if (logger.isLoggable(Level.FINER)) {
165-
StringBuilder registeredCleaners = new StringBuilder();
166-
for(CleanerRef cleanerRef = firstCleanable; cleanerRef != null; cleanerRef = cleanerRef.next) {
167-
if(registeredCleaners.length() != 0) {
168-
registeredCleaners.append(", ");
169-
}
170-
registeredCleaners.append(cleanerRef.cleanupTask.toString());
171-
}
172-
logger.log(Level.FINER, "Registered Cleaners: {0}", registeredCleaners.toString());
173-
}
174-
}
175-
}
176-
} catch (InterruptedException ex) {
177-
// Can be raised on shutdown. If anyone else messes with
178-
// our reference queue, well, there is no way to separate
179-
// the two cases.
180-
// https://groups.google.com/g/jna-users/c/j0fw96PlOpM/m/vbwNIb2pBQAJ
181-
break;
182-
} catch (Exception ex) {
183-
Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex);
184-
}
237+
public void clean() {
238+
if(cleaner.remove(this.number) && cleanupTask != null) {
239+
cleanupTask.run();
240+
cleanupTask = null;
185241
}
186242
}
187243
}
244+
245+
public static interface Cleanable {
246+
public void clean();
247+
}
188248
}

‎test/com/sun/jna/CallbacksTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
import com.sun.jna.Callback.UncaughtExceptionHandler;
4040
import com.sun.jna.CallbacksTest.TestLibrary.CbCallback;
41+
import com.sun.jna.internal.Cleaner;
4142
import com.sun.jna.ptr.IntByReference;
4243
import com.sun.jna.ptr.PointerByReference;
4344
import com.sun.jna.win32.W32APIOptions;
@@ -362,7 +363,7 @@ public void callback() {
362363

363364
cb = null;
364365
System.gc();
365-
for (int i = 0; i < 100 && (ref.get() != null || refs.containsValue(ref)); ++i) {
366+
for (int i = 0; i < Cleaner.MasterCleaner.CLEANUP_INTERVAL_MS / 10 + 5 && (ref.get() != null || refs.containsValue(ref)); ++i) {
366367
Thread.sleep(10); // Give the GC a chance to run
367368
System.gc();
368369
}
@@ -371,11 +372,11 @@ public void callback() {
371372

372373
ref = null;
373374
System.gc();
374-
for (int i = 0; i < 100 && (cbstruct.peer != 0 || refs.size() > 0); ++i) {
375+
for (int i = 0; i < Cleaner.MasterCleaner.CLEANUP_INTERVAL_MS / 10 + 5 && (cbstruct.peer != 0 || refs.size() > 0); ++i) {
375376
// Flush weak hash map
376377
refs.size();
377378
try {
378-
Thread.sleep(10); // Give the GC a chance to run
379+
Thread.sleep(Cleaner.MasterCleaner.CLEANUP_INTERVAL_MS + 10); // Give the GC a chance to run
379380
System.gc();
380381
} finally {}
381382
}

0 commit comments

Comments
 (0)