RFR: 8193234: When using -Xcheck:jni an internally allocated buffer can leak (original issue).

[David Holmes]

Hi Mauro,

On 11/12/2020 4:01 am, Mauro Lacy wrote:
> Hello,
> 
> I'm a contributor of the [jni-rs](https://github.com/jni-rs/jni-rs/) project. Rust bindings to the JNI.
> 
> When running tests against OpenJDK with the flag "-Xcheck:jni" enabled, we detected the following: the behaviour of the `JNI_COMMIT` mode of `ReleasePrimitiveArrayCritical` seems to be inconsistent, when running with `-Xcheck:jni` enabled, vs. without `-Xcheck:jni`.
> 
> If using "-Xcheck:jni" we're hitting a double free using `ReleasePrimitiveArrayCritical` with `JNI_COMMIT`, when we attempt to release the buffer at the end. It is our understanding that the behaviour of `ReleasePrimitiveArrayCritical` must be consistent, independently of whether the check JNI mode is enabled or not.
> 
> You can check [https://github.com/jni-rs/jni-rs/issues/283](https://github.com/jni-rs/jni-rs/issues/283) for details / comments on this.
> 
> We've tracked down the issue to this February 2019 commit: [openjdk commit 3e904a4](https://github.com/openjdk/jdk/commit/3e904a4801b2bf2e988ba096e5cb64a17fd5fce7). The related discussion is here: [openjdk bug JDK-8193234](https://bugs.openjdk.java.net/browse/JDK-8193234).
> 
> As there are no associated tests or functionality in the OpenJDK code base, I've simply reverted the commit. Just in case, I've also built and tested the JDK, without issues.
> 
> Not sure what tests are the ones mentioned in the discussions that led to the introduction of this behaviour. It is our understanding that tests of `JNI_COMMIT` functionality must still make a call to `ReleasePrimitiveArrayCritical` with a proper mode (`0` or `JNI_ABORT`) at the end, to release the buffer. And this, independently of `-Xcheck:jni` being enabled or not.
> 
> If there are any issues related to this that we're not aware of, please let us know.

If you just reverse the previous fix then you have reintroduced the 
memory leak that it was fixing - so we can't just do that.

I will take a closer look to understand the double-free issue that you 
are seeing.

Note that your PR is not valid as you can't create a PR for an issue 
that is already closed and fixed. This will need a new issue created in 
JBS and then a PR created for that. Please close the PR you opened for this.

Thanks,
David

> This reverts commit 3e904a4801b2bf2e988ba096e5cb64a17fd5fce7.
> 
> -------------
> 
> Commit messages:
>   - Revert "8193234: When using -Xcheck:jni an internally allocated buffer can leak"
> 
> Changes: https://git.openjdk.java.net/jdk/pull/1697/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1697&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8193234
>    Stats: 7 lines in 1 file changed: 4 ins; 3 del; 0 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/1697.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/1697/head:pull/1697
> 
> PR: https://git.openjdk.java.net/jdk/pull/1697
>