> On May 12, 2016, at 9:31 AM, Xuelei Fan <xuelei....@oracle.com> wrote: > > Looks fine to me except a minor comment: > > AbstractDrbg.java > ================= > 54 * Since 8098581, there is no more synchronized keyword on > SecureRandom APIs. > 55 * An implementation is required to protect shared access to > instantiate states > 56 * (instantiated, nonce) and DRBG states (v, c, key, reseedCounter). > > This looks more like a code review description and may not suitable in > source code as the code reader may not want to search for 8098581 and > look back the history. I would suggest remove these lines.
I'll do. Thanks. --Max > > Thanks, > Xuelei > > On 5/12/2016 9:16 AM, Wang Weijun wrote: >> Ping again, and webrev updated at >> >> http://cr.openjdk.java.net/~weijun/8156501/webrev.01/ >> >> Volatile keyword added to reseedCounter. >> >> Thanks >> Max >> >>> On May 9, 2016, at 11:51 AM, Wang Weijun <weijun.w...@oracle.com> wrote: >>> >>> Hi All >>> >>> Please review the fix at >>> >>> http://cr.openjdk.java.net/~weijun/8156501/webrev.00 >>> >>> Many thanks to Siba for discovering this problem. I have only benchmarked >>> nextBytes() before. >>> >>> Some clarifications: >>> >>> 1. No need to synchronize configure anymore() because it's always called >>> inside a constructor. >>> >>> 2. synchronized-between-double-check in engineNextBytes() to protect >>> reseedCounter. >>> >>> Thanks >>> Max >>> >> >