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

[Roger Riggs]

Hi Peter,


On 12/04/2015 04:31 PM, Peter Levart wrote:
>
> ...
>>
>>>
>>>
>>> Would writing something like the following make sense: "A future 
>>> implementation may use more than one thread. The ThreadFactory 
>>> should not assume that only one thread will be requested." ?
>> I'm not sure it adds anything to talk about how many times the thread 
>> factory will be used.
>> A ThreadFactory has no specific mins or maxes.
>>
>> Roger
>
> Yes, but other parts of javadoc strictly mention a thread (as singular 
> object).
>
> Users are sloppy and like to depend on implementation details. They 
> will be tempted to write code like this:
>
> Thread t = new Thread(...);
> t.setName(...);
> ...
> Cleaner c = Cleaner.create(() -> t);
>
> ...and it will work. But will fail if some time in the future, Cleaner 
> decides to request multiple threads. Are you sure Cleaner will always 
> use a single thread? It will if the spec. says so. But I don't think 
> there is a reason to specify that and limit the implementation to one 
> thread forever.
ok,  how about:

* On each call the {@link ThreadFactory#newThread(Runnable) thread 
factory} * should return a new thread, {@link Thread#start() not 
started}, * with an appropriate * {@linkplain 
Thread#getContextClassLoader context class loader}, * {@linkplain 
Thread#getName() name}, * {@linkplain Thread#getPriority() priority}, * 
permissions, etc.

Which brings up a gap in the specification of create() that it can throw 
exceptions related
to creation and starting of threads, including SecurityExceptions, 
IllegalThreadStateException, etc.

If a future cleaner implementation were to use multiple threads, it 
would need to handle
exceptions from invoking ThreadFactory.newThread and be designed to cope 
with them.
A successfully created cleaner will have at least one thread to work with.

Thanks, Roger



>
> Regards,
>
> Peter
>
>>
>>>
>>> Regards, Peter
>>>
>>> On 12/04/2015 05:55 PM, Roger Riggs wrote:
>>>> Hi,
>>>>
>>>> Thanks for the review and comments.
>>>>
>>>> The webrev[1] and javadoc[2] are updated in place.
>>>>
>>>> Roger
>>>>
>>>> [1] http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
>>>> [2] http://cr.openjdk.java.net/~rriggs/cleaner-doc/index.html
>>>>
>>>> On 12/3/2015 4:50 PM, mark.reinhold at oracle.com wrote:
>>>>> Looks good -- thanks for the further simplification.
>>>>>
>>>>> Minor editorial comments, to add what Kim and Chris noted:
>>>>>
>>>>>    - In many places you write, e.g., "Cleaner" rather than "{@code
>>>>>      Cleaner}".  For consistency with the rest of the package it'd be
>>>>>      better in most cases just to write "cleaner" or, if its 
>>>>> nature as
>>>>>      a class is important, write "{@code Cleaner}".  The same goes 
>>>>> for
>>>>>      Cleanable, Thread, ThreadFactory, and all other types.
>>>>>
>>>>>    - The specification of Cleaner::create() mentions
>>>>>      "ThreadContextClassLoader", but that's not actually a type 
>>>>> anywhere
>>>>>      in the JDK.  Suggest "{@linkplain
>>>>>      java.lang.Thread#getContextClassLoader context class loader}.
>>>>>
>>>>>    - In the same method, it'd be helpful to provide links into the 
>>>>> Thread
>>>>>      class (or wherever) for the concepts of access-control 
>>>>> context and
>>>>>      thread locals.
>>>>>
>>>>> - Mark
>>>>
>>>
>>
>