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

[Roger Riggs]

Hi Kim,

On 12/3/2015 5:29 PM, Kim Barrett wrote:
> On Dec 3, 2015, at 4:19 PM, Roger Riggs <roger.riggs at oracle.com> wrote:
> ...
>>> src/java.base/share/classes/jdk/internal/misc/CleanerImpl.java
>>>   237         /**
>>>   238          * Returns true if the list's next reference refers to itself.
>>>   239          *
>>>   240          * @return true if the list is empty
>>>   241          */
>>>   242         boolean isListEmpty() {
>>>   243             PhantomCleanable<?> list = cleanerImpl.phantomCleanableList;
>>>   244             synchronized (list) {
>>>   245                 return next == list;
>>>   246             }
>>>   247         }
>>>
>>> Based on the description, one might expect some occurrence of "this"
>>> in the body.  This function must only be applied to the special head
>>> sentinal, so a pre-condition is "list == this".  Either an assert or
>>> some mention of this would make this easier to understand.
>>>
>>> [Applies to all three of {Soft,Weak,Phantom}Cleanable.]
>>>
>> The implicit reference to 'this' is in via cleanerImpl.
>>
>> All of the functions insert, remove, isListEmpty need to lock on the list root
>> and explicitly referring to the list root (via the clearerImpl) in each method
>> keeps them aligned.
>> IsListEmpty could be recoded to operate only on the distinguished root entry
>> but it would be harder to see that the same lock is being used consistently.
> I think that, as written, it would return true if applied to the last element of a
> non-empty list.  (That never happens at present.)  If the test were
> "list == list.next” that (potential) failure mode would be eliminated.  That
> would also make the code and the comment line up nicely, though the
> comment would then be kind of redundant, as it doesn’t provide any
> explanation for why that works.  Your call.
Good point, that would reduce some possible doubt.  Will fix.

Thanks, Roger

>
> So looks good to me.
>
>