RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization
Roger Riggs
Roger.Riggs at Oracle.com
Tue Oct 6 18:20:55 UTC 2015
Hi Peter,
Well spotted, the timeout is the easiest solution, and it will happen
infrequently.
The timeout can be quite long to avoid a 'polling' tax. Perhaps 60 seconds.
Thanks, Roger
On 10/6/2015 12:06 PM, Peter Levart wrote:
> Hi Roger (again, sorry)!
>
> I think I found a race in the implementation which could leave the
> Cleaner thread run forever:
>
> 238 public void run() {
> 239 Thread t = Thread.currentThread();
> 240 ManagedLocalsThread mlThread = (t instanceof
> ManagedLocalsThread)
> 241 ? (ManagedLocalsThread)t
> 242 : null;
> 243 while (!phantomCleanableList.isEmpty() ||
> 244 !weakCleanableList.isEmpty() ||
> 245 !softCleanableList.isEmpty()) {
> 246 if (mlThread != null) {
> 247 // Cleanable the thread locals
> 248 mlThread.eraseThreadLocals();
> 249 }
> 250 try {
> 251 Cleanable ref = (Cleanable) queue.remove();
> 252 ref.clean();
> 253 } catch (InterruptedException i) {
> 254 continue; // ignore the interruption
> 255 } catch (Throwable e) {
> 256 // ignore exceptions from the cleanup thunk
> 257 }
> 258 }
> 259 }
>
> Suppose the Cleaner instance has already been collected and it's
> associated PhantomCleanableRef has already been removed from
> phantomCleanableList. Let weakCleanableList and softCleanableList be
> (already) empty too. Let there be some last PhantomCleanableRefefence
> in the phantomCleanableList. while condition is therefore true, so the
> Cleaner thread enters loop and blocks at queue.remove(). Now some user
> thread clean()s or clear()s the remaining PhantomCleanableRefefence
> before it's referent is found phantom reachable by GC and removes the
> Reference from the list. This reference will never be enqueued by
> GC+ReferenceHandler, so queue.remove() will block forever.
>
> The simplest fix would be to use a remove() with a timeout in this
> situation.
>
> Regards, Peter
>
> On 10/06/2015 12:11 AM, Roger Riggs wrote:
>> Hi,
>>
>> Building on Peter's code to provide subclassable CleanableReferences,
>> I rearranged
>> the implementation a bit to make it easier to maintain.
>> The CleanableReferences subclasses are nested inside the Cleaner to
>> make the scoping clear.
>>
>> Please review and comment:
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-cleaner-extensible-8138696/
>>
>> javadoc:
>> http://cr.openjdk.java.net/~rriggs/cleaner-doc2/
>>
>> Thanks, Roger
>>
>>
>>
>> On 10/2/2015 11:34 AM, Roger Riggs wrote:
>>> Hi Peter,
>>>
>>> Great comments, thanks for looking into this...
>>>
>>> On 10/2/2015 7:52 AM, Peter Levart wrote:
>>>> Hi Roger,
>>>>
>>>> This is a nice and clean API. I like the trick with embedding a
>>>> private CleanerImpl in Cleaner and registering Cleaner itself as a
>>>> tracked object so that automatic thread termination can be
>>>> performed in a safe manner. You say the scope of this is not to
>>>> replace internal usages of sun.misc.Cleaner. But if they ever get
>>>> replaced, there are two differences between the implementations to
>>>> note:
>>>>
>>>> - sun.misc.Cleaner thunks get executed by a ReferenceHandler thread
>>>> directly and bypass ReferenceQueue-ing. This might be OK for
>>>> internal use, but not appropriate for public API. I doubt this
>>>> sun.misc.Cleaner exception in ReferenceHandler is necessary though.
>>>> - With sun.misc.Cleaner one has to create a minimum of 2 additional
>>>> objects per tracked object: the Cleaner object and a Runnable
>>>> thunk. This API requires creation of 3 objects per tracked object:
>>>> the Cleanup object, the internal Reference object and a Runnable
>>>> thunk.
>>>>
>>>> Do you think Cleaner will not be used in scenarios where this
>>>> additional footprint matters?
>>> I don't have any particular data on that point. When used with
>>> lambda or method references for
>>> the thunk, it is likely there will be some binding overhead.
>>>>
>>>> It might be possible to merge the roles of Cleanup and Reference
>>>> into one object, like this:
>>>>
>>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner/Cleaner.java
>>>>
>>>> Making Cleanup an interface, internal Reference(s) can implement
>>>> it. One can cast a Cleanup instance to a Reference and invoke it's
>>>> other methods, but this can be prevented by throwing
>>>> UnsupportedOperationException from them, so nobody is tempted to
>>>> employ this implementation detail.
>>>
>>> I prototyped a similar implementation but backed it out due to the
>>> code duplication and complexity.
>>> It also seemed a poor choice to break the contract of Reference by
>>> throwing UnsupportedOperationException
>>> on the unneeded but exposed methods since they could not be
>>> completely encapsulated.
>>> It seemed cleaner to have only a concrete type that was exposed to
>>> the clients.
>>>
>>> BTW, the code in you cleaner might be simpler if the cleaner lists
>>> for each of the three types were separate.
>>> (The entries don't need to be intermixed). That could simplify the
>>> virtual next/prev access but would still be
>>> duplicating the linked list management code).
>>> There could be multiple insertXXX methods so the casts were not
>>> necessary.
>>>
>>>>
>>>> There might be utility in exposing Cleanup (or better named
>>>> Cleanable) References as public abstract classes so that a
>>>> footprint sensitive application (for example in a data structure
>>>> with many elements) is possible. By subclassing such an abstract
>>>> reference and implementing the abstract method, a single object is
>>>> required per tracked object. Here I derived a proof of concept from
>>>> your code:
>>>>
>>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Cleaner/webrev.01/
>>>>
>>>> The simple API is unchanged and there is additional low-level API
>>>> that can be used if footprint matters.
>>>>
>>>> What do you think?
>>>
>>> I see what you're getting at. I would probably try to rearrange it
>>> so that only the performMethod
>>> could be overridden. Overriding clean() and clear() might disable
>>> the list management
>>> and reduce the robustness.
>>>
>>> I'll take another look, I was most concerned with simplicity, but if
>>> the overhead for the number
>>> of the objects is a concern that can be traded off against a more
>>> complex implementation.
>>>
>>> Thanks, Roger
>>>
>>>>
>>>> Regards, Peter
>>>>
>>>>
>>>> On 10/01/2015 04:12 PM, Roger Riggs wrote:
>>>>> Please review a proposal for public Cleaner API:
>>>>>
>>>>> A Cleaner is proposed to provide an easy to use alternative to
>>>>> finalization. The service would provide easy registration and
>>>>> cancellation of cleanup functions for objects. Applications create
>>>>> a cleanup service for their own use and the service terminates
>>>>> when it is no longer in use.
>>>>>
>>>>> Finalization has a long history of issues both in usage and
>>>>> performance. PhantomReferences have been proposed as the
>>>>> alternative GC based mechanism for cleaning functions but it has
>>>>> been left as an exercise to the developer to construct the
>>>>> necessary mechanisms to handle ReferenceQueues, handle threading
>>>>> issues and robust termination.
>>>>>
>>>>> The Cleaner performs cleaning functions when objects are
>>>>> unreachable as found by garbage collection using the existing
>>>>> mechanisms of PhantomReference, WeakReference, SoftReferences, and
>>>>> ReferenceQueues. It manages a thread that dequeues references to
>>>>> unreachable objects and invokes the corresponding cleaning
>>>>> function. Registered cleaning functions can be cleared if no
>>>>> longer needed, can be invoked explicitly to perform the cleanup
>>>>> immediately, or be invoked when the object is not reachable (as
>>>>> detected by garbage collection) and handled by a cleanup thread.
>>>>>
>>>>> The java.lang.ref package is proposed for the Cleaner because it
>>>>> is complementary to the reference classes and reference queues and
>>>>> to make it easy to find.
>>>>>
>>>>> It is not a goal to replace all uses of finalization or
>>>>> sun.misc.Cleaner in the JDK.
>>>>> Investigation will evaluate if and in what cases the Cleaner can
>>>>> replace finalization.
>>>>> A subsequent task will examine uses of finalization and propose
>>>>> specific changes
>>>>> on a case by base basis.
>>>>>
>>>>> Please review and comment:
>>>>>
>>>>> Javadoc:
>>>>> http://cr.openjdk.java.net/~rriggs/cleaner-doc/
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
>>>>>
>>>>> Issue:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8138696
>>>>>
>>>>> Thanks, Roger
>>>>>
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list