RFR 8157318: ThreadedSeedGenerator uses System.currentTimeMillis and stops generating when time is set back

Xuelei Fan xuelei.fan at oracle.com
Tue Jun 21 02:30:59 UTC 2016


Looks fine to me.

long l = System.nanoTime();
'l' is easy to be confused with '1'.  May be nice to use a meaningful
name, for example 'startTime'.

Xuelei

On 6/21/2016 9:39 AM, Weijun Wang wrote:
> Ping again.
> 
> On 6/16/2016 10:32, Wang Weijun wrote:
>>
>>> On Jun 16, 2016, at 6:37 AM, David M. Lloyd <david.lloyd at redhat.com>
>>> wrote:
>>>
>>> This will not work because System.nanoTime() can be negative and wrap
>>> around.  You can't compare one nanoTime to another, only differences
>>> are useful.
>>
>> Yes, you are right. How about this then?
>>
>> diff --git
>> a/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>> b/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>> ---
>> a/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>> +++
>> b/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>> @@ -354,8 +354,8 @@
>>                          // We wait 250milli quanta, so the minimum
>> wait time
>>                          // cannot be under 250milli.
>>                          int latch = 0;
>> -                        long l = System.currentTimeMillis() + 250;
>> -                        while (System.currentTimeMillis() < l) {
>> +                        long l = System.nanoTime();
>> +                        while (System.nanoTime() - l < 250000000) {
>>                              synchronized(this){};
>>                              latch++;
>>                          }
>>
>> Thanks
>> Max
>>
>>>
>>> On 06/15/2016 05:30 PM, Bradford Wetmore wrote:
>>>> Looks good, Max, thanks for taking this over.
>>>>
>>>> Xuelei, it's a threaded accumulator, which feeds into the seed
>>>> generator.  We wait a certain amount of time before generating the
>>>> values.
>>>>
>>>> Brad
>>>>
>>>>
>>>> On 6/15/2016 5:07 AM, Xuelei Fan wrote:
>>>>> I'm not sure I understand the while loop.  But this update looks
>>>>> fine to
>>>>> me as it does not change the logic of the code except to use elapsed
>>>>> time.
>>>>>
>>>>> Xuelei
>>>>>
>>>>> On 6/15/2016 11:12 AM, Wang Weijun wrote:
>>>>>> Please take a review on the patch
>>>>>>
>>>>>> diff --git
>>>>>> a/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>>>>>> b/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> a/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>>>>>>
>>>>>> +++
>>>>>> b/src/java.base/share/classes/sun/security/provider/SeedGenerator.java
>>>>>>
>>>>>> @@ -354,8 +354,8 @@
>>>>>>                         // We wait 250milli quanta, so the minimum
>>>>>> wait time
>>>>>>                         // cannot be under 250milli.
>>>>>>                         int latch = 0;
>>>>>> -                        long l = System.currentTimeMillis() + 250;
>>>>>> -                        while (System.currentTimeMillis() < l) {
>>>>>> +                        long l = System.nanoTime() + 250000000;
>>>>>> +                        while (System.nanoTime() < l) {
>>>>>>                             synchronized(this){};
>>>>>>                             latch++;
>>>>>>                         }
>>>>>>
>>>>>> nanoTime() is for elapsed time and should be used here.
>>>>>>
>>>>>> Thanks
>>>>>> Max
>>>>>>
>>>>>
>>>
>>> -- 
>>> - DML
>>




More information about the security-dev mailing list