RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization

[Roger Riggs]

Hi Peter,

I've always assumed that the arguments could not be gc'd at least until 
the method returns
in part because the caller was holding references.  But I suppose in a 
completely inlined
case an optimizer might try to make more relaxed conclusions.

Since the new object has not yet been published, the reachability of 
objects in a constructor
must have some additional rules to keep objects alive until it is published.


On 10/21/2015 8:13 AM, Peter Levart wrote:
> Not so fast, there's more...
>
> On 10/21/2015 01:50 PM, Peter Levart wrote:
>> Hi Roger,
>>
>> I think I have another race ;-)
>>
>> This time it is more theoretical than practical. If we take a look at 
>> say PhantomCleanable constructor:
>>
>>         public PhantomCleanable(T referent, Cleaner cleaner) {
>>             super(Objects.requireNonNull(referent), 
>> getCleanerImpl(cleaner).queue);
>>             this.cleanerImpl = getCleanerImpl(cleaner);
>>             insert();
>>         }
>>
>> ...somewhere in the getCleanerImpl(cleaner) method where 'cleaner' is 
>> last dereferenced and before assignment of extracted CleanerImpl to 
>> this.cleanerImpl, 'cleaner' is not needed any more. If it is not 
>> referenced from anywhere else in the program and JVM can prove it 
>> will not be used any more, JVM is free to declare it phantom 
>> reachable. The PhantomCleanable that is used to track the 'cleaner' 
>> can therefore be enqueued and the cleaner thread can process it, 
>> effectively emptying the 'phantomCleanableList' before this 
>> PhantomCleanable constructor insert()s itself into the 
>> 'phantomCleanableList'. The cleaner thread can therefore exit the 
>> loop and terminate prematurely.
>>
>> The fix would be to reverse the last two statements of the 
>> XxxCleanable constructor(s):
>>
>>         public PhantomCleanable(T referent, Cleaner cleaner) {
>>             super(Objects.requireNonNull(referent), 
>> getCleanerImpl(cleaner).queue);
>>             insert();
>>             this.cleanerImpl = getCleanerImpl(cleaner);
Except that insert() uses cleanerImpl to know what list to insert it in.
(It could be passed as an argument; if the concern about the Cleaner 
becoming phantomReachable at that moment was valid)
>>         }
>>
>> ... or to add a call to the (not yet in jdk9 but coming soon) 
>> reachabilityFence():
>>
>>         public PhantomCleanable(T referent, Cleaner cleaner) {
>>             super(Objects.requireNonNull(referent), 
>> getCleanerImpl(cleaner).queue);
>>             this.cleanerImpl = getCleanerImpl(cleaner);
>>             insert();
>>             Reference.reachabilityFence(cleaner);
>>         }
>>
>
> ...similar race could be devised around the reachability of 
> 'referent'. As soon as it is assigned to the Reference.referent field, 
> it is not needed any more, so it can be declared 
> (phantom|weakly|softly)-reachable by JVM before the XxxCleanable 
> instance is insert()ed causing it to be attempted to be remove()d 
> before it is insert()ed. The full fix therefore needs this:
>
>         public PhantomCleanable(T referent, Cleaner cleaner) {
>             super(Objects.requireNonNull(referent), 
> getCleanerImpl(cleaner).queue);
>             this.cleanerImpl = getCleanerImpl(cleaner);
>             insert();
>             Reference.reachabilityFence(cleaner);
>             Reference.reachabilityFence(referent);
>         }
>
Someone more familiar with the constructor/gc interactions might need to 
weigh in on this.
There must be some accommodation or lots of existing code in 
constructors would be/have broken.

Thanks, Roger


>
> Regards, Peter
>
>>
>> Regards, Peter
>>
>> On 10/20/2015 08:28 PM, Roger Riggs wrote:
>>> Sorry for the silence, JavaOne preparations and the availability of 
>>> folks who wanted
>>> to review have stretched things out.
>>>
>>> The Cleaner API was very simple and saw feature creep as the ideas 
>>> for how it might be used
>>> were explored.  There are concerns about committing to supporting 
>>> subclassable
>>> CleanableReferences in all future JDK versions before there had been 
>>> a chance to
>>> see how if they would be useful and necessary to address the need to 
>>> reduce the
>>> use of finalization within the OpenJDK and beyond.
>>>
>>> Recent updates:
>>>  - The Cleaner implementation classes and the CleanableReference 
>>> abstract classes are
>>>    now in the jdk.internal.misc package and are available within the 
>>> java.base module.
>>>  - The Cleanable.clear method has been dropped; there is no current 
>>> use case.
>>>    Since the CleanableReferences extend Reference, clear() is 
>>> available when subclassing.
>>>  - The tests have been extended to cover the exported and internal APIs
>>>
>>> The Runnable thunk version is very convenient to code but does 
>>> transparently create
>>> an additional object to hold the bindings.
>>> As the Cleaner is applied to the various uses of finalize we'll see 
>>> how they would be used
>>> and can re-evaluate the exported API needs.
>>>
>>> Updated Javadoc:
>>>   http://cr.openjdk.java.net/~rriggs/cleaner-doc/
>>>
>>> Updated Webrev:
>>>    http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>>>
>>
>