On Wed, 19 Jul 2023 17:26:52 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> This patch fixes random deadlocks in PKCS11 decryption and encryption code.
>>
>> The deadlocks were caused by object allocation in `ckAssertReturnValueOK`
>> waiting for GC; GC was in turn waiting for `ReleasePrimitiveArrayCritical`,
>> which never happened.
>>
>> The fix moves the call to `ckAssertReturnValueOK` after
>> `ReleasePrimitiveArrayCritical`.
>>
>> All tests in sun/security/pkcs11 were executed with
>> `JTREG=JAVA_OPTIONS=-Xcheck:jni` with and without this patch.
>> Without this patch, 5 tests produced the following warning:
>>
>> Warning: Calling other JNI functions in the scope of
>> Get/ReleasePrimitiveArrayCritical or Get/ReleaseStringCritical
>>
>> With this patch the warning was not observed.
>>
>> Tier2-3 tests are still green.
>
> src/jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c line 172:
>
>> 170: &ckEncryptedLen);
>> 171:
>> 172: if (directIn == 0 && inBufP != NULL) {
>
> with this change, inBufP and outBufP should be non-null when directIn ==0 and
> directOut == 0, right? Perhaps we only need to check for one instead of both.
Same goes for the rest of the file. Otherwise, changes look good.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14931#discussion_r1268402091
