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

[Mandy Chung]

> On Dec 4, 2015, at 8:56 AM, Roger Riggs <roger.riggs at oracle.com> wrote:
> 
> 
> [1] http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
> [2] http://cr.openjdk.java.net/~rriggs/cleaner-doc/index.html

  67  * Unless otherwise noted, passing a {@code null} argument to a constructor or
  68  * method in any class or interface in this package will cause a

s/in any class or interface in this package/in this class/

 100      * Returns a new Cleaner.
s/Cleaner/{@code Cleaner}

For the create() factory method: should it prepare for future optimization (e.g. returning a shared Cleaner object, or multiple Cleaner object sharing among one or more dedicated threads)?   Would it be better not to require a new thread be created and also “Return a Cleaner” instead of “a new Cleaner”.

 105      * of the thread is set to the system class loader.

  - can you add a link to ClassLoader#getSystemClassLoader

 106      * The thread has no permissions, enforced only if a
 107      * {@link java.lang.System#setSecurityManager(SecurityManager) SecurityManager is set}.

I realized I didn’t bring this up in my previous comment.  I meant to say that it may be fine not to mention about “inherited ACC” and permissions.  I included that sentence just in case there is a reason it needs.  I am not aware of any specification talking about the inherited ACC of a newly created thread.   This sentence can be removed.  Maybe you can add a @apiNote to explain it.

> 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.

Regarding the exceptions if threadFactory.newThread returns a started thread, @throws IllegalThreadStateException is a good one and I think that’s all.  A new thread implies “not started” - perhaps adding a link to Thread.State.NEW.  I did look at whether the security exception will be thrown and I don’t see any.  You should double check. 

Cleanable is missing @since 9

 164      * Cleanable is a registered cleaning function.

I read “thunk” passed to Cleaner::register is the “registered cleaning function”.  What about: 

{@code Cleanable} represents the object and its associated cleaning function registered in a {@code Cleaner}

Mandy