RFR: 8258077: Using -Xcheck:jni can lead to a double-free after JDK-8193234
Mauro Lacy
github.com+11656534+maurolacy at openjdk.java.net
Fri Dec 11 08:44:57 UTC 2020
On Fri, 11 Dec 2020 08:17:13 GMT, Mauro Lacy <github.com+11656534+maurolacy at openjdk.org> wrote:
>> There are no tests for this, this was about external users of JNI encountering the leak. I also just re-checked the JDK source and it seems this code also suffers from the COMMIT-only problem:
>>
>> ./jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c
>>
>> The debate is over what constitutes correct and incorrect usage here - as I just wrote in the bug report. I will see if there is a reasonable way to support both.
>
>> There are no tests for this, this was about external users of JNI encountering the leak. I also just re-checked the JDK source and it seems this code also suffers from the COMMIT-only problem:
>>
>> ./jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_crypt.c
>>
>> The debate is over what constitutes correct and incorrect usage here - as I just wrote in the bug report.
>
> Yes. The fact that the method name starts with `Release`, and is being used for `COMMIT` doesn't help in that regard.
> This is probably for another place / discussion, but IMO that functionality should be in different API calls (`CommitPrimitiveArrayCritical`, and `Commit<Type>ArrayElements`) which do just that: commit the buffer changes.
> The use of `JNI_COMMIT` in `ReleasePrimitiveArrayCritical` and `Release<Type>ArrayElements` is misleading, to say the least.
>
> In fact, in the Rust lib we just separated those into different methods.
> Ref. [ReleaseMode](https://github.com/jni-rs/jni-rs/blob/master/src/wrapper/objects/release_mode.rs) vs. [commit()](https://github.com/jni-rs/jni-rs/blob/ff6aafd98974fb8d9fff2eac4a05bf477002af09/src/wrapper/objects/auto_primitive_array.rs#L50).
>
>> I will see if there is a reasonable way to support both.
>
> I don't think so. Not consistently, at least.
David, I can't comment in the issue page (AFAIK), but I agree with what you wrote there.
Just one more thing: The behaviour of `ReleasePrimitiveArrayCritical` with `JNI_COMMIT` **changes** depending on if you use the `-Xcheck:jni` flag or not. This is inconsistent behaviour. Probably introduced when trying to satisfy both use cases.
Besides, I don't think the same is true of the other `Release` methods. This happens only with the `Critical` variant.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1697
More information about the hotspot-dev
mailing list