RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use > alternative to finalization (Roger Riggs)

[Roger Riggs]

Hi Mike,

Thanks for the review and comments...

On 10/2/2015 3:33 PM, Mike Duigou wrote:
> Hi Roger;
>
> This looks like an interesting proposal and would be a good 
> replacement for alternative solutions.
>
> I am curious why the thread is run at max priority. Also, why not set 
> priority and daemon status in the factory?
Yes, that should probably be left to the factory and for built-in 
factory would be moved to the InnocuousThreadFactory.
>
> You should clear the Thread intrerruption if you are going to ignore 
> it. Thread.interrupted();
Will do.
>
> I would suggest surrounding the thunk.run() with a catch of Throwable 
> or Exception. In the current implementation one bad egg spoils the 
> party for everyone. There is a catch of RuntimeException described as 
> "catch (RuntimeException e) { // ignore exceptions from the cleanup 
> thunk }" but I would suggest that this is insufficiently paranoid.
Good point,  catches more bad eggs.
>
> Consider a pattern like:
>
> private static final ReferenceQueue<Object> weakListeners = new 
> ReferenceQueue<>();
>
>
> private static class CleanerThread extends Thread {
> { setDaemon(true); setPriority(Thread.MIN_PRIORITY); }
>     @Override
>     @SuppressWarnings("unchecked")
>     public void run() {
>     // Using weak ref to queue allows shutdown if class is unloaded
>     WeakReference<ReferenceQueue<Object>> weakQueue = new 
> WeakReference<>(weakListeners);
>     ReferenceQueue<Object> queue = null;
>     while((queue = (queue == null ? weakQueue.get() : queue) != null) 
> try {
>       Object dead = queue.remove(10000);
>       if(dead != null) {
>          // do stuff with "dead"
>       } else {
>         queue = null; // recheck to see if queue is gone.
>       }
>     } catch(InterruptedException woken) {
>       Thread.interrupted();
>     }
>   }
> }
>
> When 'weakListeners' is cleared when the containing class is unloaded 
> then the thread will shut down after roughly 10 seconds.
>
> I have been meaning to check whether CleanerThread needs to be in a 
> separate class--an inner class, even a static one, may prevent it's 
> containing class from being GCed. IDK.
I think that 'nested' classes aka  static class has no links to the 
outer class and can be GC'ed but will check.

Thanks, Roger