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

[Mandy Chung]

> On Dec 3, 2015, at 1:19 PM, Roger Riggs <roger.riggs at oracle.com> wrote:
> 
> [1] http://cr.openjdk.java.net/~rriggs/cleaner-doc/
> [2] http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/

The implementation looks good.  I’ll wait for an updated javadoc and re-review that incorporates the editorial comments that Mark made.

Some additional comments:
- null should be {@code null}

 150      * Refer to the API Note above for cautions about the behavior
 151      * of cleaning functions.

It may be good to add a link back to the API note.

76  * and not block. If the cleaning function blocks it may delay processing
- should there be “,”  before “it…”

  77  * other cleaning functions using the same Cleaner.
  78  * All cleaning functions using a Cleaner should be mutually compatible.
- “using a cleaner” - should the terminology be “registered in a cleaner” consistent with other references in the spec?

 104      * The thread sets the ThreadContextClassLoader to the system class loader,
 105      * sets the AccessControlContext to an empty ProtectionDomain,
 106      * clears ThreadLocals, and sets the thread to be a
 107      * {@link Thread#setDaemon(boolean) daemon thread}.

I think clearing thread locals may not need to be mentioned.  Below is one suggestion to rephrase the above.  Similarly can be applied to the thread factory variant.

* The thread is a {@linkplain Thread#isDaemon() daemon thread}.
* The thread's {@linkplain Thread#getContextClassLoader context class loader}
* is set to {@linkplain ClassLoader#getSystemClassLoader() system class loader}.
* The thread has no permission to access any security operation.

Minor comment on the CleanerTest.java:

 530                     // unless it is know this case throws an exception, rethrow

typo: s/know/known

It would be better to rename the test[1-5] methods to reflect what it verifies.

 254             Reference<?> r = queue.remove();

Minor one: remove(long timeout) may be a better option while in practice remove() will not block forever.

Mandy