RFR: JDK-8049516: sun.security.provider.SeedGenerator throws ArrayIndexOutOfBoundsException

[Jamil Nimeh]

Hi Max and Xuelei, thanks for the feedback.


On 09/20/2016 07:52 PM, Wang Weijun wrote:
>> On Sep 21, 2016, at 9:58 AM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>
>>   359   while (System.nanoTime() - startTime < 250000000) {
>>   360       synchronized(this){};
>> - 361       latch++;
>> + 361       latch = (latch + 1) % Integer.MAX_VALUE;
>>   362   }
>>
>> This block may be not CPU friendly as it may loop a large amount of times in a very short period (250milli).
You asked about the empty synchronized block also: From what I've been 
reading on this topic it looks like the use of the empty synchronized 
block can be used to force cache coherency between multiple threads.  In 
terms of it being CPU intensive, has seed generation ever pegged a 
processor in the past?

There were cases in the past where it would hang, but that was fixed 
back when Max changed things in the inner loop to use System.nanoTime()  
(see JDK-8157318) but at that point (only 3 months ago) we didn't feel 
the need to restructure the loop.  I don't know that we do at this point 
either.  But we certainly can fix the overflow of the latch easily enough.
> To get a <255 index I think we only need to loop for <66536 times.
>
> How about we stop at every millisecond and see if it's enough? Something like this:
>
>      long next = startTime + 1000000;
>      while (next < startTime + 250000000) {
>          while (System.nanoTime() < next) {
>              synchronized(this){};
>              latch++;
>          }
>          if (latch > 65535 || latch < 0) break;
>          next += 1000000;
>      }
>
>> What's the usage of line 360?  Just for some computation?
>>
>> 367   counter += latch;
>> The counter variable may be overflow too.
> I find this strange. Were computers so slow in 1996 that within 250ms latch cannot exceed 64000?
1996?  You're talking about pentium and pentium 2 machines so at best 
you're talking 450MHz.  I don't know if the latch wouldn't pop under 
those conditions.

As for the counter, a potential overflow I don't think is that bad given 
the way the loop control is written.  At worst it just means another 
spin around the outer loop and another byte dropped in the pool.  And 
that loop can only iterate 6 times at the most so it's not like things 
can run away.


>
> --Max
>
>> Xuelei
>>
>> On 9/21/2016 8:57 AM, Jamil Nimeh wrote:
>>> Hello all,
>>>
>>> This fixes a bug found in stress testing where on faster CPUs the latch
>>> can overflow resulting in a negative array index.  The fix avoids the
>>> overflow by resetting the latch to 0 when it reaches Integer.MAX_VALUE -
>>> 1 and will continue increasing from there.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8049516
>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8049516/webrev.01/
>>>
>>> Thanks,
>>> --Jamil