Review Request : 7071826 : Avoid benign race condition in initialization of UUID

[Mike Duigou]

On May 11 2012, at 03:12 , Alan Bateman wrote:

> On 11/05/2012 00:11, Mike Duigou wrote:
>> 
>> Hello all;
>> 
>> A benign but potentially expensive race condition was discovered in java.util.UUID. The initialization of numberGenerator may cause the creation of a number of SecureRandom instances if multiple threads are attempting to initialize the field at the same time. This can happen because creation of the SecureRandom instances, depending upon system configuration and state, may take a significant amount of time.
>> 
>> Using a shared SecureRandom instance for the numberGenerator is an optimization--nothing bad happens as a result of each thread using it's own instance. However, creation of multiple SecureRandom instances, especially during system startup, may be expensive or have high latency if no previous SecureRandom instances have been created.
>> 
>> Accordingly, a holder class is used to ensure that only a single SecureRandom instance is created. The holder also serves to defer initialization of numberGenerator until randomUUID() is first called. 
>> 
>> Webrev:
>> 
>> http://cr.openjdk.java.net/~mduigou/7071826/0/webrev/
>> 
>> This issue will be proposed for backport to Java 7u6 once integrated into Java 8
>> 
> The implementation change looks good to me. For the javadoc change then could you link to RFC 4122 on the IEFT site?

Yes.

> 
> I don't quite get the test update, it improves the test but I don't see how it might demonstrate the race condition.

It doesn't demonstrate the race condition. The test changes are mostly just a result of reviewing the tests while I planned the changes.

> Minor consistency comment is that the if statements use braces whereas the existing code doesn't,

According to javastyle guidelines we're supposed to always use braces. I've not modified existing cases where braces are not present unless modifying the if expression.

> also missing space in "if(". 

Fixed.

> 
> -Alan.
>