On 11/2/2016 8:47 PM, Wang Weijun wrote:

On Nov 2, 2016, at 5:34 PM, Xuelei Fan <xuelei....@oracle.com> wrote:

1.  More specific

  "A SecureRandom service provider can advertise that it is
   thread-safe by setting the service provider attribute
   "ThreadSafe" to "true" when registering the provider."

A service provider may contains many services implementations.  May need to be more 
specific to set "ThreadSafe" for SecureRandom only, rather the full provider is 
thread safe.  For example:

   map.put("SecureRandom.SHA1PRNG ThreadSafe", "true");

Otherwise, a service provider need to make sure all services are thread safe, 
or all services implementation are not thread safe.

How about changing "A SecureRandom service provider" to "A SecureRandom 
implementation"?

I don't think it is sufficient.  See bellow.


2. "true" is the only true property value.
   "If this attribute is not set or is "false", this class will
    instead ..."

If the attribute is set to "yes" or "hello, world!", I think it is the same as set to 
"false" per your current implementation.

   "Otherwise, this class will ... "

OK.


May need to update the implementation accordingly if you accept the comments.

Why update the implementation?

If I'm reading correct, you are setting the "ThreadSafe" for provider, but not for SecureRandom implementation. I may use the property similar to:

   map.put("SecureRandom.SHA1PRNG ThreadSafe", "true");

That's why I don't think above update is not sufficient.

Xuelei

Thanks
Max


Xuelei


On 11/2/2016 3:27 PM, Wang Weijun wrote:
Ping again.

There is an updated version at 
http://cr.openjdk.java.net/~weijun/7004967/webrev.01/ with doc-only changes.

Thanks
Max

On Aug 25, 2016, at 10:00 AM, Weijun Wang <weijun.w...@oracle.com> wrote:

Please review the enhancement at

http://cr.openjdk.java.net/~weijun/7004967/webrev.00/

Basically, we want SecureRandom to be more efficient by removing all 
synchronized keywords from its public methods and let an implementation to take 
care of thread-safety (We already did some in JDK-8098581). On the other hand, 
we need to make sure that existing implementations that have not synchronized 
correctly to behave just as good as before.

Therefore a new Service Attribute "ThreadSafe" is introduced. If you think your 
implementation is already thread-safe, set it to "true" and SecureRandom will be happy. 
Otherwise, don't set it and SecureRandom will continuously call your SecureRandomSpi engine methods 
in synchronized blocks.

Thanks
Max


Reply via email to