Replaced the Disposer

src/org/adblockplus/libadblockplus/Disposer.java

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 package org.adblockplus.libadblockplus;
 
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
 import java.util.HashSet;
 
 public final class Disposer extends WeakReference<Disposable>
 {
-  final static ReferenceQueue<Disposable> referenceQueue = new ReferenceQueue<Disposable>();
-  final static HashSet<Disposer> disposerSet = new HashSet<Disposer>();

[Felix Dahlke, 2014/04/30 15:34:08]

Should be "static final".

[René Jeschke, 2014/04/30 15:43:16]

Done.

+  static final ReferenceQueue<Disposable> referenceQueue = new ReferenceQueue<Disposable>();
+  private static final HashSet<Disposer> disposerSet = new HashSet<Disposer>();
   private final Disposable disposable;
-  private volatile boolean isDisposed = false;

[Felix Dahlke, 2014/04/30 15:34:08]

Should be "disposed" according to the bean convention IMO. "isDisposed" would be
the name of the getter.

[René Jeschke, 2014/04/30 15:43:16]

Done.

+  private volatile boolean disposed = false;
 
   static
   {
     final Thread thread = new Thread(new Cleaner());
     thread.setName(Cleaner.class.getCanonicalName());
     thread.setDaemon(true);
     thread.start();
   }
 
   public Disposer(final Disposable referent, final Disposable disposable)

[Felix Dahlke, 2014/04/30 15:34:08]

How about renaming "disposable" to "reference"? That'd be a bit more clear IMO.

[René Jeschke, 2014/04/30 15:43:16]

'referent' is the reference that get stored in the ReferenceQueue to track when
'referent' is only weakly reachable.

'disposable' is the object that actually does the disposing.

So, IMO: both names are correct.

[Felix Dahlke, 2014/04/30 15:50:43]

Isn't disposable the object that _gets_ disposed once the referent is GCed?
Anyway, yeah, "reference" is probably too ambiguous here. We could call
"referent" "reference" and it'd still make sense.

   {
     super(referent, referenceQueue);

[Felix Dahlke, 2014/04/30 15:34:08]

Shouldn't it be the referentQueue if it stores the referents?

[René Jeschke, 2014/04/30 15:43:16]

It stores the value from a local variable called 'referent', yep, but it stores
and manages 'WeakReference's, that's why it's a 'ReferenceQueue' and that's also
why the member is called 'referenceQueue'.

     this.disposable = disposable;
 
     synchronized (disposerSet)
     {
       disposerSet.add(this);
     }
   }
 
   public synchronized void dispose()
   {
-    if (!this.isDisposed)
+    if (!this.disposed)
     {
       try
       {
         this.disposable.dispose();
       }
       catch (final Throwable t)
       {
         // catch to set state to 'disposed' on all circumstances
       }
 
-      this.isDisposed = true;
+      this.disposed = true;
       synchronized (disposerSet)
       {
         disposerSet.remove(this);
       }
     }
   }
 
   private static final class Cleaner implements Runnable
   {
     public Cleaner()
@@ skipping 11 matching lines @@
           ((Disposer) Disposer.referenceQueue.remove()).dispose();
         }
         catch (final Throwable t)
         {
           // ignored
         }
       }
     }
   }
 }