RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more

[Kim Barrett]

> On Feb 23, 2016, at 11:35 AM, Peter Levart <peter.levart at gmail.com> wrote:
> 
> Hi Roger, Mandy,
> 
> Here's another webrev:
> 
> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.05/
> 
> I renamed the method and reworded the specification. Is this better now?
> 
> 
> On 02/22/2016 10:56 PM, Roger Riggs wrote:
>> Hi Mandy,
>> 
>> On 2/22/2016 4:41 PM, Mandy Chung wrote:
>> 
>>> 
>>> The existing way to do that is to register phantom references in your own ReferenceQueue and then drain the queue at appropriate point.  Would you consider having a method to return ReferenceQueue maintained by the cleaner instead?
>> If the queue is exposed, there is no assurance that the cleanable function would be called.
>> Any caller could introduce a bug by not doing the proper cleaning.
>> 
>> I was more concerned with the crossing of Reference.tryHandlePending with the cleaning thread.
>> The method description does not mention anything related to the Reference processing thread
>> though that is all implementation.  The @implNote might be a bit more concise and less informal.
>> 
>> Roger
> 
> Yes, ReferenceHandler thread is just an implementation detail. The specification of java.lang.Reference subclasses doesn't even mention it. It talks about GC enqueueing Reference objects:
> 
> "Suppose that the garbage collector determines at a certain point in time...
> ...At the same time or at some later time it will enqueue those newly-cleared weak references that are registered with reference queues."
> 
> So in this respect ReferenceHandler is just an extension of GC Reference discovery/enqueuing logic, delegated to a background thread on Java side. The Cleaner.cleanNextPending() method tries to be true to its specification - it tries to invoke next cleanup action for which GC has determined that its monitored object is phantom reachable without waiting for ReferenceHandler thread to transfer it from pending list to the queue.
> 
> Since Reference.tryHandlePending is just transfering Reference objects from one list to another and does that using two locks (the Reference.lock and the ReferenceQueue.lock) but never holds them both together or calls any outside code while holding any of the locks, there's no danger of dead-locking, if that was your concern.
> 
> Regards, Peter

------------------------------------------------------------------------------ 
src/java.base/share/classes/java/lang/ref/Cleaner.java
 242     public boolean cleanNextPending() {
 243         while (!impl.cleanNextEnqueuedCleanable()) {
 244             if (!Reference.tryHandlePending(false)) {
 245                 return false;
 246             }
 247         }
 248         return true;
 249     }

This can loop for an arbitrarily long time if there are many pending
references that are unrelated to the invoking Cleaner.  It could even
loop arbitrarily long if there are lots of pending references and only
a few scattered ones are for the invoking Cleaner, because the
Cleaner's thread might take care of each of them before the helping
thread re-checks the queue.

The Disposer class that was mentioned as a use-case for this new
function doesn't try to do anything like that; it just loops until
there aren't any more in the queue.  The caller there is only affected
by the number of enqueued objects, not by the number of unrelated
pending objects.

OTOH, while not structured exactly the same, I think this has a
similar effect to the existing Direct-X-Buffer support provided by
Bits.  Helping the single reference processing thread could be useful,
though contention for the pending list lock could be a problem.  And
this isn't *quite* the same as what we have now, because the reference
will be enqueued and the Cleaner will be notified, so that the helper
and the Cleaner compete to process the queued cleanup; with
sun.misc.Cleaner the helper just directly invokes the cleanup.

I'm not sure what to do about any of this; I'm just feeling like maybe
the proposed API / implementation isn't quite right yet.

------------------------------------------------------------------------------ 
src/java.base/share/classes/java/nio/Direct-X-Buffer.java
  34 import jdk.internal.ref.CleanerFactory;
&etc.

It might make sense to give Direct-X-Buffer its own dedicated Cleaner,
rather than sharing the jdk.internal Cleaner.

------------------------------------------------------------------------------