RFR: 8258077: Using -Xcheck:jni can lead to a double-free after JDK-8193234

[Mauro Lacy]

On Tue, 15 Dec 2020 08:18:28 GMT, Mauro Lacy <github.com+11656534+maurolacy at openjdk.org> wrote:

>> 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 the `-Xcheck:jni` flag is set or not. This is inconsistent behaviour. Probably introduced when trying to satisfy both usage patterns.
>> 
>> Besides, I don't think the same is true of the other `Release` methods. This happens only with the `Critical` variant.
>
>> Besides, I don't think the same is true of the other `Release` methods. This happens only with the `Critical` variant.
> 
> Sorry for (contributing to) the confusion. The reported double free error always mentions `ReleasePrimitiveArrayCritical`, but as Dmitry says, this ends up affecting both APIs.

Hi David,

Thanks for researching this thoroughly.
What you propose is reasonable, given the circumstances: undocumented or badly documented usage in the specs, and a lot of existing code (even in books, etc) that use `JNI_COMMIT` basically as `0`. We faced the same weeks ago, when looking for examples, documentation, and use cases for this functionality.

A couple comments:
- Given that `JNI_COMMIT` is then effectively useless, maybe a good idea would be to deprecate it. This will break existing code, true, but a deprecation notice can be used, by example in the form of a compilation warning or so, for code that calls / uses it. Same in the docs, i. e. this is kept for historical reasons but will be deprecated soon, and so on.
- I think in this case the best for us would be to simply drop support for `JNI_COMMIT` in the Rust JNI library. At least, for the ArrayCritical methods.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1697