RFR (S) 8059677: Thread.getName() instantiates Strings

[Aleksey Shipilev]

Hi Roger,

On 11/10/2014 05:31 PM, roger riggs wrote:
> 1) The Thread class javadoc says:
> " Unless otherwise noted, passing a {@code null} argument to a constructor
>  * or method in this class will cause a {@link NullPointerException} to be
>  * thrown."
> 
> So, NPE is already specified for setThreadName(null) or any other method.

Ah, thanks! It is odd to see this specified in a blanked fashion in the
class Javadoc, oh well. So we need to restore the NP check.


> I'm not infavor of adding the Objects.requireNonNull, the NPE will
> be thrown soon enough and it is just noise in the source code in most
> cases that creates larger bytecodes and extra work for the compiler
> /interpreter.

Sorry, I have a hard time understanding what you are saying. How would
you guarantee NPE (as per Javadoc contract above) in the new version of
Thread.setName otherwise?


> 2) About not storing the name as a String, I have some vague 
> recollection of the issue being related to exposing an object
> settable by the application that can be used with synchronize and
> allows communication and sync issues between threads.

Again, I don't quite understand. Is it about storing the reference to
String as the thread name, that can potentially be used for external
synchronization?

If so, I have a hard time devising a sane test case that might fail with
this change. Internal code does not synchronize on Thread.name. Anyone
synchronizing on Thread.getName() result has broken synchronization with
current code. Anyone synchronizing on Thread.getName() result after this
patch will have that (ahem) fixed, plus a performance problem.


> Just because some test doesn't fail, doesn't mean there isn't a
> design/implementation constraint.

I should have said, "I *also* run the jvmti and serviceability tests" to
confirm the change in innocuous. See the HS code change itself -- it
does seem contained.

Thanks,
-Aleksey.