Skip to content

Commit 4a76b10

Browse files
pmconradpmconrad
committed
Re-implemented Cleaner
1 parent 542ab54 commit 4a76b10

File tree

2 files changed

+179
-85
lines changed

2 files changed

+179
-85
lines changed

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

+175-82
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,117 +40,205 @@
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();
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+
}
4292

43-
public static Cleaner getCleaner() {
44-
return INSTANCE;
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+
}
99+
100+
protected void put(long n, CleanerRef ref) {
101+
cleanables.put(n, ref);
102+
}
103+
104+
protected boolean remove(long n) {
105+
return cleanables.remove(n) != null;
106+
}
45107
}
46108

47-
private final ReferenceQueue<Object> referenceQueue;
48-
private final Thread cleanerThread;
49-
private CleanerRef firstCleanable;
50-
51-
private Cleaner() {
52-
referenceQueue = new ReferenceQueue<Object>();
53-
cleanerThread = new Thread() {
54-
@Override
55-
public void run() {
56-
while(true) {
57-
try {
58-
Reference<? extends Object> ref = referenceQueue.remove();
59-
if(ref instanceof CleanerRef) {
60-
((CleanerRef) ref).clean();
109+
private static class MasterCleanerImpl extends CleanerImpl {
110+
@Override
111+
protected synchronized void put(long n, CleanerRef ref) {
112+
super.put(n, ref);
113+
}
114+
115+
@Override
116+
protected synchronized boolean remove(long n) {
117+
return super.remove(n);
118+
}
119+
}
120+
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;
124+
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+
});
139+
}
140+
141+
private static synchronized boolean deleteIfEmpty() {
142+
if (INSTANCE != null && INSTANCE.cleanerImpls.isEmpty()) {
143+
INSTANCE = null;
144+
return true;
145+
}
146+
return false;
147+
}
148+
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);
61175
}
62-
} catch (InterruptedException ex) {
63-
// Can be raised on shutdown. If anyone else messes with
64-
// our reference queue, well, there is no way to separate
65-
// the two cases.
66-
// https://groups.google.com/g/jna-users/c/j0fw96PlOpM/m/vbwNIb2pBQAJ
67-
break;
68-
} catch (Exception ex) {
69-
Logger.getLogger(Cleaner.class.getName()).log(Level.SEVERE, null, ex);
70176
}
71177
}
178+
};
179+
cleanerThread.setName("JNA Cleaner");
180+
cleanerThread.setDaemon(true);
181+
cleanerThread.start();
182+
}
183+
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+
}
72192
}
73-
};
74-
cleanerThread.setName("JNA Cleaner");
75-
cleanerThread.setDaemon(true);
76-
cleanerThread.start();
193+
}
77194
}
78195

79-
public synchronized Cleanable register(Object obj, Runnable cleanupTask) {
80-
// The important side effect is the PhantomReference, that is yielded
81-
// after the referent is GCed
82-
return add(new CleanerRef(this, obj, referenceQueue, cleanupTask));
196+
private static final ThreadLocal<Cleaner> MY_INSTANCE = new ThreadLocal<Cleaner>() {
197+
@Override
198+
protected Cleaner initialValue() {
199+
return new Cleaner(false);
200+
}
201+
};
202+
203+
public static Cleaner getCleaner() {
204+
return MY_INSTANCE.get();
83205
}
84206

85-
private synchronized CleanerRef add(CleanerRef ref) {
86-
if(firstCleanable == null) {
87-
firstCleanable = ref;
207+
protected final CleanerImpl impl;
208+
209+
private Cleaner(boolean master) {
210+
if (master) {
211+
impl = new MasterCleanerImpl();
88212
} else {
89-
ref.setNext(firstCleanable);
90-
firstCleanable.setPrevious(ref);
91-
firstCleanable = ref;
213+
impl = new CleanerImpl();
214+
MasterCleaner.add(this);
92215
}
93-
return ref;
94216
}
95217

96-
private synchronized boolean remove(CleanerRef ref) {
97-
boolean inChain = false;
98-
if(ref == firstCleanable) {
99-
firstCleanable = ref.getNext();
100-
inChain = true;
101-
}
102-
if(ref.getPrevious() != null) {
103-
ref.getPrevious().setNext(ref.getNext());
104-
}
105-
if(ref.getNext() != null) {
106-
ref.getNext().setPrevious(ref.getPrevious());
107-
}
108-
if(ref.getPrevious() != null || ref.getNext() != null) {
109-
inChain = true;
110-
}
111-
ref.setNext(null);
112-
ref.setPrevious(null);
113-
return inChain;
218+
public Cleanable register(Object obj, Runnable cleanupTask) {
219+
return impl.register(obj, cleanupTask);
114220
}
115221

116222
private static class CleanerRef extends PhantomReference<Object> implements Cleanable {
117-
private final Cleaner cleaner;
118-
private final Runnable cleanupTask;
119-
private CleanerRef previous;
120-
private CleanerRef next;
223+
private static final AtomicLong COUNTER = new AtomicLong(Long.MIN_VALUE);
224+
225+
private final CleanerImpl cleaner;
226+
private final long number = COUNTER.incrementAndGet();
227+
private Runnable cleanupTask;
121228

122-
public CleanerRef(Cleaner cleaner, Object referent, ReferenceQueue<? super Object> q, Runnable cleanupTask) {
229+
public CleanerRef(CleanerImpl impl, Object referent, ReferenceQueue<Object> q, Runnable cleanupTask) {
123230
super(referent, q);
124-
this.cleaner = cleaner;
231+
this.cleaner = impl;
125232
this.cleanupTask = cleanupTask;
233+
cleaner.put(number, this);
126234
}
127235

128236
public void clean() {
129-
if(cleaner.remove(this)) {
237+
if(cleaner.remove(this.number) && cleanupTask != null) {
130238
cleanupTask.run();
239+
cleanupTask = null;
131240
}
132241
}
133-
134-
CleanerRef getPrevious() {
135-
return previous;
136-
}
137-
138-
void setPrevious(CleanerRef previous) {
139-
this.previous = previous;
140-
}
141-
142-
CleanerRef getNext() {
143-
return next;
144-
}
145-
146-
void setNext(CleanerRef next) {
147-
this.next = next;
148-
}
149242
}
150243

151244
public static interface Cleanable {

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

+4-3
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)