> On Feb 23, 2016, at 11:35 AM, Peter Levart <peter.lev...@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. ------------------------------------------------------------------------------