RFR 8241960: The SHA3 message digests are not thread safe when cloned

[Alexey Bakhtin]

Hello Valeri,

Thank you for review. Please, have a look at updated patch at :  https://cr.openjdk.java.net/~abakhtin/8241960/webrev.v1/

I’ve fixed your findings and renamed test class to ThreadSafetyTest. Jtreg already has ThreadSafeTest class for java.text.Normalizer functionality.

Thank you
Alexey

On 3 Apr 2020, at 02:51, Valerie Peng <valerie.peng at oracle.com<mailto:valerie.peng at oracle.com>> wrote:


Hi Alexey,

In general looks pretty good, just some comments on the regression test:

- line 28: The duration value 10 may be lowered to shorten the execution time. On a Linux machine, with threadFactor=5, each digest algo takes about (2xduration+2) sec and overall takes ~283sec. Cutting the duration value drastically to 2, I still can reproduce the bug. Maybe 4 would be a better default value considering the execution time.

- line 43: add "SHA-512/224", "SHA-512/256" to the ALGORITHM_ARRAY?

- line 50: instead of hardcoding 5 here, why not just use threadsFactor (assigned with default value 5 on line 48)?

- line 52-55: I think you meant to use 5 (instead of the value 180) as the default min duration on line 52. Then, use duration variable instead of the hardcoded  5 on line54. For testing purpose, why not just use the supplied value? You already have default values if none are supplied.

- Consider iterating through existing providers and remove isSHA3supported() method. This way all providers which supports the digest algorithm are tested and no provider-specific knowledge is needed. For example:

        Provider[] provs = Security.getProviders();

        for (Provider p : provs) {

            System.out.println("Testing provider " + p.getName() + "...");

            for (String alg: ALGORITHM_ARRAY) {

                try {

                    MessageDigest md = MessageDigest.getInstance(alg, p);

                    testThreadSafe(md, input, nTasks, duration, false);

                ....

- line 76: missing space char between } and catch.

Thanks,
Valerie


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20200403/f4757425/attachment.htm>