Hi David,
Thanks for the comments,
Updated the javadoc and webrev with editorial changes.
[1]http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
[2]http://cr.openjdk.java.net/~rriggs/cleaner-doc/index.html
On 12/6/15 7:46 PM, David Holmes wrote:
Hi Roger,
Sorry to be late here but was trying not to get involved :)
It is already implicit that ThreadFactory.newThread should return
unstarted threads - that is what a new Thread is - so I don't think
IllegalThreadStateException needs to be documented here as it is
documenting behaviour of a broken ThreadFactory (and a broken
ThreadFactory could throw anything) .
It does seem that new is fairly well understood but one can read of
ThreadFactory is as a bit ambiguous, lacking a direct reference to the
Thread.State of the new thread
and since it allows various attributes of the thread to be modified
after the constructor.
Since the runnable is supplied as an argument it is ready to be started,
why not.
It seemed useful to reinforce the salient points.
Also the no-arg cleaner() can also throw SecurityException.
The thread construction is done in doPriv so it should not throw.
Did I miss some edge case?
Also this:
127 * On each call the {@link ThreadFactory#newThread(Runnable)
thread factory}
128 * should return a {@link Thread.State#NEW new thread} with
an appropriate
129 * {@linkplain Thread#getContextClassLoader context class
loader},
130 * {@linkplain Thread#getName() name},
131 * {@linkplain Thread#getPriority() priority},
132 * permissions, etc.
then begs the questions as to what is "appropriate". I think this can
be said much more simply as: "The ThreadFactory must provide a Thread
that is suitable for performing the cleaning work". Though even that
raises questions. I'm not sure why a ThreadFactory is actually needed
here ?? Special security context? If so that could be mentioned, but I
don't think name or priority need to be discussed.
It was intended to prod the client to be deliberate about the threadFactory.
Since the client is integrating the Cleaner and respective cleaning
functions
with the client code, the ThreadFactory makes it possible for the client to
initialize a suitable thread and the comments serve as a reminder.
I agree that the phrase 'suitable for performing the cleaning work' is
the operative one.
Thanks, Roger
Thanks,
David
