> On Dec 4, 2015, at 8:56 AM, Roger Riggs <[email protected]> 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
