Is it possible to try "md instanceof Cloneable || md.clone() returns"? Hopefully this is fast enough in most cases and also has the chance to recognize more actually-cloneable ones.
I'm also OK with simply using "md instanceof Cloneable". --Max > On Jun 6, 2020, at 12:02 PM, Valerie Peng <valerie.p...@oracle.com> wrote: > > > I am merely following the spec's recommendation of trying the clone() for > cloneability check. > > If you both are ok with it and prefer the instanceof check, I can sure > reverting back the changes in HmacCore and HandshakeHash classes. > > Valerie > > On 6/5/2020 2:04 AM, Seán Coffey wrote: >> I share the same concern. clone() is a heavy weight operation in >> constructors that can be called alot during intensive crypto operations. >> >> Now that you've done work on Delegate class - why not also keep the >> (instanceof Cloneable) test ? That can serve as your fastpath for the >> default JDK configuration AFAIK. >> >> regards, >> Sean. >> >> On 05/06/2020 00:16, Weijun Wang wrote: >>> >>>> 在 2020年6月5日,03:19,Valerie Peng <valerie.p...@oracle.com> 写道: >>>> >>>>> Can you give an example when these 2 rules have different results? Is >>>>> this only true for those implementation that directly subclass >>>>> MessageDigest? >>>> Before this fix, even the Spi impl implements Cloneable the instanceof >>>> check will always fail because the wrapper class, i.e. >>>> MessageDigest.Delegate does not. However, if you call the clone() (made >>>> public by the MessageDigest class), it will succeed because >>>> Delegate.clone() checks to see if the spi object implements the Cloneable >>>> interface, if yes, it will proceed to call the spi clone(). So, for this >>>> scenario, the results are different, e.g. instanceof returns false, but >>>> clone() succeeds. This is just one example. Is this what you are asking? >>> No. >>> >>> I understand this case, but this has already been fixed. Is there any other >>> example? Or are you only follow the words in the spec? i.e. try clone() to >>> see if it’s cloneable. >>> >>> I am worried that try clone() is much heavier than just check instanof >>> Cloneable. >>> >>> Thanks, >>> Max